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

Improve wording on the whereJsonContains method and SQLite support #9198

Merged
merged 2 commits into from Dec 15, 2023
Merged

Improve wording on the whereJsonContains method and SQLite support #9198

merged 2 commits into from Dec 15, 2023

Conversation

danieleambrosino
Copy link
Contributor

I find the wording "This feature [whereJsonContains] is not supported by SQLite database versions less than 3.39.0" misleading. This wording suggests that the whereJsonContains method can be used by merely using a compatible version of SQLite, which is not true; while SQLite does support JSON functions since version 3.39.0 (as already stated in the previous paragraph), the SQLite Grammar does not support the whereJsonContains clause regardless of the version of SQLite.

I find the wording "This feature [whereJsonContains] is not supported by SQLite database versions less than 3.39.0" misleading.
This wording suggests that the "whereJsonContains" method can be used by merely using a compatible version of SQLite, which is not true; while SQLite does support JSON functions since version 3.39.0 (as already stated in the previous paragraph), the SQLite Grammar does not support the "whereJsonContains" clause regardless of the version of SQLite.
@taylorotwell
Copy link
Member

taylorotwell commented Dec 14, 2023

Hmm, what happens if you do it? Wouldn't it fall back to the base grammar's whereJsonContains? What error are you actually receiving?

Please mark as ready for review after responding.

@taylorotwell taylorotwell marked this pull request as draft December 14, 2023 19:34
@danieleambrosino
Copy link
Contributor Author

danieleambrosino commented Dec 15, 2023

If you try to build a query adding a whereJsonContains constraint while using the SQLite driver, a RuntimeException is thrown with the message "This database engine does not support JSON contains operations."
The exception is thrown here:

protected function compileJsonContains($column, $value)
{
    throw new RuntimeException('This database engine does not support JSON contains operations.');
}

This is due to the fact that SQLiteGrammar does not provide any implementation of the protected method compileJsonContains, hence it falls back to the implementation provided by its parent Illuminate\Database\Query\Grammars\Grammar, which just throws the aforementioned exception.

Edit: I also checked in the test suite and this behavior seems to be as expected, judging by the contents of this test:

public function testWhereJsonContainsSqlite()
{
    $this->expectException(RuntimeException::class);
    
    $builder = $this->getSQLiteBuilder();
    $builder->select('*')->from('users')->whereJsonContains('options->languages', ['en'])->toSql();
}

@danieleambrosino danieleambrosino marked this pull request as ready for review December 15, 2023 02:33
@danieleambrosino
Copy link
Contributor Author

I also just found out this issue opened a couple of weeks ago in the laravel/framework repo regarding the same topic, that's still open and flagged as "help wanted".
This kind of query is actually feasible in SQLite nowadays. I have a decent amount of experience dealing with unstructured data in SQLite. I'd be glad to contribute submitting a PR with a proposal of implementation.

@taylorotwell taylorotwell merged commit 0991136 into laravel:10.x Dec 15, 2023
@taylorotwell
Copy link
Member

@danieleambrosino got it - thanks. yeah, open to PRs if it's not too hard to fix!

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