Skip to content

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Jun 16, 2022

Background

Fixes REGEX OPTION can be parsed anywhere in the SQL including within a STRING literal.
See: #8906 for more details.

Details

  1. only allow single OPTION keyword
  2. only allow OPTION at end of file

Release Note

This PR introduces a backward incompatible change to OPTION keyword parsing. Multiple OPTION keyword is no longer supported.
For SQL such as SELECT * FROM tbl OPTION(k1=v1) OPTION(k2=v2) please use
SELECT * FROM tbl OPTION(k1=v1,k2=v2)

1. only allow single OPTION keyword
2. only allow OPTION at end of file
@walterddr walterddr mentioned this pull request Jun 16, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2022

Codecov Report

Merging #8905 (5090b28) into master (00ec121) will decrease coverage by 6.71%.
The diff coverage is 100.00%.

❗ Current head 5090b28 differs from pull request most recent head 776829b. Consider uploading reports for the commit 776829b to get more accurate results

@@             Coverage Diff              @@
##             master    #8905      +/-   ##
============================================
- Coverage     69.74%   63.03%   -6.72%     
+ Complexity     4681     4662      -19     
============================================
  Files          1808     1761      -47     
  Lines         94276    92215    -2061     
  Branches      14057    13825     -232     
============================================
- Hits          65756    58129    -7627     
- Misses        23956    29916    +5960     
+ Partials       4564     4170     -394     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 66.39% <100.00%> (+<0.01%) ⬆️
unittests2 14.89% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...org/apache/pinot/sql/parsers/CalciteSqlParser.java 86.21% <100.00%> (-1.76%) ⬇️
...va/org/apache/pinot/core/routing/RoutingTable.java 0.00% <0.00%> (-100.00%) ⬇️
...va/org/apache/pinot/common/config/NettyConfig.java 0.00% <0.00%> (-100.00%) ⬇️
...a/org/apache/pinot/common/metrics/MinionMeter.java 0.00% <0.00%> (-100.00%) ⬇️
...g/apache/pinot/common/metrics/ControllerMeter.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/BrokerQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/MinionQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
...apache/pinot/common/helix/ExtraInstanceConfig.java 0.00% <0.00%> (-100.00%) ⬇️
...ache/pinot/server/access/AccessControlFactory.java 0.00% <0.00%> (-100.00%) ⬇️
...he/pinot/common/messages/SegmentReloadMessage.java 0.00% <0.00%> (-100.00%) ⬇️
... and 405 more

Continue to review full report at Codecov.

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

@walterddr walterddr marked this pull request as ready for review June 16, 2022 16:09
@Jackie-Jiang Jackie-Jiang added release-notes Referenced by PRs that need attention when compiling the next release notes backward-incompat Referenced by PRs that introduce or fix backward compat issues bugfix labels Jun 16, 2022
@walterddr walterddr merged commit 080e849 into apache:master Jun 16, 2022
@walterddr walterddr deleted the fix_query_option_regex branch December 6, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backward-incompat Referenced by PRs that introduce or fix backward compat issues bugfix release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants