github-actions[bot] commented on code in PR #66244:
URL: https://github.com/apache/doris/pull/66244#discussion_r3675649507
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonColumnValue.java:
##########
@@ -200,9 +219,39 @@ public void unpackStruct(List<Integer> structFieldIndex,
List<ColumnValue> value
RowType rowType = (RowType) dataType;
// Projection entries are original child indexes, so the binary row
must keep the full RowType arity.
InternalRow row = record.getRow(idx, rowType.getFieldCount());
+ if (structValues == null) {
+ structValues = new ArrayList<>();
+ }
for (int i : structFieldIndex) {
- values.add(new PaimonColumnValue(row, i,
dorisType.getChildTypes().get(i),
- rowType.getFields().get(i).type(), timeZone));
+ values.add(reuseColumnValue(structValues, i, row, i,
dorisType.getChildTypes().get(i),
+ rowType.getFields().get(i).type()));
+ }
+ }
+
+ private PaimonColumnValue reuseColumnValue(
+ List<PaimonColumnValue> cache, int cacheIndex, DataGetters
childRecord, int childIndex,
+ ColumnType childDorisType, DataType childPaimonType) {
+ while (cache.size() <= cacheIndex) {
+ cache.add(null);
Review Comment:
These recursive per-position caches keep the historical union of nested
shapes until the scanner is closed. For `ARRAY<ARRAY<INT>>` with outer size N,
successive rows can put an M-element inner array at a different outer position
while the others are empty; every row has O(N+M) elements, but each outer
wrapper permanently keeps its own M-entry child cache, leaving O(N*M) wrappers
(and their last Paimon container references) reachable from the scanner.
Shrink/null paths never prune them, and the fixed-size benchmark does not
exercise this. Please bound/prune nested caches or use a scanner-owned
scratch/pool design whose retained size follows the maximum live row/batch
shape, and add a shifting-nested-shape retention test.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonColumnValue.java:
##########
@@ -70,7 +81,7 @@ public void setOffsetRow(InternalRow record) {
}
public void setTimeZone(String timeZone) {
- this.timeZone = timeZone;
+ this.timeZone = ZoneId.of(timeZone);
Review Comment:
Doris accepts short session-zone IDs through `TimeUtils.timeZoneAliasMap`
(for example, `EST`) and forwards them unchanged to `JniReader`; plain
`ZoneId.of("EST")` throws `ZoneRulesException`. Because `PaimonJniScanner`
calls this setter unconditionally, this change makes even an `INT`-only Paimon
scan fail during scanner construction, whereas the previous setter did not
resolve the zone unless an LTZ value was read. Please resolve with
Doris-equivalent alias semantics (including its `CST` handling) and cover a
supported short ID on a non-temporal scan.
--
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]
