Gabriel39 commented on code in PR #66007:
URL: https://github.com/apache/doris/pull/66007#discussion_r3675456922
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTransaction.java:
##########
@@ -289,14 +291,23 @@ public void beginDelete(ExternalTable dorisTable) throws
UserException {
}
}
- /**
- * Begin merge operation for Iceberg UPDATE (single scan RowDelta).
- */
- public void beginMerge(ExternalTable dorisTable) throws UserException {
+ private Table createTransactionTable(ExternalTable dorisTable, Table
retainedTable) {
Review Comment:
Fixed in 69aafa01a19. DELETE now carries the retained Iceberg table through
logical/physical/planner sinks into the executor and starts RowDelta from that
scanned generation. Covered by IcebergTransactionTest and
IcebergDeleteExecutorTest.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -177,6 +180,11 @@ public PaimonScanNode(PlanNodeId id,
@Override
protected void doInitialize() throws UserException {
+ Optional<MvccSnapshot> relationSnapshot = getRelationSnapshot();
+ if (desc.getTable() instanceof PaimonExternalTable) {
+ // Rebuild the source before applying query options so both layers
use this relation's snapshot.
+ source = new PaimonSource(desc, relationSnapshot);
Review Comment:
Fixed in 69aafa01a19. A relation bound to Paimon's invalid snapshot sentinel
now short-circuits split planning instead of consulting the live table. Covered
by the empty-bind/first-commit unit test.
##########
be/src/core/data_type_serde/orc_serde_utils.h:
##########
@@ -0,0 +1,74 @@
+// 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) {
+ const size_t length = static_cast<size_t>(strings->length[i]);
+ // Some serdes already allocate their payload in this Arena;
copying it again would
+ // double the serialized string memory without extending its
lifetime.
+ if (length > 0 && !arena.contains(strings->data[i], length)) {
+ total_size += length;
+ }
+ }
+ char* cursor = total_size == 0 ? nullptr : arena.alloc(total_size);
+ for (size_t i = 0; i < strings->numElements; ++i) {
+ const size_t length = static_cast<size_t>(strings->length[i]);
+ const char* source = strings->data[i];
+ if (length > 0 && !arena.contains(source, length)) {
+ std::memcpy(cursor, source, length);
+ strings->data[i] = cursor;
+ cursor += length;
+ } else if (length == 0) {
Review Comment:
Fixed in 69aafa01a19. Zero-length ORC strings keep a lifetime-safe non-null
sentinel, and the masked collection test verifies empty-string leaf min/max
statistics.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -232,7 +232,8 @@ private IcebergSnapshotCacheValue
loadSnapshotProjection(ExternalTable dorisTabl
latestIcebergSnapshot.getSnapshotId(),
latestIcebergSnapshot.getSchemaId());
}
return new IcebergSnapshotCacheValue(
- icebergPartitionInfo, latestIcebergSnapshot,
IcebergUtils.getNameMapping(icebergTable));
+ icebergPartitionInfo, latestIcebergSnapshot,
IcebergUtils.getNameMapping(icebergTable),
+ icebergTable);
Review Comment:
Fixed in 69aafa01a19. Iceberg TableOperations.current() is captured once
before deriving snapshot, partitions, name mapping, or the retained table.
Covered by the atomic projection unit test.
##########
be/src/format_v2/table_reader.h:
##########
@@ -1515,15 +1585,34 @@ class TableReader {
Status _materialize_array_mapping_column(const ColumnMapping& mapping,
const ColumnPtr& file_column,
const size_t rows,
- ColumnPtr* column) {
+ ColumnPtr* column,
+ const NullMap*
nullable_parent_null_map = nullptr) {
DORIS_CHECK(mapping.child_mappings.size() == 1);
const auto full_file_column =
file_column->convert_to_full_column_if_const();
const NullMap* parent_null_map = nullptr;
const auto* nested_file_column =
_nested_column_if_nullable(full_file_column, &parent_null_map);
+ if (parent_null_map != nullptr && !mapping.table_type->is_nullable()) {
+ DORIS_CHECK(parent_null_map->size() == rows);
+ if (nullable_parent_null_map != nullptr) {
+ DORIS_CHECK(nullable_parent_null_map->size() == rows);
+ }
+ for (size_t i = 0; i < rows; ++i) {
+ // ARRAY row masks cannot be forwarded to elements because
they use different
+ // coordinates, so validate the container before dropping its
nullable wrapper.
+ if ((*parent_null_map)[i] &&
+ (nullable_parent_null_map == nullptr ||
!(*nullable_parent_null_map)[i])) {
+ return Status::InternalError(
+ "Source array contains NULL for non-nullable table
column");
+ }
+ }
+ }
const auto* file_array = assert_cast<const
ColumnArray*>(nested_file_column);
ColumnPtr nested_column = file_array->get_data_ptr();
- const auto& element_mapping = mapping.child_mappings[0];
+ auto element_mapping = mapping.child_mappings[0];
+ // Keep the descriptor type for schema matching. ARRAY's nullable
element wrapper is a
+ // storage invariant, so add it only at the materialization boundary.
+ element_mapping.table_type = make_nullable(element_mapping.table_type);
RETURN_IF_ERROR(_materialize_present_child_mapping_column(
Review Comment:
Fixed in 69aafa01a19. Nullable Array/Map row masks are projected through
offsets into descendant entry coordinates before required-child validation.
Both hidden Array and Map payload cases pass under ASAN.
--
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]
