github-actions[bot] commented on code in PR #68161:
URL: https://github.com/apache/doris/pull/68161#discussion_r4269229531
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -381,21 +421,37 @@ private Split getSplit() {
}
private void resetDatetimeV2Precision() {
+ boolean repairHistoricalPrecision = Boolean.parseBoolean(
+ params.getOrDefault(PAIMON_OPTION_PREFIX +
DORIS_TIMESTAMP_PRECISION_REPAIR, "true"));
Review Comment:
[P1] Filter legacy FE predicates before enabling default repair
An older FE emits no repair option and still sends timestamp predicates.
During an old-FE/new-BE upgrade, this default requests p9 timestamps but
`initReader` trusts the old predicate list. A historical p6 `2025-01-01
00:00:01.600000` evolved to p0 previously matched `WHERE ts = '2025-01-01
00:00:01'`: the old reader narrowed it before filtering. The new reader
preserves `.600000`, then [Paimon's
executeFilter](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/table/source/AbstractDataTableRead.java#L142-L159)
compares it directly with the whole-second literal and discards it before the
getter repairs it. Evolved timestamp metadata/raw filters are withheld by the
SDK, so the split survives. Unlike the existing same-version leading-zero
threads, this positive p6-to-p0 query worked at base and fails on the newly
added missing-option branch. Conservatively remove timestamp-dependent
predicates for older producers, or repair before filteri
ng, and add an omitted-option historical-reader test.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1289,6 +1306,130 @@ public Map<String, String> getScanNodeProperties(
return props;
}
+ static boolean containsTimestampType(DataType type) {
+ if (type instanceof org.apache.paimon.types.TimestampType
+ || type instanceof
org.apache.paimon.types.LocalZonedTimestampType) {
+ return true;
+ }
+ if (type instanceof org.apache.paimon.types.RowType) {
+ return ((org.apache.paimon.types.RowType)
type).getFields().stream()
+ .anyMatch(field -> containsTimestampType(field.type()));
+ }
+ if (type instanceof org.apache.paimon.types.ArrayType) {
+ return containsTimestampType(((org.apache.paimon.types.ArrayType)
type).getElementType());
+ }
+ if (type instanceof org.apache.paimon.types.MapType) {
+ org.apache.paimon.types.MapType mapType =
(org.apache.paimon.types.MapType) type;
+ return containsTimestampType(mapType.getKeyType()) ||
containsTimestampType(mapType.getValueType());
+ }
+ return false;
+ }
+
+ static boolean filterReadsTimestamp(RowType rowType, ConnectorExpression
filter) {
+ Set<String> names = new HashSet<>();
+ collectFilterColumnNames(filter, names);
+ return rowType.getFields().stream().anyMatch(field ->
+ names.contains(field.name().toLowerCase(Locale.ROOT)) &&
containsTimestampType(field.type()));
+ }
+
+ boolean requiresTimestampPrecisionRepair(ConnectorSession session, Table
table, PaimonTableHandle handle) {
+ if (!containsTimestampType(table.rowType())) {
+ return false;
+ }
+ Supplier<Boolean> lookup = () -> withSchemaHistoryAuthentication(
+ () ->
hasTimestampPrecisionHistory(resolveSchemaDictTable(table, handle)));
Review Comment:
[P1] Read timestamp history for row-backed system tables
This delegates repair detection to `resolveSchemaDictTable`, which accepts
only `FileStoreTable` and `ReadOptimizedTable`. Supported `$audit_log`,
`$binlog` and `$row_tracking` wrappers therefore resolve to null and history
returns false. A current-FE/current-BE `$audit_log` projection of historical
Parquet `TIMESTAMP(6)` value `1969-12-31 23:59:59.600000`, evolved to p0,
explicitly sends repair=false. JNI requests current p0, which
[AuditLogRead.withReadType](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/table/system/AuditLogTable.java#L726-L748)
forwards to the inner reader; the SDK narrows -400ms to the epoch before the
getter can restore the expected `1969-12-31 23:59:59`. Ordinary tables and
`$ro` repair the earlier threads, but this separate wrapper/history-gate
instance remains. Resolve each row-backed wrapper's pinned underlying
file-store history for repair decisions and add historical `$audit_log`
projection coverage.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -224,11 +227,11 @@ private void initReader() throws IOException {
: null;
variantProjections.add(variantProjection);
if (variantProjection == null) {
- // The engine may have pruned this complex column down to the
sub-fields the query touches.
- // Mirroring that shape with paimon's own types lets
withReadType push the same projection
- // through the ROW/ARRAY/MAP readers instead of reading the
whole column and discarding it.
DataType projectedType =
PaimonReadTypeProjection.project(tableField.type(),
types[outputIndex]);
+ if (requiresDatetimeV2PrecisionRepair) {
Review Comment:
[P1] Retain widened types when the SDK adds policy columns
With `query-auth.enabled=true` and a catalog row filter `id = 7`, `SELECT
ts` omits `id` from `readFields`. FE routes the resulting `QueryAuthSplit` to
JNI with repair enabled. However, [Paimon's
authedReader](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/table/source/AbstractDataTableRead.java#L112-L140)
adds the missing column by replacing the read schema with
`tableType.project(ts,id)`, restoring current p0 for `ts` before
`reader(split)`. A historical p6 `1969-12-31 23:59:59.600000` is then narrowed
from -400ms to the epoch; the permitted row reaches the getter with an aligned
value it cannot recover. The ordinary pre-epoch thread is fixed, but this later
schema-augmentation branch defeats that repair even with current FE and no SQL
timestamp predicate. Preserve widened requested types while adding SDK-required
fields and cover a historical `QueryAuthSplit` whose policy references an
unprojected integer column.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -224,11 +227,11 @@ private void initReader() throws IOException {
: null;
variantProjections.add(variantProjection);
if (variantProjection == null) {
- // The engine may have pruned this complex column down to the
sub-fields the query touches.
- // Mirroring that shape with paimon's own types lets
withReadType push the same projection
- // through the ROW/ARRAY/MAP readers instead of reading the
whole column and discarding it.
DataType projectedType =
PaimonReadTypeProjection.project(tableField.type(),
types[outputIndex]);
+ if (requiresDatetimeV2PrecisionRepair) {
+ projectedType = createSafeTimestampReadType(projectedType);
Review Comment:
[P1] Evaluate timestamp policy filters after precision repair
A current FE can send a `QueryAuthSplit` containing a catalog timestamp
policy while the SQL predicate list is empty. With `query-auth.enabled=true`, a
historical p6 `2025-01-01 00:00:01.600000` evolved to p0 and policy `ts =
'2025-01-01 00:00:01'`, `SELECT ts` requests p9 here. The policy column is
already projected, so the SDK keeps p9 and
[TableQueryAuthResult.doAuth](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/catalog/TableQueryAuthResult.java#L129-L135)
tests `.600000` against the whole-second literal before the getter truncates
it; a row that passed the baseline p0 reader is excluded. FE-approved
`getPredicates()` does not govern this split-carried policy. This differs from
the hidden-column case: the widened type survives, and preserving it alone does
not fix the filtering. Evaluate the required policy against
repaired/current-precision values and add a historical-reader regression with
the policy on the projected timestam
p.
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -60,6 +60,16 @@ namespace doris::format {
namespace {
+bool requires_paimon_datetime_precision_conversion(const DataTypePtr&
source_type,
+ const DataTypePtr&
target_type) {
+ const auto source = remove_nullable(source_type);
+ const auto target = remove_nullable(target_type);
+ const auto primitive = source->get_primitive_type();
+ return primitive == target->get_primitive_type() &&
+ (primitive == TYPE_DATETIMEV2 || primitive == TYPE_TIMESTAMPTZ) &&
Review Comment:
[P1] Truncate native timestamp casts across families
The same-primitive guard skips supported `TIMESTAMP(6) -> TIMESTAMP_LTZ(0)`
evolution and the reverse. With V2 enabled and `force_jni_scanner=false`,
historical raw Parquet remains native: history annotation preserves
`DATETIMEV2(6)` while the target is `TIMESTAMPTZ(0)`, so this flag stays false.
The generic cast calls `TimestampTzValue::from_datetime/to_datetime`, which
rounds `2025-01-01 00:00:01.600000` to `00:00:02` in UTC instead of truncating
to `00:00:01`; equality on the expected second misses the row. The new
cross-family regressions force JNI and miss this route. This is distinct from
[the fixed FE history
guard](https://github.com/apache/doris/pull/68161#discussion_r4256366486).
Preserve zone conversion while truncating these native casts, or route affected
files to JNI, and cover both directions without forced JNI.
--
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]
