github-actions[bot] commented on code in PR #68161:
URL: https://github.com/apache/doris/pull/68161#discussion_r4306501388
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -2087,27 +2254,61 @@ static FileStoreTable
legacyOrcSplitTable(FileStoreTable table, DataSplit split)
return base;
}
- static boolean requiresLegacyOrcTimestampReader(Table table,
Optional<List<RawFile>> rawFiles,
+ private static boolean requiresCrossFamilyTimestampReader(FileStoreTable
table, Optional<List<RawFile>> rawFiles,
+ Set<Integer> readFieldIds, Map<Long, Boolean> schemaTimestamps) {
+ RowType readType = new RowType(table.rowType().getFields().stream()
+ .filter(field ->
readFieldIds.contains(field.id())).collect(Collectors.toList()));
+ if (!containsTimestampType(readType)) {
+ return false;
+ }
+ for (RawFile file : rawFiles.get()) {
+ if (schemaTimestamps.computeIfAbsent(file.schemaId(), id ->
hasTimestampPrecisionNarrowing(
+ table.schemaManager().schema(id).logicalRowType(),
readType, 0, true))) {
+ // Generic native casts round after zone conversion. The SDK
keeps its conversion
+ // semantics while the JNI widened read type delays truncation
until materialization.
+ return true;
+ }
+ }
+ return false;
+ }
+
+ static boolean requiresOrcTimestampReader(Table table,
Optional<List<RawFile>> rawFiles,
Set<Integer> readFieldIds, Map<Long, Boolean> schemaTimestamps) {
if (readFieldIds.isEmpty() || !rawFiles.isPresent()
- || rawFiles.get().stream().noneMatch(f ->
f.path().endsWith(".orc"))
- || !new org.apache.paimon.options.Options(table.options()).get(
-
org.apache.paimon.format.OrcOptions.ORC_TIMESTAMP_LTZ_LEGACY_TYPE)) {
+ || rawFiles.get().stream().noneMatch(f ->
f.path().endsWith(".orc"))) {
return false;
}
// Only fields present in a file require SDK timezone conversion. A
newly added nullable LTZ
// column reads as NULL in older files and must not disable their
native metadata scans.
FileStoreTable fileStoreTable = (FileStoreTable) table;
+ boolean legacyLtz = new
org.apache.paimon.options.Options(table.options()).get(
+
org.apache.paimon.format.OrcOptions.ORC_TIMESTAMP_LTZ_LEGACY_TYPE);
+ RowType readType = new
RowType(fileStoreTable.rowType().getFields().stream()
+ .filter(field ->
readFieldIds.contains(field.id())).collect(Collectors.toList()));
Review Comment:
[P1] Translate read-field IDs into the selected fallback branch
`readFieldIds` comes from the outer table, but `fileStoreTable` is the
branch selected for this split. Paimon accepts fallback schemas with matching
positional types and different field IDs. For example, start with `(pt#0, ts#1
TIMESTAMP(9))`; fork, then drop/re-add `ts` as #2/p0 on main while fallback
retains #1 and narrows p9 to p0. `SELECT ts` supplies `{2}`, so this filter
removes fallback `ts#1` and both timestamp-routing helpers miss its historical
narrowing. The ORC file stays native and `00:00:01.999999500` becomes
`00:00:02`, rather than `00:00:01`. This can reach materialization without a
missing-column failure: write fallback's p9 file under schema ID 2 after two
option-only changes, matching main schema 2's same-name re-added field; the
native history annotator assigns main's ID by name while retaining physical
precision. Translate the read selection to each branch's ID domain before
checking history, and cover independently evolved branch IDs with automatic
routing. T
his is a separate field-identity gap in the new FE routing guard, beyond the
existing ordinary ORC precision issue.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1241,15 +1251,27 @@ public Map<String, String> getScanNodeProperties(
// ("encodedStr is null"). Mirrors legacy
PaimonScanNode.createScanRangeLocations, which always called
// setPaimonPredicate(encodeObjectToString(predicates)) regardless of
whether predicates was empty.
List<org.apache.paimon.predicate.Predicate> predicates =
Collections.emptyList();
+ // The same predicate must stay out of the BE Paimon reader: JNI
materializes and repairs the
+ // value first, then the generic scan path evaluates the residual
against the repaired column.
if (filter.isPresent()) {
RowType rowType = table.rowType();
- PaimonPredicateConverter converter = new
PaimonPredicateConverter(rowType);
+ PaimonPredicateConverter converter = new PaimonPredicateConverter(
+ rowType, filterReadsTimestamp(rowType, filter.get())
+ && requiresTimestampPrecisionRepair(session,
table, paimonHandle));
predicates = converter.convert(filter.get());
}
props.put("paimon.predicate", encodeObjectToString(predicates));
// Paimon JDBC metastore options for BE (if applicable)
Map<String, String> backendOptions = new
LinkedHashMap<>(getBackendPaimonOptions());
+ Set<Integer> readFieldIds = scanReadFieldIds(table.rowType(), columns,
filter);
+ boolean readsTimestamp =
table.rowType().getFields().stream().anyMatch(field ->
+ readFieldIds.contains(field.id())
+ && containsTimestampType(field.type()));
+ backendOptions.put(DORIS_TIMESTAMP_PRECISION_REPAIR, String.valueOf(
+ (readsTimestamp || new
org.apache.paimon.options.Options(table.options()).get(
Review Comment:
[P2] Check query-auth enablement on both fallback branches
`table.options()` exposes the wrapped branch's options, while the other
branch can independently enable query auth. With wrapped auth disabled,
fallback auth enabled, and `SELECT id` with no SQL timestamp reference, this
emits `doris.timestamp-precision-repair=false` even when a fallback policy
reads an evolved timestamp. JNI then refuses to activate its hidden-policy
repair. For a supported typed SDK policy comparing historical p6
`00:00:01.000001`, narrowed to p4, with `00:00:01.0000`, Paimon's ordinary
precision cast leaves the fraction and the policy rejects the matching logical
row. The existing hidden-policy reader test manually passes `true`, and the FE
test enables auth on the whole pair. Inspect branch-local auth settings, or let
the actual split-carried policy activate repair, and cover integer-only output
when auth is enabled exclusively on the secondary branch. This is a missed
repair path in the new gate; remote JSON timestamp-policy end-to-end behavior
has not been v
alidated.
--
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]
