Skip to content

json: address nested members, and remove them - #141

Open
hammad-ikhlaq-dubbizlelabs wants to merge 2 commits into
Faveod:masterfrom
hammad-ikhlaq-dubbizlelabs:json-nested-paths-and-remove
Open

json: address nested members, and remove them#141
hammad-ikhlaq-dubbizlelabs wants to merge 2 commits into
Faveod:masterfrom
hammad-ikhlaq-dubbizlelabs:json-nested-paths-and-remove

Conversation

@hammad-ikhlaq-dubbizlelabs

Copy link
Copy Markdown

Stacked on #140: this builds on the path builder that PR introduces, so the branch carries both commits and the diff against master shows both. Merge #140 first and this one collapses to its own commit — 22011b4 is the only one that belongs to this PR. Happy to rebase, squash the two together, or split differently, whatever suits how you want to take them.

Two things a json document could not be asked for.

A path with more than one segment

get and set took a single member name, so a member of a nested document was only reachable by composing calls — and on MSSQL not even then: JSON_VALUE returns a scalar, so a second get has nothing left to walk into. An Array is now a path:

Arel.json(col).get(['dynamic_fields', 'beds'])
# mysql: JSON_EXTRACT("col", '$."dynamic_fields"."beds"')
# pg:    "col"::jsonb #>> array['dynamic_fields', 'beds']
# mssql: JSON_VALUE("col", '$."dynamic_fields"."beds"')

Arel.json(col).set(['dynamic_fields', 'beds'], 3)
# mysql: JSON_SET("col", '$."dynamic_fields"."beds"', 3)
# pg:    jsonb_set("col"::jsonb, array['dynamic_fields', 'beds'], '3'::jsonb, true)

An Array is always a path and never a member name — a name is a string, an integer index, or an expression the server evaluates — and segment kinds mix freely, so ['rooms', 0, 'size'] is $."rooms"[0]."size". A segment the server computes still gets spliced in there, and only the literal runs around it are folded into the path string:

Arel.json(col).get(['dynamic_fields', t[:name]])
# JSON_EXTRACT("col", CONCAT('$."dynamic_fields"."', CAST("t"."name" AS char), '"'))

A single-segment path renders exactly as it did before, ->> on postgres included, which is what the should keep addressing a single member the way it did test pins down — it is the one new test that passes without the lib/ change.

Removal

There was no way to express one, so remove is new, taking any number of paths:

Arel.json(col).remove(['dynamic_fields', 'beds'], 'top')
# mysql: JSON_REMOVE("col", '$."dynamic_fields"."beds"', '$."top"')
# pg:    (("col"::jsonb #- array['dynamic_fields', 'beds']) #- array['top'])

JSON_REMOVE takes every path in one call; #- takes one, so several chain. Removing nothing is the document itself rather than invalid SQL, the way merge({}) already is.

MSSQL is left out on purpose: it removes a member by assigning NULL through JSON_MODIFY, which is the same function it would need for JsonSet — and it has no JsonSet today either. That felt like one coherent piece of work rather than half of it here.

Smaller things that fell out

  • On postgres a jsonb path is a text[], so an integer segment is rendered as text. array[0] is an int[], which no jsonb function accepts, so set(0, v) could not have run at all before this.
  • JsonGet#key and JsonSet#key are kept as readers, returning the single segment for a one-segment path and the whole path otherwise. Visitors read path now. Shout if you'd rather they were dropped outright.

Deliberately not here

A nested set still does nothing when an intermediate level is missing: JSON_SET will not create one, and jsonb_set's create_missing only covers the last segment. So set(['a', 'b'], 1) on {} is a no-op on both backends rather than {"a":{"b":1}}.

Making an assignment mean an assignment needs the container bootstrapped inside the same expression — on MySQL roughly JSON_SET(COALESCE(col, '{}'), '$."a"', COALESCE(JSON_EXTRACT(col, '$."a"'), JSON_OBJECT()), '$."a"."b"', 1). That is a semantic choice about what set promises, not a rendering detail, so it seemed better asked than assumed. Glad to follow up with it if you want that behaviour.

Verification

