MartijnVisser commented on code in PR #29003:
URL: https://github.com/apache/flink/pull/29003#discussion_r3912320510
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/utils/SortUtil.scala:
##########
@@ -74,6 +74,21 @@ object SortUtil {
rowType.getFieldList.get(idx)
}
+ /** Error message when the primary streaming sort key is not a time
attribute. */
+ def sortKeyNotTimeAttributeMessage(column: String, tpe: LogicalType): String
=
Review Comment:
Good catch, it resolved to Calcite's `EXPR$n` alias (and `$fn` via the Table
API). For those the message now says "the sort key expression is BIGINT"; added
SQL and Table API tests.
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/stream/StreamExecSort.java:
##########
@@ -100,12 +103,21 @@ public StreamExecSort(
@Override
protected Transformation<RowData> translateToPlanInternal(
PlannerBase planner, ExecNodeConfig config) {
+ ExecEdge inputEdge = getInputEdges().get(0);
+ RowType inputType = (RowType) inputEdge.getOutputType();
if
(!config.get(InternalConfigOptions.TABLE_EXEC_NON_TEMPORAL_SORT_ENABLED)) {
- throw new TableException("Sort on a non-time-attribute field is
not supported.");
+ // Backstop for compiled plans loaded without passing through
StreamPhysicalSortRule.
+ int firstSortField = sortSpec.getFieldIndices()[0];
Review Comment:
Added compile-with-flag / load-without-flag round-trips for both messages,
plus the empty sort-key guard. The round-trip also showed the backstop's type
check missed a TIMESTAMP_LTZ rowtime; fixed with
`LogicalTypeChecks.isTimeAttribute`.
--
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]
