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


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -87,7 +87,7 @@ static CatalogView toCatalogView(
                 context.getSqlValidator().getNamespace(validateQuery);
         validateDuplicatedColumnNames(validateQuery, viewFields, 
validatedNamespace);
 
-        String expandedQuery = context.toQuotedSqlString(validateQuery);
+        String expandedQuery = context.expandSqlIdentifiers(originalQuery);

Review Comment:
   it was a workaround because of a bug with `LATERAL` in Calcite and there was 
a huge comment trying to explain why we need it.
   The problem became more serious with MATERIALIZED TABLEs where we might want 
to use expanded query.
   Also if you try to look at other vendors: they also expand star which Flink 
didn't do.
   
   With this issue we see that we are in a lack of tests for `MATCH_RECOGNIZE` 
(looks like Calcite as well) and that's why it was not detected earlier.
   
   for short term we could consider rolling it back, however so far nobody was 
heavily complaining that hot fix is urgently required. In this case I think we 
still have time to fix it in a proper way



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