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

[9.x] Database queries containing JSON paths support array index references #38391

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

derekmd
Copy link
Contributor

@derekmd derekmd commented Aug 16, 2021

Fixes: #26415

When running queries that update JSON paths including an array index, Laravel compiles an SQL statement that escapes the array index as part of a JSON object key.

e.g.,

Database column owner.packages row with JSON payload:

[
    {
        "name":"laravel/dusk",
        "versions":["6.17.0", "6.16.0", "6.15.1", "6.15.0"]
    },
    {
        "name":"laravel/framework",
        "versions":["8.54.0", "8.53.1", "8.53.0"]
    }
]
DB::table('owner')
    ->where('packages[1]->name', 'laravel/framework')
    ->update(['packages[1]->versions[0]' => '9.0.0']);

For MySQL this compiles to:

UPDATE `owner`
SET `packages` = JSON_SET(`packages`, $"[1]"."versions[0]", '9.0.0')
WHERE json_unquote(json_extract(`packages[1]`, '$."name"')) = 'laravel/framework';

Which would instead append to the above payload with a new nested JSON object key:

{
    "0": {
        "name":"laravel/dusk",
        "versions":["6.17.0", "6.16.0", "6.15.1", "6.15.0"]
    },
    "1": {
        "name":"laravel/framework",
        "versions":["8.54.0", "8.53.1", "8.53.0"]
    },
    "[1]": {
        "versions[0]": "9.0.0"
    }
}

When escaping JSON path segments, avoid enclosing array dereference characters [ and ]:

UPDATE `owner`
SET `framework` = JSON_SET(`framework`, $[1]."versions"[0], '9.0.0')
WHERE json_unquote(json_extract(`packages[1]`, '$."name"')) = 'laravel/framework';

Database documentation for JSON path support

Each of Laravel's supported database drivers use the same syntax for JSON path array indices.

JSON path features supported by database drivers

Database software Identifier Object key Array index Key asterisk Array asterisk Key match double-asterisk
MySQL [x] [x] [x] [x] [x] [x]
MariaDB [x] [x] [x] [x] [x] [x]
PostgreSQL [x] [x] [x] [x]
SQLite [x] [x] [x]
SQL Server [x] [x] [x]

For the above features, Laravel supports the first three columns that all the database drivers implement. Segment-matching * / ** characters partially supported by some drivers still require Laravel developers to use raw SQL.

9.x upgrade guide

In the low likelihood use case that applications use JSON column payload object key names ending in regex \[.+\], databases will need to be updated to no longer use such characters.

@derekmd derekmd force-pushed the allow-json-path-array-indices branch from 0309f2c to c1d8a25 Compare August 16, 2021 00:22
@derekmd derekmd marked this pull request as ready for review August 16, 2021 00:24
@derekmd
Copy link
Contributor Author

derekmd commented Aug 16, 2021

I guess there's no CI test suite feedback targeting the 9.x master branch until Symfony 6 compatibility gets sorted?

@driesvints
Copy link
Member

Thanks for sending this in @derekmd!

Yes, I'm gonna try to get those sorted out today.

@driesvints driesvints marked this pull request as draft August 16, 2021 10:28
@driesvints
Copy link
Member

Put this PR in draft for now.

@driesvints
Copy link
Member

@derekmd you can rebase this now : )

e.g.,

DB::table('owner')
    ->where('packages[1]->name', 'laravel/framework')
    ->update(['packages[1]->versions[0]' => '9.0.0']);

Stop compiling:

UPDATE `owner`
SET `packages` = JSON_SET(`packages`, $"[1]"."versions[0]", '9.0.0')
WHERE json_unquote(json_extract(`packages[1]`, '$."name"')) = 'laravel/framework';
...

Instead avoid escaping array dereference characters:

UPDATE `owner`
SET `framework` = JSON_SET(`framework`, $[1]."versions"[0], '9.0.0')
WHERE json_unquote(json_extract(`packages[1]`, '$."name"')) = 'laravel/framework';
@derekmd derekmd force-pushed the allow-json-path-array-indices branch from c1d8a25 to f7c5b9e Compare August 17, 2021 15:41
@derekmd derekmd marked this pull request as ready for review August 17, 2021 15:47
@taylorotwell taylorotwell merged commit c95392a into laravel:master Aug 19, 2021
@taylorotwell
Copy link
Member

Thanks!

@driesvints
Copy link
Member

Thanks @derekmd! 👏

@CraigMonva
Copy link

Is there a limitation on including this in the next laravel 8 minor? Is it backwords breaking?

Always running into issues writing tests around this issue.

January 25th, 2022 for Laravel 9 seems so far off for this feature :(

@derekmd
Copy link
Contributor Author

derekmd commented Oct 8, 2021

@CraigMonva: It's possible for apps to introduce the fix with 8.x by extending the Grammar class for your DB driver and configuring it using Connection::setQueryGrammar():

app/Database/JsonPathFixGrammar.php

namespace App\Database;

// extend the driver-appropriate grammar for your DB connection, like MySqlGrammar
use Illuminate\Database\Query\Grammars\MySqlGrammar;
use Illuminate\Support\Str;

class JsonPathFixGrammar extends MySqlGrammar
{
    protected function wrapJsonPath($value, $delimiter = '->')
    {
        $value = preg_replace("/([\\\\]+)?\\'/", "''", $value);

        $jsonPath = collect(explode($delimiter, $value))
            ->map(function ($segment) {
                return $this->wrapJsonPathSegment($segment);
            })
            ->join('.');

        return "'$".(Str::startsWith($jsonPath, '[') ? '' : '.').$jsonPath."'";
    }

    protected function wrapJsonPathSegment($segment)
    {
        if (preg_match('/(\[[^\]]+\])+$/', $segment, $parts)) {
            $key = Str::beforeLast($segment, $parts[0]);

            if (! empty($key)) {
                return '"'.$key.'"'.$parts[0];
            }

            return $parts[0];
        }

        return '"'.$segment.'"';
    }
}

app/Providers/AppServiceProvider.php

namespace App\Providers;

use App\Database\JsonPathFixGrammar;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\ServiceProvider;

class AppServiceProvider extends ServiceProvider
{
    public function boot()
    {
        DB::connection()->setQueryGrammar(new JsonPathFixGrammar);
    }
}

@CraigMonva
Copy link

@derekmd

Hi Derek.

Upgrading to L9 which should include this? Am I miss understanding what this change was meant to fix?

Screenshot 2022-02-17 at 16 12 22

Screenshot 2022-02-17 at 16 14 13

My tests pass without asserting

'preferences->energy->channel->0' => 'email',
'preferences->energy->channel->1' => 'sms',

@CraigMonva
Copy link

@derekmd

Jesus christ nevermind. Thought Id been given a laravel shift branch with composer update already run on it. Aparently not.

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 this pull request may close these issues.

None yet

4 participants