bundle exec rake test:to_sql — 52 runs, 261 assertions, 0 failures (45 after #140). With lib/ reverted, 6 of the 7 new tests fail: 3 failures + 3 errors.

Postgres and MySQL are both asserted at the to_sql level, since all_agnostic_test.rb#test_json is skipped unconditionally and does not run in CI. I have no server to hand, so nothing here has been executed against a real MySQL, postgres or MSSQL — the SQL is reasoned from the docs for each, and the MSSQL side is by symmetry with MySQL.

`JsonGet` and `JsonSet` built the path by concatenating '$.' with the key, so
the member name landed in the path unquoted. MySQL and MSSQL only accept that
bare form for a name that happens to look like an identifier: a dot, a dash or
a space in the name produces an illegal path expression and the server rejects
the whole statement. `Arel.json(col).get('floor-number')` was not addressable
at all.

Both accept the quoted form, `$."member"`, for any name, so the name is now
always quoted, with `"` and `\` escaped for the path parser. The connection's
own quoting round-trips the value it is given, so the two levels of escaping
compose.

The path is also emitted as one string literal rather than a concatenation
whenever the member name is known when the query is built, which MSSQL needs:
`JSON_VALUE` only accepts a literal or a variable as its path argument, never
an expression. A name the server computes - a column, a concatenation - still
goes through `CONCAT`, now into the quoted form too.

While here, an integer index rendered as `"$[0]"` with double quotes, which is
a string only while `ANSI_QUOTES` is off in MySQL and is an identifier in MSSQL
under the default `QUOTED_IDENTIFIER`. It is now a normal string literal.

The existing json coverage lives in `all_agnostic_test.rb#test_json`, which
opens with an unconditional `skip`, so none of it runs in CI. The tests here go
through the MySQL visitor at the to_sql level instead, which `test:to_sql`
already runs on every matrix entry; `FakeRecord::Connection` gained the two
methods the dialect version gates ask for so that is possible.

Postgres needs no change: it addresses members with `->>` and `jsonb_set(...,
array[...])`, where the name is a value rather than part of a path string.
Two things a json document could not be asked for.

**A path with more than one segment.** `get` and `set` took a single member
name, so a member of a nested document was only reachable by composing calls -
and on MSSQL not even then, because `JSON_VALUE` returns a scalar, so a second
`get` has nothing to walk into. An Array is now a path:

    Arel.json(col).get(['dynamic_fields', 'beds'])
    # mysql: JSON_EXTRACT("col", '$."dynamic_fields"."beds"')
    # pg:    "col"::jsonb #>> array['dynamic_fields', 'beds']
    # mssql: JSON_VALUE("col", '$."dynamic_fields"."beds"')

An Array is always a path and never a member name: a name is a string, an
integer index, or an expression the server evaluates, and those can be mixed
freely - `['rooms', 0, 'size']` is `$."rooms"[0]."size"`. A single-segment path
renders exactly as it did before, `->>` on postgres included.

**Removal.** There was no way to express one at all, so `remove` is new, taking
any number of paths:

    Arel.json(col).remove(['dynamic_fields', 'beds'], 'top')
    # mysql: JSON_REMOVE("col", '$."dynamic_fields"."beds"', '$."top"')
    # pg:    (("col"::jsonb #- array['dynamic_fields', 'beds']) #- array['top'])

`JSON_REMOVE` takes every path in one call; `#-` takes one, so several chain.
Removing nothing is the document itself rather than invalid SQL, the way
`merge({})` already is. MSSQL is left out: it removes a member by assigning
`NULL` through `JSON_MODIFY`, which is the same function it would need for
`JsonSet`, and it has no `JsonSet` today either.

On postgres a jsonb path is a `text[]`, so an integer segment is rendered as
text. `array[0]` is an `int[]`, which no jsonb function accepts, so `set(0, v)`
could not have run before this.

`JsonGet#key` and `JsonSet#key` are kept as readers, returning the single
segment for a one-segment path. Visitors now read `path`.

Note that a nested `set` still does nothing when an intermediate level is
missing - `JSON_SET` will not create one, and `jsonb_set`'s `create_missing`
only covers the last segment. Making an assignment mean an assignment needs the
container bootstrapped in the same expression, which is a separate change.
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.

2 participants