Skip to content

Handle custom string not equal case for Pinot #6266

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

Merged
merged 10 commits into from
Sep 9, 2024

Conversation

bowenxia
Copy link
Member

@bowenxia bowenxia commented Sep 5, 2024

What changed?

  • Handle custom string not equal case for Pinot in pinot_query_validator
  • Deleted redundant parenthesis for and clause
  • Updated test cases accordingly

Why?

  • It is a bug. Previously we didn't catch this edge case.
    For example, when search for --query "CustomStringField != 'xxx'", it had the same result as "CustomStringField = 'xxx'"
  • To make it the same behavior as ES

How did you test it?
Unit test, manually test in QueryBuilder

Potential risks
no

Release notes

Documentation Changes

@@ -453,21 +453,22 @@ func processCustomKeyword(operator string, colNameStr string, colValStr string)
colNameStr, operator, colValStr, colNameStr, operator, colValStr)
}

func processCustomString(comparisonExpr *sqlparser.ComparisonExpr, colNameStr string, colValStr string) string {
// change to like statement for partial match
comparisonExpr.Operator = sqlparser.LikeStr
Copy link
Member Author

Choose a reason for hiding this comment

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

This part was a legacy code from the first version of pinot_query_validator. Forgot to delete previously.

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.08%. Comparing base (0f66adc) to head (b23ecf9).

Additional details and impacted files
Files with missing lines Coverage Δ
common/pinot/pinotQueryValidator.go 87.70% <100.00%> (ø)

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f66adc...b23ecf9. Read the comment docs.

@davidporter-id-au
Copy link
Member

@bowenxia it's not super clear to me what the bug actually was (sorry, I may have not had enough context or missed something). Do you mind adding an example of a query that was failing ?

@bowenxia bowenxia requested a review from shijiesheng September 9, 2024 17:02
@bowenxia bowenxia merged commit 2d0b891 into master Sep 9, 2024
23 checks passed
@bowenxia bowenxia deleted the xbowen_pinot_handle_custom_string_not_equal branch September 9, 2024 22:33
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.

3 participants