You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "chrisdennis (via GitHub)" <gi...@apache.org> on 2026/06/10 19:07:47 UTC
[PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
chrisdennis opened a new pull request, #5012:
URL: https://github.com/apache/calcite/pull/5012
[CALCITE-7599](https://issues.apache.org/jira/browse/CALCITE-7599)
This is a small refinement to CALCITE-7498 to finish alignment of the reference documentation example, the reference documentation grammar, and the actual parser grammar. The alignment here could have used a different fixed point, but I figured I would follow the original example and align on the example query as the source of truth. This also has the nice side-effect of supporting the largest set of valid hints.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5012:
URL: https://github.com/apache/calcite/pull/5012#discussion_r3399821053
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -9693,7 +9693,12 @@ private static Consumer<List<? extends Throwable>> checkWarnings(
final String sql1 = "select "
+ "/*+ properties(^k1^=123, k2='v2'), no_hash_join() */ "
+ "empno, ename, deptno from emps";
- sql(sql1).fails("(?s).*Encountered \"k1 = 123\" at .*");
+ // Allow numeric literal k/v values.
+ final String expected1 = "SELECT\n"
Review Comment:
Do you want to add a few more tests with the more complex numeric literals supported, e.g. `1e-2`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "chrisdennis (via GitHub)" <gi...@apache.org>.
chrisdennis commented on PR #5012:
URL: https://github.com/apache/calcite/pull/5012#issuecomment-4673701304
In the process of cleaning some test fallout here... right now I don't think this impacts the actual fix however.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "chrisdennis (via GitHub)" <gi...@apache.org>.
chrisdennis commented on code in PR #5012:
URL: https://github.com/apache/calcite/pull/5012#discussion_r3399996833
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -9693,7 +9693,12 @@ private static Consumer<List<? extends Throwable>> checkWarnings(
final String sql1 = "select "
+ "/*+ properties(^k1^=123, k2='v2'), no_hash_join() */ "
+ "empno, ename, deptno from emps";
- sql(sql1).fails("(?s).*Encountered \"k1 = 123\" at .*");
+ // Allow numeric literal k/v values.
+ final String expected1 = "SELECT\n"
Review Comment:
That would make sense, but I'll add them to `SqlHintsConverterTest` rather than here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #5012:
URL: https://github.com/apache/calcite/pull/5012
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5012:
URL: https://github.com/apache/calcite/pull/5012#issuecomment-4673953693
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5012) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5012&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5012&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5012&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5012&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5012&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5012)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #5012:
URL: https://github.com/apache/calcite/pull/5012#issuecomment-4702555280
Let's see if there are other comments before merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5012:
URL: https://github.com/apache/calcite/pull/5012#discussion_r3408670954
##########
core/src/main/java/org/apache/calcite/sql/SqlHint.java:
##########
@@ -197,18 +197,19 @@ public enum HintOptionFormat implements Symbolizable {
* The hint options are list of key-value pairs.
* For each pair,
* the key is a simple identifier or string literal,
- * the value is a string literal.
+ * the value is a string or numeric literal.
*/
KV_LIST
}
//~ Tools ------------------------------------------------------------------
- private static String getOptionAsString(SqlNode node) {
+ private String getOptionAsString(SqlNode node) {
Review Comment:
why remove the static?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "chrisdennis (via GitHub)" <gi...@apache.org>.
chrisdennis commented on code in PR #5012:
URL: https://github.com/apache/calcite/pull/5012#discussion_r3408852049
##########
core/src/main/java/org/apache/calcite/sql/SqlHint.java:
##########
@@ -197,18 +197,19 @@ public enum HintOptionFormat implements Symbolizable {
* The hint options are list of key-value pairs.
* For each pair,
* the key is a simple identifier or string literal,
- * the value is a string literal.
+ * the value is a string or numeric literal.
*/
KV_LIST
}
//~ Tools ------------------------------------------------------------------
- private static String getOptionAsString(SqlNode node) {
+ private String getOptionAsString(SqlNode node) {
Review Comment:
So that we can reference `options` in the `requireNonNull` message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7599] Enhance parser to allow numeric literals as values in k/v hints as suggested by documentation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5012:
URL: https://github.com/apache/calcite/pull/5012#issuecomment-4686506607
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5012) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5012&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5012&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5012&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [75.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5012&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5012&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5012)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org