twalthr commented on code in PR #26389:
URL: https://github.com/apache/flink/pull/26389#discussion_r2035013674
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/AggregateQueryOperation.java:
##########
@@ -77,23 +78,25 @@ public List<ResolvedExpression> getAggregateExpressions() {
}
@Override
- public String asSerializableString() {
- final String groupingExprs = getGroupingExprs();
+ public String asSerializableString(SqlFactory sqlFactory) {
+ final String groupingExprs = getGroupingExprs(sqlFactory);
return String.format(
"SELECT %s FROM (%s\n) %s\nGROUP BY %s",
Stream.concat(groupingExpressions.stream(),
aggregateExpressions.stream())
.map(
expr ->
OperationExpressionsUtils.scopeReferencesWithAlias(
INPUT_ALIAS, expr))
- .map(ResolvedExpression::asSerializableString)
+ .map(
+ resolvedExpression ->
+
resolvedExpression.asSerializableString(sqlFactory))
.collect(Collectors.joining(", ")),
- OperationUtils.indent(child.asSerializableString()),
+ OperationUtils.indent(child.asSerializableString(sqlFactory)),
INPUT_ALIAS,
groupingExprs);
}
- private String getGroupingExprs() {
+ private String getGroupingExprs(SqlFactory context) {
Review Comment:
```suggestion
private String getGroupingExprs(SqlFactory sqlFactory) {
```
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/CatalogPropertiesUtil.java:
##########
@@ -72,11 +73,12 @@ public final class CatalogPropertiesUtil {
public static final String FLINK_PROPERTY_PREFIX = "flink.";
/** Serializes the given {@link ResolvedCatalogTable} into a map of string
properties. */
- public static Map<String, String>
serializeCatalogTable(ResolvedCatalogTable resolvedTable) {
+ public static Map<String, String> serializeCatalogTable(
+ ResolvedCatalogTable resolvedTable, SqlFactory context) {
Review Comment:
```suggestion
ResolvedCatalogTable resolvedTable, SqlFactory sqlFactory) {
```
Maybe perform a `SqlFactory context` search on the code base?
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -561,12 +560,12 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
BuiltInFunctionDefinition.newBuilder()
.name("ifThenElse")
.callSyntax(
- (sqlName, operands) ->
+ (sqlName, operands, context) ->
Review Comment:
```suggestion
(sqlName, operands, sqlFactory) ->
```
in entire class
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/QueryOperation.java:
##########
@@ -44,6 +47,19 @@ public interface QueryOperation extends Operation {
* @see Operation#asSummaryString()
*/
default String asSerializableString() {
+ return asSerializableString(new DefaultSqlFactory());
+ }
+
+ /**
+ * Returns a string that fully serializes this instance. The serialized
string can be used for
Review Comment:
```suggestion
* Returns a SQL string that fully serializes this instance. The
serialized string can be used for
```
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/QueryOperation.java:
##########
@@ -44,6 +47,19 @@ public interface QueryOperation extends Operation {
* @see Operation#asSummaryString()
*/
default String asSerializableString() {
+ return asSerializableString(new DefaultSqlFactory());
+ }
+
+ /**
+ * Returns a string that fully serializes this instance. The serialized
string can be used for
+ * storing the query in e.g. a {@link
org.apache.flink.table.catalog.Catalog} as a view.
+ *
+ * @param sqlFactory can be used to customize the serialization to a SQL
string
+ * @return detailed string for persisting in a catalog
Review Comment:
```suggestion
* @return Flink SQL string for persisting in a catalog
```
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/QueryOperation.java:
##########
@@ -44,6 +47,19 @@ public interface QueryOperation extends Operation {
* @see Operation#asSummaryString()
*/
default String asSerializableString() {
+ return asSerializableString(new DefaultSqlFactory());
Review Comment:
nit: `DefaultSqlFactory.INSTANCE`?
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/WindowAggregateQueryOperation.java:
##########
@@ -262,23 +266,23 @@ public String asSummaryString() {
}
}
- public String asSerializableString(String table) {
+ public String asSerializableString(String table, SqlFactory context) {
Review Comment:
```suggestion
public String asSerializableString(String table, SqlFactory
sqlFactory) {
```
--
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]
