Skip to content

fix npe regression in json_value when filtering non-existent paths#14250

Merged
gianm merged 2 commits intoapache:masterfrom
clintropolis:fix-json-value-regression
May 11, 2023
Merged

fix npe regression in json_value when filtering non-existent paths#14250
gianm merged 2 commits intoapache:masterfrom
clintropolis:fix-json-value-regression

Conversation

@clintropolis
Copy link
Copy Markdown
Member

@clintropolis clintropolis commented May 10, 2023

Description

Fixes a regression caused by #14139 when filtering on non-existent paths with JSON_VALUE. The (one of the) added test fails without the fix.

While I was here, I adjusted the nested column group by query test to skip group-by v1 since this stuff practically doesn't work at all except against string paths on historical segments, which feels like it is effectively unusable, so I just removed them because it was tedious to get the flag combination set correctly, and it cuts down the run time quite a bit.


This PR has:

  • been self-reviewed.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

Copy link
Copy Markdown
Contributor

@somu-imply somu-imply left a comment

Choose a reason for hiding this comment

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

LGTM

@gianm gianm merged commit aaaff74 into apache:master May 11, 2023
@clintropolis clintropolis deleted the fix-json-value-regression branch May 11, 2023 05:40
clintropolis added a commit to clintropolis/druid that referenced this pull request May 11, 2023
…pache#14250)

* fix npe regression in json_value when filtering non-existent paths

* more coverage
clintropolis added a commit that referenced this pull request May 11, 2023
…14250) (#14254)

* fix npe regression in json_value when filtering non-existent paths

* more coverage
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.

4 participants