twalthr commented on a change in pull request #17662: URL: https://github.com/apache/flink/pull/17662#discussion_r742953576
########## File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/connector/source/abilities/SupportsProjectionPushDown.java ########## @@ -49,6 +52,15 @@ /** Returns whether this source supports nested projection. */ boolean supportsNestedProjection(); + /** @deprecated You should implement {@link #applyProjection(int[][], DataType)} */ Review comment: nit: `You should` -> `Please`. Usually, we don't use "you" in code. ########## File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/connector/source/abilities/SupportsReadingMetadata.java ########## @@ -123,7 +123,9 @@ * <p>Note: Use the passed data type instead of {@link ResolvedSchema#toPhysicalRowDataType()} * for describing the final output data type when creating {@link TypeInformation}. If the * source implements {@link SupportsProjectionPushDown}, the projection is already considered in - * the given output data type. + * the given output data type, so you should reuse the {@code producedDataType} provided by this Review comment: nit: `, so you should reuse` -> `. Use` ########## File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/connector/source/abilities/SupportsProjectionPushDown.java ########## @@ -52,6 +52,15 @@ /** Returns whether this source supports nested projection. */ boolean supportsNestedProjection(); + /** @deprecated You should implement {@link #applyProjection(int[][], DataType)} */ + @Deprecated + default void applyProjection(int[][] projectedFields) { + throw new IllegalStateException( Review comment: rather `UnsupportedOperationException`? ########## File path: flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/logical/PushProjectIntoTableSourceScanRule.java ########## @@ -300,7 +300,15 @@ private RowType performPushDown( .getLogicalType(); if (supportsProjectionPushDown(source.tableSource())) { - abilitySpecs.add(new ProjectPushDownSpec(physicalProjections, newProducedType)); + abilitySpecs.add( + new ProjectPushDownSpec( + physicalProjections, + (RowType) + DataTypeUtils.projectRow( Review comment: move `newProducedType` after this `if` branch and introduce a local variable with a helpful name e.g. `RowType projectedPhysicalType` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org