[9.x] Database queries containing JSON paths support array index braces, part 2 - #41767
Merged
taylorotwell merged 3 commits intoApr 1, 2022
Merged
Conversation
Illuminate\Database\Query\Grammars\Grammar@wrapJsonFieldAndPath() and Illuminate\Database\Schema\Grammars\Grammar@wrapJsonFieldAndPath() were copy and pasted duplicates but the query version had JSON path fixes applied that the schema version doesn't have. Instead add a trait to make the two classes share these methods.
derekmd
commented
Mar 31, 2022
| protected function wrapJsonPathAttributes($path) | ||
| { | ||
| return array_map(function ($attribute) { | ||
| $quote = func_num_args() === 2 ? func_get_arg(1) : "'"; |
Contributor
Author
There was a problem hiding this comment.
protected function wrapJsonPathAttributes($path, $quote = "'")
{I forget if Laravel considers protected method argument changes as breaking. I guess userland may extend PostgresGrammar and override this method.
Similar to MySQL, SQL Server, and SQLite, make Postgres support
`$query->where('column->json_key[0]', 'foo')`. Postgres also allows
equivalent call `$query->where('column->json_key->0', 'foo')`.
Unlike the other database drivers, the SQL doesn't compile to a JSON
path expression. The array indices must be parsed from the string,
separating them into new segments. e.g.,
$query->where('column->json_key[0]', 'foo')
derekmd
force-pushed
the
fix-postgres-json-paths-with-array-index
branch
from
April 1, 2022 14:39
fb5179c to
ab59bf5
Compare
Member
|
Thanks @derekmd! We would be pretty lost without you 😅 |
|
I notice that Is that expected, can we have in any way the same |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #38391 meant to fix #26415
Related to today's PR #41756
The previous PR allowed the above query to match by JSON array index. But the fixes didn't apply to the Postgres database driver that overrides
PostgresGrammer@wrapJsonSelector().Postgres does already support this:
This fix allows the first PHP snippet to also work, bringing it in line with the other database drivers. SQL:
Now the GitHub Actions config has a Postgres container I've added some integration tests.
Illuminate\Database\Schema\Grammars\Grammar@wrapJsonFieldAndPath()was a copy and paste duplicate fromIlluminate\Database\Query\Grammars\Grammarand didn't receive the same fixes from the previous PR. It's a far edge case for column methodsvirtualAsJson()andstoredAsJson()to use an array index but it probably makes sense for these classes to share a trait for compiling SQL containing JSON paths.