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

sql/pgwire: need support for decoding JSON[] OID by aliasing to JSONB[] OID [unknown oid type: 199] #90839

Closed
danieljharvey opened this issue Oct 28, 2022 · 3 comments · Fixed by #94705
Assignees
Labels
A-tools-hasura C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-blathers-triaged blathers was able to find an owner

Comments

@danieljharvey
Copy link

danieljharvey commented Oct 28, 2022

Describe the problem

Since the fix in #88355 , CockroachDB recognises oid 114 as JSON, and internally aliases it to JSONB (with the oid 3802).

Would it be possible to make the same fix with JSON arrays? Ideally it would treat oid 199 (JSON[]) as oid 3807 (JSONB[])?

If we send the same queries with 3807 instead everything works.

Expected behavior
Ideally, JSON[] input with oid 199 would be accepted in the wire format and treated as JSONB[] internally.

Additional context
Discovered whilst implementing subscriptions for Hasura Graphql Engine

Expected oid values found in here: https://github.com/postgres/postgres/blob/5543677ec90a15c73dab5ed4f0902b3b920f0b87/src/include/catalog/pg_type.dat#L443

Jira issue: CRDB-20967
Epic CRDB-11916

@danieljharvey danieljharvey added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Oct 28, 2022
@blathers-crl
Copy link

blathers-crl bot commented Oct 28, 2022

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Oct 28, 2022
@rafiss
Copy link
Collaborator

rafiss commented Oct 28, 2022

We could do this, but CRDB does not support JSON arrays well right now (#23468). I expect that if we fix this, it would just fail at the next step.

We do have limited support for JSON arrays in aggregate functions (#70041), but I don't expect it to work as a query argument. Based on what you're saying, you got things to work with oid 3807. Could you share example code or queries of what you tried?

@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Oct 28, 2022
@rafiss rafiss added A-tools-hasura and removed T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Oct 28, 2022
@rafiss rafiss changed the title "unknown oid type: 199" sql/pgwire: need support for decoding JSON[] OID by aliasing to JSONB[] OID [unknown oid type: 199] Oct 28, 2022
@rafiss
Copy link
Collaborator

rafiss commented Nov 9, 2022

Here is an example that we can use to test the changes we make:

CREATE TABLE example (id INTEGER PRIMARY KEY, name TEXT);


SELECT "_subs"."result_id",
       "_fld_resp"."root" AS "result"
FROM unnest($1::UUID[], $2::JSON[]) AS "_subs"("result_id",
                                                   "result_vars")
LEFT OUTER JOIN LATERAL
  (SELECT json_build_object('hasura_example', "hasura_example"."root") AS "root"
   FROM
     (SELECT coalesce(json_agg("root"), '[]') AS "root"
      FROM
        (SELECT row_to_json(
                              (SELECT "_e"
                               FROM
                                 (SELECT "_root.base"."id" AS "id", "_root.base"."name" AS "name") AS "_e")) AS "root"
         FROM
           (SELECT *
            FROM "example"
            WHERE ('true') ) AS "_root.base") AS "_root") AS "hasura_example") AS "_fld_resp" ON ('true');
  • For $1 we're sending an array of UUIDs as unknown (sample value '{35638bc7-5d39-4860-8002-de1c4bd937c4}')
  • For $2 we're sending an array of JSON as oid 3807 (sample value: ARRAY['{"cursor":{},"query":{},"session":{},"synthetic":[]}'])

@rafiss rafiss added the E-quick-win Likely to be a quick win for someone experienced. label Nov 17, 2022
@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Dec 5, 2022
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jan 5, 2023
fixes cockroachdb#90839

Previously, with the parameter type oid (199) for JSON[], the pgwire parse message
will fail to be executed with a `unknown oid type: 199`. We now aliasing it
to the one for `JSONB[]`.

This commit follows the changes in cockroachdb#88379.

Release note (sql change): The pgwire protocol implementation can
now accept arguments of the JSON[] type (oid=199). Previously, it could
only accept JSONB[] (oid=3804). Internally, JSON[] and JSONB[] values are
still identical, so this change only affects how the values are received
over the wire protocol.
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jan 6, 2023
fixes cockroachdb#90839

Previously, with the parameter type oid (199) for JSON[], the pgwire parse message
will fail to be executed with a `unknown oid type: 199`. We now aliasing it
to the one for `JSONB[]`.

This commit follows the changes in cockroachdb#88379.

Release note (sql change): The pgwire protocol implementation can
now accept arguments of the JSON[] type (oid=199). Previously, it could
only accept JSONB[] (oid=3804). Internally, JSON[] and JSONB[] values are
still identical, so this change only affects how the values are received
over the wire protocol.
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jan 6, 2023
fixes cockroachdb#90839

Previously, with the parameter type oid (199) for JSON[], the pgwire parse message
will fail to be executed with a `unknown oid type: 199`. We now aliasing it
to the one for `JSONB[]`.

This commit follows the changes in cockroachdb#88379.

Release note (sql change): The pgwire protocol implementation can
now accept arguments of the JSON[] type (oid=199). Previously, it could
only accept JSONB[] (oid=3804). Internally, JSON[] and JSONB[] values are
still identical, so this change only affects how the values are received
over the wire protocol.
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jan 18, 2023
fixes cockroachdb#90839

Previously, with the parameter type oid (199) for JSON[], the pgwire parse message
will fail to be executed with a `unknown oid type: 199`. We now aliasing it
to the one for `JSONB[]`.

This commit follows the changes in cockroachdb#88379.

Release note (sql change): The pgwire protocol implementation can
now accept arguments of the JSON[] type (oid=199). Previously, it could
only accept JSONB[] (oid=3804). Internally, JSON[] and JSONB[] values are
still identical, so this change only affects how the values are received
over the wire protocol.
craig bot pushed a commit that referenced this issue Jan 19, 2023
94705: sql/pgwire: support for decoding JSON[] OID by aliasing to JSONB[] OID r=ZhouXing19 a=ZhouXing19

Previously, with the parameter type oid (199) for JSON[], the pgwire parse message
will fail to be executed with a `unknown oid type: 199`. We now aliasing it
to the one for `JSONB[]`.

This PR also includes a test revealing #95434.

This commit follows the changes in #88379.

fixes #90839

Release note (sql change): The pgwire protocol implementation can
now accept arguments of the JSON[] type (oid=199). Previously, it could
only accept JSONB[] (oid=3804). Internally, JSON[] and JSONB[] values are
still identical, so this change only affects how the values are received
over the wire protocol.

Co-authored-by: Jane Xing <zhouxing@uchicago.edu>
@craig craig bot closed this as completed in ce51054 Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tools-hasura C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-blathers-triaged blathers was able to find an owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants