snuyanzin commented on code in PR #28381:
URL: https://github.com/apache/flink/pull/28381#discussion_r3663344553


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkCalciteSqlValidator.java:
##########
@@ -142,6 +141,74 @@ public FlinkCalciteSqlValidator(
                         
.get(TableConfigOptions.TABLE_COLUMN_EXPANSION_STRATEGY);
     }
 
+    private static SqlValidator.Config enableGroupByOrdinalIfConfigured(
+            SqlValidator.Config config, RelOptCluster relOptCluster) {
+        final boolean enabled =
+                ShortcutUtils.unwrapTableConfig(relOptCluster)
+                        
.get(TableConfigOptions.TABLE_GROUP_BY_ORDINAL_ENABLED);
+        if (!enabled) {
+            return config;
+        }
+        return config.withConformance(
+                new SqlDelegatingConformance(config.conformance()) {
+                    @Override
+                    public boolean isGroupByOrdinal() {
+                        return true;
+                    }

Review Comment:
   i'm not sure it is a right place to change configs
   this change makes config configuration in multiple places which only 
introduces another level of confusion...
   
   why can't we do this in FlinkPlannerImpl?



-- 
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]

Reply via email to