kgyrtkirk commented on code in PR #15711:
URL: https://github.com/apache/druid/pull/15711#discussion_r1474088487


##########
sql/src/main/codegen/includes/common.ftl:
##########
@@ -18,58 +18,51 @@
  */
 
 // Using fully qualified name for Pair class, since Calcite also has a same 
class name being used in the Parser.jj
-org.apache.druid.java.util.common.Pair<Granularity, String> 
PartitionGranularity() :
+SqlNode PartitionGranularity() :
 {
   SqlNode e;
-  Granularity granularity;
-  String unparseString;
+  SqlNode result;
 }
 {
   (
     <HOUR>
     {
-      granularity = Granularities.HOUR;
-      unparseString = "HOUR";
+      result = SqlLiteral.createCharString(DruidSqlParserUtils.HOUR_GRAIN, 
getPos());

Review Comment:
   I wonder if it would be possible to use `SqlLiteral.createSymbol` here 
instead; that could remove the need for the string based matching as well...



##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java:
##########
@@ -133,6 +141,41 @@ public CalcitePlanner(FrameworkConfig config)
     this.context = config.getContext();
     this.connectionConfig = connConfig(context);
     this.typeSystem = config.getTypeSystem();
+
+    // With the catalog, schemas can provide functions.
+    // Add the necessary indirection. The type factory used here
+    // is the Druid one, since the per-query one is not yet available
+    // here. Nor are built-in function associated with per-query types.
+    this.operatorTable = new ChainedSqlOperatorTable(

Review Comment:
   I wonder if this is new functionality - could it be in a separate PR?



##########
sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlUnparseTest.java:
##########
@@ -58,7 +58,7 @@ public void testUnparseReplaceAll() throws ParseException
                             + "OVERWRITE ALL\n"
                             + "SELECT *\n"
                             + "    FROM \"foo\"\n"
-                            + "PARTITIONED BY ALL TIME "
+                            + "PARTITIONED BY 'ALL TIME' "

Review Comment:
   is the result of the unparse valid ?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to