github-actions[bot] commented on code in PR #66007:
URL: https://github.com/apache/doris/pull/66007#discussion_r3672114430
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergSnapshotCacheValue.java:
##########
@@ -54,4 +77,65 @@ public IcebergSnapshot getSnapshot() {
public Optional<Map<Integer, List<String>>> getNameMapping() {
return nameMapping;
}
+
+ public Optional<Table> getIcebergTable() {
+ return icebergTable;
+ }
+
+ private static Table freezeTableGeneration(Table table) {
+ if (!(table instanceof HasTableOperations)) {
+ return table;
+ }
+ TableOperations operations = ((HasTableOperations) table).operations();
+ TableOperations frozenOperations = new
FrozenTableOperations(operations, operations.current());
+ if (table instanceof BaseTable) {
+ return new BaseTable(frozenOperations, table.name(), ((BaseTable)
table).reporter());
+ }
+ return new BaseTable(frozenOperations, table.name());
+ }
+
+ private static class FrozenTableOperations implements TableOperations {
+ private final TableOperations delegate;
+ private final TableMetadata metadata;
+
+ private FrozenTableOperations(TableOperations delegate, TableMetadata
metadata) {
+ this.delegate = delegate;
+ this.metadata = metadata;
+ }
+
+ @Override
+ public TableMetadata current() {
+ return metadata;
+ }
+
+ @Override
+ public TableMetadata refresh() {
+ return metadata;
+ }
+
+ @Override
+ public void commit(TableMetadata base, TableMetadata newMetadata) {
+ throw new UnsupportedOperationException("Frozen Iceberg table
generation is read-only");
Review Comment:
[P1] Keep a writable commit handle separate from the frozen read view
`getIcebergTable()` now returns a `BaseTable` backed by this wrapper, and
the new sink/rewrite plumbing passes that exact object to `beginInsert`,
`beginMerge`, and `beginRewrite`. In Iceberg 1.10.1,
`BaseTable.newTransaction()` retains these operations; after Doris stages the
append/row-delta/rewrite, `IcebergTransaction.commit()` calls
`commitTransaction()`, whose simple-transaction path calls
`underlyingOps.commit(...)` here. Therefore every non-empty INSERT,
UPDATE/MERGE, or rewrite now reaches this exception at final commit. Keep the
frozen generation for planning/validation, but commit through writable catalog
operations fenced against it, and exercise a real `BaseTable` transaction; the
new retained-target test mocks `newTransaction()` and cannot catch this.
##########
be/src/core/data_type_serde/orc_serde_utils.h:
##########
@@ -0,0 +1,69 @@
+// 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.
+
+#pragma once
+
+#include <cstring>
+#include <orc/Vector.hh>
+
+#include "core/arena.h"
+
+namespace doris {
+
+inline void copy_orc_string_data_to_arena(orc::ColumnVectorBatch* batch,
Arena& arena) {
+ if (auto* strings = dynamic_cast<orc::StringVectorBatch*>(batch)) {
+ size_t total_size = 0;
+ for (size_t i = 0; i < strings->numElements; ++i) {
+ total_size += static_cast<size_t>(strings->length[i]);
+ }
+ char* cursor = total_size == 0 ? nullptr : arena.alloc(total_size);
Review Comment:
[P2] Skip the lifetime copy for Arena-owned string leaves
This helper is invoked unconditionally after masked Array/Map children
serialize. That is required for `DataTypeStringSerDe`, which borrows from the
temporary packed column, but not every ORC string child borrows:
`DataTypeIPv6SerDe::write_column_to_orc()` already allocates its entire
serialized payload from this same Arena and stores those pointers in
`StringVectorBatch`. For a nullable `ARRAY<IPV6>`/`MAP<...,IPV6>` block
containing any masked row, this line therefore allocates the full visible
payload a second time and copies it again; nested masked collections can repeat
the amplification at each level. Track whether leaves borrow the packed owner
(or otherwise skip pointers already owned by the Arena) so the correctness fix
does not add a serialization-sized memory/copy penalty.
--
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]
