github-actions[bot] commented on code in PR #69585:
URL: https://github.com/apache/doris/pull/69585#discussion_r4370405086
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/load/NereidsLoadPlanInfoCollector.java:
##########
@@ -331,6 +333,10 @@ public Void
visitLogicalOlapTableSink(LogicalOlapTableSink<? extends Plan> logic
taskInfo.getSendBatchParallelism(),
taskInfo.isLoadToSingleTablet(), taskInfo.isStrictMode(),
timeout, uniquekeyUpdateMode,
uniquekeyUpdateNewRowPolicy, partialUpdateInputColumns);
+ if (!Config.isCloudMode() && !(loadPlanInfo.olapTableSink
instanceof GroupCommitBlockSink)) {
+
Env.getCurrentGlobalTransactionMgr().registerChangelogWriteMappings(dbId,
taskInfo.getTxnId(),
Review Comment:
[P1] Register transactional VALUES snapshots on the master
In a shared-nothing cluster, connect to a follower/observer and run `BEGIN;
INSERT INTO t VALUES (...); COMMIT;` against a changelog-enabled table.
`BatchInsertIntoTableCommand` implements `NoForward`:
`InsertUtils.beginBatchInsertTransaction` forwards only `beginTxn` to the
master, then `InsertStreamTxnExecutor` plans locally. The master's
`BACKEND_STREAMING` PREPARE state is deliberately not journaled
(`DatabaseTransactionMgr.persistTransactionState` / `enqueueTransactionState`),
so this new local registration throws `Cannot register changelog mappings for
missing transaction ...` before execution. Forward the exact write snapshot to
the transaction-owning master, or perform planning there; merely skipping
follower registration would leave the required publish snapshot missing. Please
cover explicit transactional VALUES through a non-master FE.
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/ChangelogWriteMapping.java:
##########
@@ -0,0 +1,175 @@
+// 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.transaction;
+
+import org.apache.doris.catalog.Column;
+import org.apache.doris.thrift.TChangelogIndexWriteSnapshot;
+import org.apache.doris.thrift.TChangelogWriteColumnMapping;
+import org.apache.doris.thrift.TChangelogWriteColumnMappings;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gson.annotations.SerializedName;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.TreeMap;
+
+/** Immutable UID relationships captured from the schema used to plan a write.
*/
+public final class ChangelogWriteMapping {
+ private static final class Entry {
+ @SerializedName("source")
+ private final int source;
+ @SerializedName("current")
+ private final int current;
+ @SerializedName("before")
+ private final Integer before;
+
+ private Entry(int source, int current, Integer before) {
+ this.source = source;
+ this.current = current;
+ this.before = before;
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof Entry)) {
+ return false;
+ }
+ Entry entry = (Entry) other;
+ return source == entry.source && current == entry.current &&
Objects.equals(before, entry.before);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(source, current, before);
+ }
+ }
+
+ @SerializedName("historical")
+ private final boolean needHistoricalValue;
+ @SerializedName("entries")
+ private final List<Entry> entries;
+
+ private ChangelogWriteMapping(boolean historical, List<Entry> entries) {
+ Set<Integer> sources = new HashSet<>();
+ Set<Integer> targets = new HashSet<>();
+ for (Entry entry : entries) {
+ Preconditions.checkArgument(sources.add(entry.source), "Duplicate
source UID: %s", entry.source);
+ Preconditions.checkArgument(targets.add(entry.current), "Duplicate
target UID: %s", entry.current);
+ if (entry.before != null) {
+ Preconditions.checkArgument(historical, "Before UID requires
historical values");
+ Preconditions.checkArgument(targets.add(entry.before),
"Duplicate target UID: %s", entry.before);
+ }
+ }
+ List<Entry> sorted = new ArrayList<>(entries);
+ sorted.sort(Comparator.comparingInt(entry -> entry.source));
+ this.entries = ImmutableList.copyOf(sorted);
+ this.needHistoricalValue = historical;
+ }
+
+ public static ChangelogWriteMapping fromColumns(List<Column>
sourceColumns, List<Column> changelogColumns,
+ boolean historical) {
+ Map<String, Column> targets = new
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ for (Column column : changelogColumns) {
+ Preconditions.checkState(targets.put(column.getNonShadowName(),
column) == null,
+ "Duplicate changelog column: %s", column.getName());
+ }
+ List<Entry> entries = new ArrayList<>();
+ for (Column source : sourceColumns) {
+ if (!source.isVisible() && !source.isKey()) {
+ continue;
+ }
+ Column current = targets.get(source.getNonShadowName());
+ Preconditions.checkState(current != null, "Missing current column:
%s", source.getName());
+ Integer beforeUid = null;
+ if (historical && !source.isKey()) {
+ Column before =
targets.get(Column.generateBeforeColName(source.getNonShadowName()));
+ Preconditions.checkState(before != null, "Missing before
column: %s", source.getName());
+ beforeUid = before.getUniqueId();
+ }
+ entries.add(new Entry(source.getUniqueId(), current.getUniqueId(),
beforeUid));
Review Comment:
[P1] Migrate existing changelog source IDs before requiring unique UIDs
A changelog DUP table `(k INT, v INT)` created before this change with
`light_schema_change=false` is valid, but both source columns retain UID `-1`:
the old table UID initializer returned early, while only the companion index
initialized its IDs. The new `OlapTable.initSchemaColumnUniqueId` guard fixes
new creations, but its caller is the create-table path; neither
`OlapTable.gsonPostProcess` nor `MaterializedIndexMeta.gsonPostProcess`
migrates existing source IDs. After a full upgrade, every INSERT/load into that
persisted table reaches this method and fails with `Duplicate source UID: -1`
in the constructor. Provide compatible identity migration/resolution for the
existing FE/BE schemas before requiring unique source UIDs, and add coverage
that loads an old catalog with this table property.
--
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]
