github-actions[bot] commented on code in PR #68161:
URL: https://github.com/apache/doris/pull/68161#discussion_r4290348439
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonColumnValue.java:
##########
@@ -195,11 +197,45 @@ public LocalDateTime getDateTime() {
@Override
public LocalDateTime getTimeStampTz() {
- Timestamp ts = record.getTimestamp(idx, dorisType.getPrecision());
+ Timestamp ts = truncateTimestampPrecision(
+ record.getTimestamp(idx, getPaimonTimestampPrecision()));
// Timestamp's local representation is identical to converting its
epoch instant in UTC.
return ts.toLocalDateTime();
}
+ Timestamp truncateTimestampPrecision(Timestamp timestamp) {
+ if (!(dataType instanceof TimestampType) && !(dataType instanceof
LocalZonedTimestampType)) {
+ return timestamp;
+ }
+ int precision = dorisType.getPrecision();
Review Comment:
[P1] Retain the logical precision of TIMESTAMP_NS outputs
For historical Parquet `ts TIMESTAMP(9)` narrowed to `TIMESTAMP(7)`,
`2025-01-01 00:00:01.123456789` should become `.123456700` ([SDK narrowing
rule](https://github.com/apache/paimon/blob/release-1.4.2/paimon-common/src/main/java/org/apache/paimon/casting/TimestampToTimestampCastRule.java#L72-L75)).
TIMESTAMP_NS has bridge precision 9. If `SELECT ts, plain_ts` also projects an
unchanged TIMESTAMP(6) column, FE sends repair=true and that sibling activates
the scanner-wide widened read schema. `ts` is read at p9, but
resetDatetimeV2Precision skips TIMESTAMP_NS and this getter truncates at 9,
returning `.123456789`. Selecting `ts` alone still lets the SDK truncate to p7,
so adding a projected column changes its value; equality on the expected p7
value can also lose the row. Preserve each field's current Paimon precision
separately from the widened read type, including TIMESTAMP_NS and nested
leaves, and cover historical p9-to-p7/p8 reads with a DATETIMEV2 sibling.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -198,6 +204,16 @@ protected void openInternal() throws IOException {
}
private void initReader() throws IOException {
+ Split split = getSplit();
+ QueryAuthSplit authSplit = split instanceof QueryAuthSplit ?
(QueryAuthSplit) split : null;
Review Comment:
[P1] Repair policies inside fallback split wrappers
Paimon wraps a fallback policy split as
`FallbackSplitImpl(QueryAuthSplit(...), true)` ([SDK wrapping
path](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/table/FallbackReadFileStoreTable.java#L504-L524)).
FE sends this non-DataSplit to JNI unchanged, so this top-level check leaves
authSplit null and skips PaimonTimestampQueryAuth. With historical p6
`2025-01-01 00:00:01.600000` narrowed to p0 and a typed SDK catalog policy `ts
= '2025-01-01 00:00:01'`, selecting ts widens the read to p9. The SDK unwraps
the fallback split and evaluates the original policy on `.600000` before the
getter, dropping a row the baseline p0 reader permitted. A populated main
partition with query-auth disabled and a separate fallback partition with
query-auth enabled reaches this path. Recursively adapt the inner policy split
while preserving branch dispatch, and cover a historical fallback-policy read.
This wrapper instance is separate from the previously
repaired direct QueryAuthSplit case.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonTimestampQueryAuth.java:
##########
@@ -0,0 +1,182 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.paimon;
+
+import org.apache.paimon.catalog.TableQueryAuthResult;
+import org.apache.paimon.data.GenericArray;
+import org.apache.paimon.data.GenericMap;
+import org.apache.paimon.data.GenericRow;
+import org.apache.paimon.data.InternalArray;
+import org.apache.paimon.data.InternalMap;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.data.Timestamp;
+import org.apache.paimon.predicate.FieldRef;
+import org.apache.paimon.predicate.Predicate;
+import org.apache.paimon.predicate.PredicateVisitor;
+import org.apache.paimon.predicate.Transform;
+import org.apache.paimon.reader.RecordReader;
+import org.apache.paimon.types.ArrayType;
+import org.apache.paimon.types.DataField;
+import org.apache.paimon.types.DataType;
+import org.apache.paimon.types.DataTypeChecks;
+import org.apache.paimon.types.LocalZonedTimestampType;
+import org.apache.paimon.types.MapType;
+import org.apache.paimon.types.RowType;
+import org.apache.paimon.types.TimestampType;
+import org.apache.paimon.utils.InternalRowUtils;
+
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/** Keeps SDK authorization and masking after current-precision
materialization. */
+final class PaimonTimestampQueryAuth extends TableQueryAuthResult {
+ private static final long serialVersionUID = 1L;
+ private final RowType currentReadType;
+ private final TableQueryAuthResult original;
+
+ PaimonTimestampQueryAuth(TableQueryAuthResult original, RowType
currentReadType) {
+ super(original.filter(), original.columnMasking());
+ this.original = original;
+ this.currentReadType = currentReadType;
+ }
+
+ @Override
+ public Predicate extractPredicate() {
+ return original.extractPredicate();
+ }
+
+ @Override
+ public Map<String, Transform> extractColumnMasking() {
+ return original.extractColumnMasking();
+ }
+
+ static void addPolicyFields(TableQueryAuthResult auth, RowType tableType,
+ List<DataField> readFields, List<DataField> currentFields) {
+ List<String> projected =
readFields.stream().map(DataField::name).collect(Collectors.toList());
+ Set<String> required = requiredFields(auth, new
LinkedHashSet<>(projected));
+ for (DataField field : tableType.getFields()) {
+ if (required.contains(field.name())) {
+ int index = projected.indexOf(field.name());
+ DataField widened =
field.newType(PaimonJniScanner.createSafeTimestampReadType(field.type()));
+ if (index < 0) {
+ currentFields.add(field);
+ readFields.add(widened);
+ } else if (field.type() instanceof RowType || field.type()
instanceof ArrayType
+ || field.type() instanceof MapType) {
+ // A policy can read a sibling that SQL pruned from a
projected complex column.
+ currentFields.set(index, field);
+ readFields.set(index, widened);
+ }
+ }
+ }
+ }
+
+ static boolean readsTimestamp(TableQueryAuthResult auth, RowType
tableType, List<String> projected) {
+ Set<String> required = requiredFields(auth, new
LinkedHashSet<>(projected));
+ return tableType.getFields().stream().anyMatch(field ->
required.contains(field.name())
+ && PaimonJniScanner.containsTimestampType(field.type()));
+ }
+
+ private static Set<String> requiredFields(TableQueryAuthResult auth,
Set<String> projected) {
+ Set<String> required = new LinkedHashSet<>();
+ Predicate filter = auth.extractPredicate();
+ if (filter != null) {
+ required.addAll(PredicateVisitor.collectFieldNames(filter));
+ }
+ for (Map.Entry<String, Transform> mask :
auth.extractColumnMasking().entrySet()) {
+ if (!projected.contains(mask.getKey())) {
+ continue;
+ }
+ required.add(mask.getKey());
+ for (Object input : mask.getValue().inputs()) {
+ if (input instanceof FieldRef) {
+ required.add(((FieldRef) input).name());
+ }
+ }
+ }
+ return required;
+ }
+
+ static RecordReader<InternalRow> projectOutput(RecordReader<InternalRow>
reader,
+ RowType currentReadType, RowType outputType) {
+ if (currentReadType.equals(outputType)) {
+ return reader;
+ }
+ return reader.transform(row -> repairRow(row, currentReadType,
outputType));
+ }
+
+ @Override
+ public RecordReader<InternalRow> doAuth(RecordReader<InternalRow> reader,
RowType readType) {
+ // Delegate the policy itself to Paimon; neither filters nor masks are
dropped or rewritten.
+ if (readType.equals(currentReadType)) {
+ return original.doAuth(reader, readType);
+ }
+ return original.doAuth(reader.transform(row -> repairRow(row,
readType, currentReadType)), currentReadType);
Review Comment:
[P1] Match the inner policy schema for audit-log columns
With query-auth enabled, an ordinary integer catalog policy `id = 7`, and
historical TIMESTAMP(6) files narrowed to p0, `SELECT rowkind, ts FROM
t$audit_log` enters this adapter with currentReadType containing `rowkind`,
`ts` and the added `id`. However,
[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-L752)
removes system fields before configuring the inner data reader, and that
reader invokes doAuth before AuditLogRow creates `rowkind`. Its readType
therefore contains only `ts` and `id`. Here repairRow looks up the absent
`rowkind`, gets sourceIndex=-1 and fails at getTypeAt(-1), so the previously
working query cannot return rows. Build the repair target in the inner
callback's data-schema domain, leaving system-field synthesis to the outer
wrapper, and add a historical audit-log reader case selecting rowkind under a
serialized integer policy. The added audit-log test
explicitly skips auth, so it misses this intersection.
--
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]
