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: support json_populate_record, json_populate_recordset, json_to_record, json_to_recordset #70037

Closed
4 tasks done
nvanbenschoten opened this issue Sep 10, 2021 · 8 comments · Fixed by #82435
Closed
4 tasks done
Labels
A-sql-builtins SQL built-in functions and semantics thereof. A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-postgrest C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@nvanbenschoten
Copy link
Member

nvanbenschoten commented Sep 10, 2021

Needed for #69010.

This was originally tracked in #20120.

We currently don't support the following four builtin functions, which are different ways to convert an unstructured JSON datum (or set of JSON datums) into a structure record datum (or set of record datums):

  • json_populate_record
  • json_populate_recordset
  • json_to_record
  • json_to_recordset

As with most builtins, these have corresponding jsonb versions which should get identical implementations.

These builtins will be more useful once we address #70036.

Epic CRDB-10300

Jira issue: CRDB-9913

@nvanbenschoten nvanbenschoten added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-builtins SQL built-in functions and semantics thereof. labels Sep 10, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Sep 10, 2021
@jordanlewis
Copy link
Member

I took a stab at doing this tonight. It was way more annoying than expected!

Here is the WIP branch: https://github.com/cockroachdb/cockroach/compare/master...jordanlewis:json-to-record?expand=1

The main annoyance is that functions that return records have a requirement that they're aliased to column definition lists, with types. Our parser doesn't support this - column aliases don't have types. Also, it means that we need to plumb alias information down into the thing that's getting aliased, which is awkward, and somehow plumb that info even deeper into the generator func factory. I think this is doable with a bit more work.

craig bot pushed a commit that referenced this issue Sep 20, 2021
70115: sql: add json{,b}_populate_record{,set} r=jordanlewis a=jordanlewis

Updates #69010
Updates #70037

These generator builtins permit type-safe transformation of JSON to
table data. They're added for compatibility with PostgreSQL.

Release note (sql change): add the json_populate_record,
jsonb_populate_record functions, json_populate_recordset, and
jsonb_populate_recordset functions, which transform JSON into row tuples
based on the labels in a record type.

70289: ci: add `bazel Nightlies / SQLite logic tests` job for CI r=rail a=rickystewart

Release note: None

Co-authored-by: Jordan Lewis <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
@otan
Copy link
Contributor

otan commented Oct 25, 2021

@jordanlewis your WIP branch is empty (at least, for json_to_record. did you do that?)

@jordanlewis
Copy link
Member

Oops, I must have screwed up the branch here - I ended up switching focus to the stuff we needed for json_populate_record.

json_to_record had some other problems that I never resolved, but I can re-push the WIP.

@jordanlewis
Copy link
Member

jordanlewis commented Oct 25, 2021

Re-pushed. I think the WIP was very WIP, please let me know if you have questions about it. The main summary is that the builtins need more information than what we have today - they need to know the AS clause that renames their outputs, which is kind of backwards in our system - normally, bits of a plan don't need to know the things that are consuming them. So the WIP was some work that plumbed down alias information into a place that the builtins could see.

@otan
Copy link
Contributor

otan commented Oct 25, 2021

urgh! yeah i had kinda the same problem trying to implement array concatenation (#71447), specifically with NULL array || NULL

@otan
Copy link
Contributor

otan commented Oct 26, 2021

do you recall why your WIP was still a WIP? or what else is left?

@jordanlewis
Copy link
Member

I just think I didn't finish implementing the builtin all the way. Sorry, I know this isn't much to go off. I can try to page back in the state of what was going on with the patch this week if it would help.

@otan
Copy link
Contributor

otan commented Oct 29, 2021

i've gone a little further on #72137. what a plumbing exercise!

basically, i'm stuck at passing sourceAlias through to Start so the generator knows what types/names to extract from the json here:

https://github.com/cockroachdb/cockroach/blob/d04b65a47bfd1cfc5efc8a801303aaa7ae6ee3ab/pkg/sql/rowexec/project_set.go#L159

i couldn't find anywhere obvious to put it and i'm pretty sure the way i fought the Start call in the optimiser is a complete hack. not sure what to do next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-builtins SQL built-in functions and semantics thereof. A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-postgrest C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants