github-actions[bot] commented on code in PR #68161:
URL: https://github.com/apache/doris/pull/68161#discussion_r4078697454
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -192,11 +196,52 @@ private void initReader() throws IOException {
fields.length, paimonAllFieldNames.size()));
}
int[] projected = getProjected();
+ RowType readType = requiresDatetimeV2PrecisionRepair
+ ? createSafeTimestampReadType(table.rowType()) :
table.rowType();
readBuilder.withProjection(projected);
- readBuilder.withFilter(getPredicates());
+ if (requiresDatetimeV2PrecisionRepair) {
+ // withProjection() derives a read type from the table schema and
would otherwise
+ // replace the widened timestamp types with the evolved
(lower-precision) schema.
+ readBuilder.withReadType(readType.project(projected));
+ }
+ readBuilder.withFilter(requiresDatetimeV2PrecisionRepair
+ ? Collections.emptyList() : getPredicates());
Review Comment:
[P1] Withhold affected predicates during FE split planning too
This suppression happens only when BE creates the reader, after FE's
`PaimonScanPlanProvider` has already sent the same predicate to
`newScan().plan().splits()`. For a historical p6 file under a current p4
schema, Paimon 1.3.1 evolves manifest min/max through the same timestamp cast;
a `.000001` value remains fractional because `DateTimeUtils.truncate` uses the
unpadded nano integer string, while the current equality literal is `.0000`.
The stats test can then drop the split, so neither this empty BE filter nor the
Doris residual can recover the row. Please keep affected predicates out of FE
split/file-index pruning too, and add a p6-to-p4 leading-zero regression with a
single-value file.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -325,14 +370,18 @@ private Split getSplit() {
private void resetDatetimeV2Precision() {
for (int i = 0; i < types.length; i++) {
- if (types[i].isDateTimeV2()) {
+ if (types[i].isDateTimeV2() || types[i].getType() ==
ColumnType.Type.TIMESTAMPTZ) {
Review Comment:
[P1] Activate repair for timestamp descendants of complex columns
This guard only inspects the projected root `ColumnType`. For a projected
ROW/ARRAY/MAP whose child evolved from TIMESTAMP(6) to TIMESTAMP(0), the root
is not DATETIMEV2/TIMESTAMPTZ, so `requiresDatetimeV2PrecisionRepair` stays
false and `initReader()` never installs the recursively widened read type.
Paimon can then narrow the historical child before `PaimonColumnValue` sees it;
for example, a pre-epoch `.600000` child can already have crossed the second
boundary and cannot be recovered by the getter. This is distinct from the
existing scalar-timestamp threads because no top-level timestamp is projected.
Please detect timestamp descendants before building the reader and add a nested
historical-file JNI regression.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -192,11 +196,52 @@ private void initReader() throws IOException {
fields.length, paimonAllFieldNames.size()));
}
int[] projected = getProjected();
+ RowType readType = requiresDatetimeV2PrecisionRepair
+ ? createSafeTimestampReadType(table.rowType()) :
table.rowType();
readBuilder.withProjection(projected);
- readBuilder.withFilter(getPredicates());
+ if (requiresDatetimeV2PrecisionRepair) {
+ // withProjection() derives a read type from the table schema and
would otherwise
+ // replace the widened timestamp types with the evolved
(lower-precision) schema.
+ readBuilder.withReadType(readType.project(projected));
+ }
+ readBuilder.withFilter(requiresDatetimeV2PrecisionRepair
+ ? Collections.emptyList() : getPredicates());
reader =
newReadWithOptionalIOManager(readBuilder).executeFilter().createReader(getSplit());
paimonDataTypeList =
- Arrays.stream(projected).mapToObj(i ->
table.rowType().getTypeAt(i)).collect(Collectors.toList());
+ Arrays.stream(projected).mapToObj(i ->
readType.getTypeAt(i)).collect(Collectors.toList());
+ }
+
+ static RowType createSafeTimestampReadType(RowType tableType) {
+ List<DataField> fields = tableType.getFields().stream()
+ .map(field ->
field.newType(createSafeTimestampReadType(field.type())))
+ .collect(Collectors.toList());
+ return tableType.copy(fields);
+ }
+
+ private static DataType createSafeTimestampReadType(DataType dataType) {
+ if (dataType instanceof TimestampType) {
+ return new TimestampType(dataType.isNullable(),
TimestampType.MAX_PRECISION);
+ }
+ if (dataType instanceof LocalZonedTimestampType) {
+ return new LocalZonedTimestampType(dataType.isNullable(),
LocalZonedTimestampType.MAX_PRECISION);
+ }
+ if (dataType instanceof RowType) {
+ RowType rowType = (RowType) dataType;
+ List<DataField> fields = rowType.getFields().stream()
+ .map(field ->
field.newType(createSafeTimestampReadType(field.type())))
+ .collect(Collectors.toList());
+ return rowType.copy(fields);
+ }
+ if (dataType instanceof ArrayType) {
+ ArrayType arrayType = (ArrayType) dataType;
+ return
arrayType.newElementType(createSafeTimestampReadType(arrayType.getElementType()));
+ }
+ if (dataType instanceof MapType) {
+ MapType mapType = (MapType) dataType;
+ return
mapType.newKeyValueType(createSafeTimestampReadType(mapType.getKeyType()),
Review Comment:
[P1] Do not widen timestamp map keys
Paimon 1.3.1 cannot cast MAP keys during schema evolution:
`createMapCastExecutor` requires the input and target key types to be equal.
Once any projected scalar timestamp enables repair, this helper rewrites every
projected `MAP<TIMESTAMP(p<9), ...>` key to p9, so selecting that scalar and an
otherwise unchanged timestamp-keyed map makes reader creation fail. The new
unit test asserts the widened key shape but never opens a reader, so it misses
the incompatibility. Preserve map key types while widening supported
value/element/row descendants, and add a reader-level regression for this
projection.
--
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]
