From fdb4d66d3c42ae51cf66ec09cbce654d110b18a8 Mon Sep 17 00:00:00 2001 From: Mihai Budiu Date: Fri, 1 May 2026 18:42:54 -0700 Subject: [PATCH] [CALCITE-7498] The parser rejects the example hints from the documentation Signed-off-by: Mihai Budiu --- core/src/main/codegen/templates/Parser.jj | 11 ++++++++--- .../java/org/apache/calcite/sql/SqlHint.java | 4 ++-- .../calcite/test/SqlHintsConverterTest.java | 16 ++++++++++++++++ .../calcite/test/SqlHintsConverterTest.xml | 16 ++++++++++++++++ site/_docs/reference.md | 4 ++-- .../calcite/sql/parser/SqlParserTest.java | 17 +++++++++++++++++ 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj index 44e54b076dc7..16b80218e59d 100644 --- a/core/src/main/codegen/templates/Parser.jj +++ b/core/src/main/codegen/templates/Parser.jj @@ -1261,9 +1261,14 @@ void AddKeyValueOption(List list) : key = StringLiteral() ) - value = StringLiteral() { - list.add(key); - list.add(value); + ( + value = StringLiteral() + | + value = SimpleIdentifier() + ) + { + list.add(key); + list.add(value); } } diff --git a/core/src/main/java/org/apache/calcite/sql/SqlHint.java b/core/src/main/java/org/apache/calcite/sql/SqlHint.java index 7c26e7fda07c..38f54636ebe8 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlHint.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlHint.java @@ -151,7 +151,7 @@ public Map getOptionKVPairs() { for (int i = 0; i < options.size() - 1; i += 2) { final SqlNode k = options.get(i); final SqlNode v = options.get(i + 1); - attrs.put(getOptionKeyAsString(k), ((SqlLiteral) v).getValueAs(String.class)); + attrs.put(getOptionAsString(k), getOptionAsString(v)); } return ImmutableMap.copyOf(attrs); } else { @@ -204,7 +204,7 @@ public enum HintOptionFormat implements Symbolizable { //~ Tools ------------------------------------------------------------------ - private static String getOptionKeyAsString(SqlNode node) { + private static String getOptionAsString(SqlNode node) { assert node instanceof SqlIdentifier || SqlUtil.isLiteral(node); if (node instanceof SqlIdentifier) { return ((SqlIdentifier) node).getSimple(); diff --git a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java index cff0c2047459..a34b4c2506ae 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java @@ -149,6 +149,16 @@ public final Fixture sql(String sql) { //~ Tests ------------------------------------------------------------------ + /** Test case for [CALCITE-7498] + * The parser rejects the example hints from the documentation. */ + @Test void testDocumentationExample() { + final String sql = "SELECT /*+ hint1, hint2(a='1', b='2') */ *\n" + + "FROM emp /*+ hint3(5, 'x') */\n" + + "JOIN dept /*+ hint4(c=id), hint5 */\n" + + "ON emp.deptno = dept.deptno"; + sql(sql).ok(); + } + @Test void testQueryHint() { final String sql = HintTools.withHint("select /*+ %s */ *\n" + "from emp e1\n" @@ -1064,6 +1074,12 @@ static HintStrategyTable createHintStrategies(HintStrategyTable.Builder builder) .hintStrategy( "preserved_project", HintStrategy.builder( HintPredicates.PROJECT).excludedRules(CoreRules.FILTER_PROJECT_TRANSPOSE).build()) + // meaningless hints for the example in the documentation + .hintStrategy("hint1", HintPredicates.JOIN) + .hintStrategy("hint2", HintPredicates.JOIN) + .hintStrategy("hint3", HintPredicates.TABLE_SCAN) + .hintStrategy("hint4", HintPredicates.TABLE_SCAN) + .hintStrategy("hint5", HintPredicates.TABLE_SCAN) .build(); } diff --git a/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml index 8ac1e96de7c7..e2cd95622d57 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml @@ -55,6 +55,22 @@ from orders, products_temporal for system_time as of orders.rowtime]]> + + + + + + + + diff --git a/site/_docs/reference.md b/site/_docs/reference.md index 5f12b22149d6..1c13f10324dc 100644 --- a/site/_docs/reference.md +++ b/site/_docs/reference.md @@ -3532,11 +3532,11 @@ optionKey: | stringLiteral optionVal: - stringLiteral + simpleIdentifier + | stringLiteral hintOption: simpleIdentifier - | numericLiteral | stringLiteral {% endhighlight %} diff --git a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java index cfea5b6ce0e9..fad96f70b956 100644 --- a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java +++ b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java @@ -9368,6 +9368,23 @@ private static Consumer> checkWarnings( sql(sql2).ok(expected); } + /** Test case for [CALCITE-7498] + * The parser rejects the example hints from the documentation. */ + @Test void testDocumentationExample() { + final String sql = "SELECT /*+ hint1, hint2(a='1', b='2') */ *\n" + + "FROM emp /*+ hint3(5, 'x') */\n" + + "JOIN dept /*+ hint4(c=id), hint5 */\n" + + "ON emp.deptno = dept.deptno"; + final String expected = "SELECT\n" + + "/*+ `HINT1`, `HINT2`(`A` = '1', `B` = '2') */\n" + + "*\n" + + "FROM `EMP`\n" + + "/*+ `HINT3`(5, 'x') */\n" + + "INNER JOIN `DEPT`\n" + + "/*+ `HINT4`(`C` = `ID`), `HINT5` */ ON (`EMP`.`DEPTNO` = `DEPT`.`DEPTNO`)"; + sql(sql).ok(expected); + } + @Test void testQueryHint() { final String sql1 = "select " + "/*+ properties(k1='v1', k2='v2', 'a.b.c'='v3'), "