MartijnVisser commented on code in PR #29003:
URL: https://github.com/apache/flink/pull/29003#discussion_r3971824529
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/stream/StreamExecSort.java:
##########
@@ -100,12 +103,25 @@ 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.
+ if (sortSpec.getFieldSize() == 0) {
+ throw new TableException(
+ "Compiled plan contains a streaming sort without sort
keys.");
+ }
+ int firstSortField = sortSpec.getFieldIndices()[0];
+ String column = inputType.getFieldNames().get(firstSortField);
+ LogicalType type = inputType.getTypeAt(firstSortField);
+ if (LogicalTypeChecks.isTimeAttribute(type)
+ && !sortSpec.getFieldSpecs()[0].getIsAscendingOrder()) {
+ throw new TableException(
+
SortUtil.sortKeyTimeAttributeMustBeAscendingMessage(column));
+ }
+ throw new
TableException(SortUtil.sortKeyNotTimeAttributeMessage(column, type));
Review Comment:
The condition is right: a descending time attribute takes the first branch
and gets the "must be sorted in ascending order" message. Verified for `rowtime
DESC`, `proctime DESC` and a TIMESTAMP_LTZ rowtime, both through the rule and
through a compiled plan loaded without the flag;
`testLoadedPlanRejectsDescendingTimeAttributeSort` and the `rowtime DESC, c`
case in `nonTemporalSorts` assert exactly that. The message you quote only
comes out if a plan is hand-edited to put an ascending time attribute into a
`StreamExecSort`, which the planner never emits (that becomes a temporal sort).
Did you see it with a specific query?
--
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]