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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
