morningman commented on code in PR #69571:
URL: https://github.com/apache/doris/pull/69571#discussion_r4369185240
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -2292,6 +2301,108 @@ static FileStoreTable
legacyOrcSplitTable(FileStoreTable table, DataSplit split)
return base;
}
+ /** Positions in the partition row of the partition columns that hold
binary values. */
+ private static int[] binaryPartitionFields(Table table) {
+ RowType partitionType = table.rowType().project(table.partitionKeys());
+ return java.util.stream.IntStream.range(0,
partitionType.getFieldCount()).filter(field -> {
+ DataTypeRoot typeRoot =
partitionType.getTypeAt(field).getTypeRoot();
+ return typeRoot == DataTypeRoot.BINARY || typeRoot ==
DataTypeRoot.VARBINARY;
+ }).toArray();
+ }
+
+ /**
+ * Paimon finds the files of a partition under the directory its values
name. With {@code partition.legacy-name}
+ * = true, the default, it names a partition after the string of each
value, which for a non-NULL binary value
+ * is the Java array object ({@code [B@1b6d3586}) rather than its bytes,
and every resolution of the name
+ * differs. A file written under such a name, or looked up under one, is
never found: Paimon's own read fails,
+ * while the BE skips a missing file of an external table and would
silently drop the partition's rows. A file
+ * with an external path is found by the path its metadata records.
Without a schema dictionary (system tables
+ * other than {@code $ro}, which the JNI reader reads with Paimon's own
reader) the schemas of the files are not
+ * consulted.
+ *
+ * <p>The schema a file refers to does not prove how the file was named,
because dynamic options
+ * ({@code spark.paimon.*}, Flink hints, {@code Table.copy}) keep the
schema ID: a file whose schema uses legacy
+ * names fails only if it is also missing at its planned path. With {@code
index-file-in-data-file-dir} = true, a
+ * deletion vector written without an external data path is looked up
under the directory of its partition too,
+ * whatever the path of its data file, and the BE skips the whole data
file when it cannot open the deletion
+ * vector, so every deletion vector must exist. Each path is checked once
per scan.
+ */
+ private void checkBinaryPartitionFiles(PaimonTableHandle handle, Table
table, DataSplit split,
+ int[] binaryPartitionFields, Supplier<Table> schemaTable,
+ Map<FileStoreTable, Map<Long, Boolean>>
legacyPartitionNameSchemas, Map<String, Boolean> fileExistence) {
+ BinaryRow partition = split.partition();
+ OptionalInt nonNullField = Arrays.stream(binaryPartitionFields)
+ .filter(field -> !partition.isNullAt(field)).findFirst();
+ if (!nonNullField.isPresent()) {
+ return;
+ }
+ int binaryField = nonNullField.getAsInt();
+ FileStoreTable splitTable = schemaTable.get() == null
+ ? null : legacyOrcSplitTable((FileStoreTable)
schemaTable.get(), split);
+ Table optionsTable = splitTable == null ? table : splitTable;
Review Comment:
Confirmed, and fixed in `cf79a1ac228`. `resolveSchemaDictTable` has no
answer for `$audit_log`, since an `AuditLogTable` is a `DataTable` rather than
a `FileStoreTable`. The check then fell back to the wrapper's own options, and
`AuditLogTable.options()` returns those of the fallback pair, which are the
main branch's. The splits still come from `FallbackReadScan`, which plans a
partition that is missing on main from the fallback branch and reads it with
that branch's read. I replayed your history with the Paimon 2.1.0 API. Main
keeps the default `partition.legacy-name = true` and holds only a NULL
partition, and `backup` has `false` and a row in `key = 0x4142`. `$audit_log`
reports `true`, while the fallback split's file is at `key=AB/bucket-0/…`,
exists, and Paimon's own read returns the row. The check rejected that scan
although master reads it through JNI.
For the system tables read through JNI, the check now resolves the table
that produced the splits from the wrapper's pinned source, the same source the
timestamp-history checks already use (`resolveTimestampHistoryTable`). It then
picks each split's branch as it does for data tables, so a fallback split of
`$audit_log` or `$row_tracking` is judged by the naming and schemas of its own
branch. In the reverse history (main `false`, `backup` `true`) the scan now
fails at planning with the legacy-name error. Before, it planned and Paimon's
reader failed at execution with a `FileNotFoundException`. `$binlog` is not
affected: it exposes its partition columns as arrays, so it has no binary
partition column to check. `fallbackBinaryPartitionsUseTheNamingOfTheirBranch`
covers both directions for the data table and `$audit_log`, and reads the
planned `$audit_log` splits back through the wrapper.
While there: a `scan.file-creation-time-millis` read plans a fallback table
through the pair's `SnapshotReader`, which returns plain `DataSplit`s of the
wrapped branch. `legacyOrcSplitTable` cast every split to `FallbackSplit`, so
such a scan of a table with a non-NULL binary partition failed with a
`ClassCastException`. A split that is not a `FallbackSplit` now maps to the
wrapped branch. The same cast on the native timestamp path predates this PR and
is fixed with it.
`fileCreationTimeScanOfFallbackTableChecksBinaryPartitionsOfTheMainBranch`
plans such a read.
--
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]
