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

Eloquent ->where on JSON boolean does not perform as expected #45642

Closed
shengslogar opened this issue Jan 13, 2023 · 11 comments
Closed

Eloquent ->where on JSON boolean does not perform as expected #45642

shengslogar opened this issue Jan 13, 2023 · 11 comments

Comments

@shengslogar
Copy link
Contributor

shengslogar commented Jan 13, 2023

  • Laravel Version: 9.47.0
  • PHP Version: 8.2.1
  • Database Driver & Version: MySQL 8.0.27

Description:

Apologies if this has already been discussed. #27847 suggests that we should be able to query primitives on JSON columns, but an actual, raw, true or false inside a JSON column (that isn't nested in any object) cannot be queried via ->where.

Steps To Reproduce:

  1. Create JSON column: $table->json('json_column');
  2. Create model with JSON cast: class TestModel extends Model { protected $casts = [ 'json_column' => 'json' ]; }
  3. TestModel::create([ 'json_column' => true ]);
> TestModel::where('json_column', true)->exists();
false

> TestModel::whereRaw('json_column = true')->exists();
true

> TestModel::whereRaw('json_column IS true')->exists();
true

Conversely, if TestModel::create([ 'json_column' => [ 'foo' => true ] ]);:

> TestModel::where('json_column->foo', true)->exists();
true
@daniilskr
Copy link

https://laravel.com/docs/9.x/queries#json-where-clauses

To query a JSON column, use the -> operator:

I guess usage of the where clause for JSON columns without -> is not officially supported even though it might work in some cases

@github-actions
Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Jan 19, 2023

Problem is Connection@prepareBindings converts boolean bindings to integers

public function prepareBindings(array $bindings)
{
$grammar = $this->getQueryGrammar();
foreach ($bindings as $key => $value) {
// We need to transform all instances of DateTimeInterface into the actual
// date string. Each query grammar maintains its own date string format
// so we'll just ask the grammar for the format to get from the date.
if ($value instanceof DateTimeInterface) {
$bindings[$key] = $value->format($grammar->getDateFormat());
} elseif (is_bool($value)) {
$bindings[$key] = (int) $value;
}
}
return $bindings;
}

I don't see this changing any time soon, as many applications use boolean values for flag columns filtering, and most databases store those as integers.

To compare a boolean stored in a JSON field unwrapped of an object or array, you can keep using Builder@whereRaw, or propose a new where clause, something like whereJson, that will prevent these bindings to be cast.

@shengslogar
Copy link
Contributor Author

I haven't used JSON in MySQL enough to hold an informed opinion, but whereJson feels elegant enough, or simply exposing whereJsonBoolean that was added in #27847 (not exactly sure how stuff in Grammar is surfaced) could suffice:

/**
* Compile a "where JSON boolean" clause.
*
* @param \Illuminate\Database\Query\Builder $query
* @param array $where
* @return string
*/
protected function whereJsonBoolean(Builder $query, $where)
{
$column = $this->wrapJsonBooleanSelector($where['column']);
$value = $this->wrapJsonBooleanValue(
$this->parameter($where['value'])
);
return $column.' '.$where['operator'].' '.$value;
}

But – since we're talking about not converting bindings in any way (whereRaw('json_column = ?', [ true ]) in my top example doesn't work either), maybe a variation of whereRaw would be valuable in other scenarios? A flag to not touch bindings or a whereTrulyRaw variant... 🤔

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Jan 20, 2023

You are right ->whereRaw('json_column = ?', [ true ]) won't work as its bindings would still be processed into the Grammar@whereJsonBoolean method.

I considered your example without bindings, but of course a real world scenario would require bindings.

I just tried locally using Illuminate\Database\Query\Expression to wrap the value and it worked:

dd(TestModel::where('json_column', new Expression('true'))->exists()); // true

Generated query below:

select exists(select * from `dummies` where `json_column` = true) as `exists`

As this is verbose, you can add a macro to the query builder:

\Illuminate\Database\Query\Builder::macro('whereJsonBool', function ($column, $value) {
    $this->where(
        $column,
        new \Illuminate\Database\Query\Expression($value ? 'true' : 'false')
    );
});

dd(TestModel::whereJsonBool('json_column', true)->exists());

Note: I kept the full qualified class names to make clear which classes I am using.

@shengslogar
Copy link
Contributor Author

Ah, fascinating! This seems like a perfectly sound workaround. I'll probably implement this (not even with the macro) for now. I'll leave this issue open in case someone has a more creative PR idea, but wouldn't be butt-hurt if it was closed in favor of prioritizing more important issues 😁

Thanks for taking the time to look into this!

@rodrigopedra
Copy link
Contributor

@shengslogar as a side note, a good place to register your macros is inside a Service Provider's boot method.

Also see my updated code above, I changed the macro name so it does not conflict with the protected method you mentioned before.

@rodrigopedra
Copy link
Contributor

You're welcome, glad to help =)

@driesvints usually reviews updated issues in his morning, but I believe this one won't make the cut into the framework, as storing unwrapped JSON scalar types seems to be a very particular use case, and there is a workaround.

But any way you can give it a try.

@shengslogar
Copy link
Contributor Author

Agreed. Unless there's a smart way to have this work with normal query builder methods, it seems overkill to add a specific method for this. Thanks again for your help!

@rodrigopedra
Copy link
Contributor

@shengslogar I found this much tidy way:

dd(TestModel::whereRaw('IF(json_column, 1, 0) = ?', [ true ])->exists()); // true

Hope it helps you

@driesvints
Copy link
Member

Seems like there's a workaround to this for now so going to leave it at this. Thanks all 👍

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

No branches or pull requests

4 participants