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.

Sign up for GitHub

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

Jump to bottom

sql/json: fix null's in array in inverted index support #101059

Merged
merged 1 commit into from
Apr 10, 2023
Merged

sql/json: fix null's in array in inverted index support #101059

merged 1 commit into from
Apr 10, 2023

Conversation

cucaroach
Copy link
Contributor

We assumed all the array elements were strings w/o checking for null,
now we use AsDString to check it.

Found internally with expanded sqlsmith testing.

Fixes: #101025
Epic: None
Release note: None

@cucaroach
sql/json: fix null array support in inverted index support
bc47c8b 
We assumed all the array elements were strings w/o checking for null,
now we use AsDString to check it.

Found internally with expanded sqlsmith testing.

Fixes: cockroachdb#101025
Epic: None
Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@cucaroach cucaroach changed the title sql/json: fix null array support in inverted index support sql/json: fix null's in array in inverted index support Apr 10, 2023
@cucaroach cucaroach marked this pull request as ready for review April 10, 2023 15:59
@cucaroach cucaroach requested a review from a team as a code owner April 10, 2023 15:59
@cucaroach cucaroach requested a review from yuzefovich April 10, 2023 15:59
yuzefovich
yuzefovich approved these changes Apr 10, 2023
View reviewed changes
Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

:lgtm:

nit: add backport labels.

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)

@cucaroach cucaroach added backport-23.1.x Flags PRs that need to be backported to 23.1 backport-22.2.x and removed backport-22.1.x labels Apr 10, 2023
@cucaroach
Copy link
Contributor Author

bors r=yuzefovich

@craig
Copy link
Contributor

craig bot commented Apr 10, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Apr 10, 2023

Build succeeded:

@craig craig bot merged commit 9c55b61 into cockroachdb:master Apr 10, 2023
This was referenced Apr 10, 2023
Merged
Merged
@mgartner
Copy link
Collaborator

In the future, don't forget to add release note for bug fixes.

@cucaroach
Copy link
Contributor Author

In the future, don't forget to add release note for bug fixes.

My bad! I didn't think it rose to the level of needing a release note since it was found internally and kind of an edge case, I guess fixing a source of internal errors is always worthy of a release note?

@mgartner
Copy link
Collaborator

My bad! I didn't think it rose to the level of needing a release note since it was found internally and kind of an edge case, I guess fixing a source of internal errors is always worthy of a release note?

I figure it's nice to have a release note in case a user stumbles into this in the future (if they are running an older release) - it might make it easier to find and figure out what they need to upgrade to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuzefovich yuzefovich yuzefovich approved these changes

Assignees
No one assigned
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

json: unexpected internal error: interface conversion: tree.Datum is tree.dNull, not *tree.DString
4 participants
@cucaroach @cockroach-teamcity @mgartner @yuzefovich