Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON columns dont appear to be working #149

Open
matthewjumpsoffbuildings opened this issue Nov 28, 2023 · 10 comments
Open

JSON columns dont appear to be working #149

matthewjumpsoffbuildings opened this issue Nov 28, 2023 · 10 comments

Comments

@matthewjumpsoffbuildings
Copy link
Contributor

I have a model with a json column. As usual I have $casts = ['json_column' => 'array'] in the model definition, but for some reason every time I try to save/update the model I get this error

Value of type STRING cannot be assigned to data, which has type JSON 
@matthewjumpsoffbuildings
Copy link
Contributor Author

Upon further investigation, when using the JSON column type the generated SQL query should be

set column_name = json '{"foo":1}'

Note the json prefix. This is currently not being generated in theSQL

@taka-oyama
Copy link
Collaborator

There is no good solution to this.
The query builder has no way of distinguishing between a normal string and a json string.

@matthewjumpsoffbuildings
Copy link
Contributor Author

I noticed that in Connection.php, here -

$rowCount = $transaction->executeUpdate($query, ['parameters' => $this->prepareBindings($bindings)]);

I can do this, and it works:

$types = ['p0' => Database::TYPE_JSON];
$rowCount = $transaction->executeUpdate($query, ['parameters' => $bindings, 'types' => $types]);

Of course this is just me manually typing a particular binding, but surely there is some way, perhaps by assigning something other than a string to the Model, which laraval-spanner can then intercept before it gets converted into a string, and once its been intercepted it can know to add the 'types' => [] info to the transaction?

@taka-oyama
Copy link
Collaborator

Spanner didn't have JSON types initially (2019) and we just stored JSON as STRING(MAX).
That was good enough for our use case.

I'll update the docs to say it's not supported for the moment.

@taka-oyama
Copy link
Collaborator

PR is always welcome btw.

@matthewjumpsoffbuildings
Copy link
Contributor Author

I haven't dealt with custom database/eloquent drivers before so its hard to follow the chain of how data is set/escaped before being passed to the execute statement.

I am trying to work out how the escape functions work here:
https://github.com/colopl/laravel-spanner/blob/724159abe1ee844fd4cec7d16c08c50d4d200d7e/src/Connection.php#L419C3-L442C6

I was thinking perhaps if you pass an associative array, the escapeArray() method, instead of throwing an exception, could treat it as JSON?

But in my experimenting it doesnt even seem that the escapeArray() method gets called, at least not before this

$rowCount = $transaction->executeUpdate($query, ['parameters' => $this->prepareBindings($bindings)]);

@taka-oyama
Copy link
Collaborator

I am trying to work out how the escape functions work here

From what I can recall, escape is only used for the Builder::toRawSql which is only really used for logging and debugging.

@taka-oyama
Copy link
Collaborator

taka-oyama commented Nov 29, 2023

I was thinking perhaps if you pass an associative array, the escapeArray() method, instead of throwing an exception, could treat it as JSON?

That could potentially work but you might need to pass it as an object instead since empty arrays can't be treated as assoc array.

@matthewjumpsoffbuildings
Copy link
Contributor Author

matthewjumpsoffbuildings commented Dec 8, 2023

Im really struggling to work out how to intercept the query generation to insert the relevant types.

What I have so far is on the Model I have added a protected $types = [ 'ColumnName' => Database::TYPE_JSON ] property.

Then I have added this to the Builder

    protected $types = [];
    public function setTypes($types)
    {
        $this->types = $types;
        return $this;
    }

And in the model, I have overridden the newModelQuery() method to provide the types

    public function newModelQuery()
    {
        return parent::newModelQuery()->setTypes($this->types);
    }

So now every Builder for a given Model has a $types property that can contain the desired types for columns. The part I cant work out is how to inject these types into the Connections select(), statement() or affectingStatement() methods

When the query values are converted into bindings p0, p1 etc, I need to intercept this process, and if any of these pN values is for a column specified in the Model/Builders $types array, I need to generate an additional array of the structure

$types = [ 'pX' => Database::TYPE_X, 'pY' => Database::TYPE_Y;

But for the life of me I cant figure out where to do that

@matthewjumpsoffbuildings
Copy link
Contributor Author

The solution is a custom Cast with a custom ValueInterface, as per this comment

#154 (comment)

Should add this to the docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants