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 in-memory SQL arrays of JSON #70041

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

jordanlewis
Copy link
Member

@jordanlewis jordanlewis commented Sep 10, 2021

refs #23468
refs #69010

This commit adds support for SQL arrays of JSON in memory. This allows
functions like array_agg to work against JSON objects, which is useful
for compatibility.

Notably, this commit does not add support for storing JSON arrays in
table columns.

Release note (sql change): add support for SQL arrays containing JSON
for in-memory processing. This does not add support for storing SQL
arrays of JSON in tables.

@jordanlewis jordanlewis requested review from a team September 10, 2021 17:22
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jordanlewis jordanlewis changed the title sql: support in-memory JSON arrays sql: support in-memory SQL arrays of JSON Sep 10, 2021
@rafiss
Copy link
Collaborator

rafiss commented Sep 10, 2021

out of curiosity, how do we handle the example from #59427 with this change?

@@ -2452,8 +2452,6 @@ func IsStringType(t *T) bool {
// the issue number should be included in the error report to inform the user.
func IsValidArrayElementType(t *T) (valid bool, issueNum int) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like we can remove this function now. unless you think it will be useful to track other types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought of this, but I figured if we do end up needing something like this again we'll have to remember both of these places and be sad. I think it's fine to leave this in.

@jordanlewis
Copy link
Member Author

The behavior is exactly the same. I think Andrew's explanation on that issue makes sense - we can't auto-cast arrays of JSON to json-array.

Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice quick win, thank you!

@jordanlewis jordanlewis force-pushed the json-arr branch 2 times, most recently from 4e05409 to 9ebe52a Compare September 12, 2021 18:44
This commit adds support for SQL arrays of JSON in memory. This allows
functions like `array_agg` to work against JSON objects, which is useful
for compatibility.

Notably, this commit does not add support for storing JSON arrays in
table columns.

Release note (sql change): add support for SQL arrays containing JSON
for in-memory processing. This does not add support for storing SQL
arrays of JSON in tables.
@jordanlewis
Copy link
Member Author

TFTRs 😎

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 13, 2021

Build succeeded:

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.

4 participants