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

[release/7.0] Fix to #29572 - Json: predicate on json bool property generates incorrect sql #31264

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Jul 14, 2023

Port of #29574
Fixes #29572

Description

Search condition visitor wasn't handling json scalar expressions properly - we should convert them into search conditions/values just like we do with regular columns

Customer impact

Queries that perform filtering based on bool value inside JSON generate invalid SQL. There is a workaround, but it's not straightforward for some of the cases. (need to convert bool column to object and then back to bool to trick our query optimizer)

How found

Multiple customer reports on 7.0

Regression

No. JSON support is new functionality in 7.0.

Testing

Added regression tests for affected scenarios.

Risk

Minimal: the bug is result of an oversight and one line/trivial change. We were not performing search condition conversion for JSON scalars, like we do for every other operator. Change only affects JSON scenarios of type bool. Also added quirk to revert to old behavior if necessary.

…rect sql

Search condition visitor wasn't handling json scalar expressions properly - we should convert them into search conditions/values just like we do with regular columns

Fixes #29572
@maumar maumar requested review from ajcvickers and roji July 14, 2023 04:36
@maumar maumar added this to the 7.0.x milestone Jul 14, 2023
@wtgodbe wtgodbe merged commit d40c9bc into release/7.0 Aug 2, 2023
@wtgodbe wtgodbe deleted the fix29572_70 branch August 2, 2023 18:21
@ajcvickers ajcvickers removed this from the 7.0.x milestone Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants