Gabriel39 commented on code in PR #67207:
URL: https://github.com/apache/doris/pull/67207#discussion_r3870757216
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -1223,12 +1247,18 @@ public boolean
supportsColumnHandleSnapshotPin(ConnectorSession session) {
return true;
}
- private static Map<String, ConnectorColumnHandle>
buildColumnHandles(List<DataField> fields) {
- Map<String, ConnectorColumnHandle> handles = new
LinkedHashMap<>(fields.size());
+ private static Map<String, ConnectorColumnHandle>
buildColumnHandles(List<DataField> fields,
+ boolean appendDataFileMetadataColumns) {
+ Map<String, ConnectorColumnHandle> handles = new
LinkedHashMap<>(fields.size() + 2);
for (int i = 0; i < fields.size(); i++) {
String name = fields.get(i).name();
handles.put(name, new PaimonColumnHandle(name, i));
}
+ if (appendDataFileMetadataColumns) {
+ handles.put(PAIMON_FILE_PATH_COL, new
PaimonColumnHandle(PAIMON_FILE_PATH_COL, -1));
Review Comment:
[P1] Please preserve valid physical Paimon columns with this name. Paimon
1.3.1 does not reserve `__paimon_file_path` or `__paimon_row_index`, so an
external table created by Flink/Spark may legitimately contain either column.
`buildTableSchema` then appends a duplicate hidden column, and these `put`
calls overwrite the physical handle with the synthetic `-1` handle; BE also
gives the virtual-name mapping precedence over physical fields. Selecting such
an existing user column will therefore return the RawFile path/row position
instead of the stored value. The CREATE check only protects tables created
through Doris. Please distinguish metadata handles from physical field handles
end to end (or otherwise fail safely on collision), and add coverage for an
externally-created Paimon table containing these names.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -686,6 +694,7 @@ private List<ConnectorScanRange> planScanInternal(
Optional<ConnectorExpression> filter,
boolean countPushdown) {
IcebergTableHandle iceHandle = (IcebergTableHandle) handle;
+ validateMetadataColumnReader(session, columns);
Review Comment:
[P2] This validation is only reached by the eager `planScanInternal` path.
Large Iceberg scans selected by `streamingSplitEstimate` call `streamSplits`
directly, and that entry does not invoke `validateMetadataColumnReader(session,
columns)`. Consequently, a metadata-column query with
`enable_file_scanner_v2=false` or `force_jni_scanner=true` is rejected for a
small/eager scan but proceeds to the unsupported V1/JNI reader for a
large/streaming scan, where `_file`/`_pos` cannot be materialized. Please reuse
this validation at the start of `streamSplits` as well (before its
empty-snapshot return, for parity), and add streaming-path coverage.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
