From 8091f9688c62af100af9a0803163d3181f84ce07 Mon Sep 17 00:00:00 2001 From: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Date: Sun, 12 Jul 2026 03:27:10 +0800 Subject: [PATCH] [fix](fe) Route binlog format changes through validation ### What problem does this PR solve? Issue Number: close #65383 Related PR: None Problem Summary: ALTER TABLE changes to binlog.format were not recognized as binlog configuration updates. They fell through to the generic schema-change path and returned the ROW-binlog schema-change error before the existing immutable-format validation could run. Route binlog.format through updateBinlogConfig so the existing BinlogConfig validation returns the specific format-change error. ### Release note ALTER TABLE attempts to change binlog.format now report the immutable binlog-format error instead of the generic ROW schema-change error. - Test: Unit Test - AlterOperationsTest - SchemaChangeHandlerTest#testChangeBinlogFormatReportsBinlogError - ./build.sh --fe - Behavior changed: Yes. Only the error reported for unsupported binlog.format changes is corrected. - Does this need documentation: No --- .../apache/doris/alter/AlterOperations.java | 3 +- .../doris/alter/AlterOperationsTest.java | 41 +++++++++++++++++++ .../doris/alter/SchemaChangeHandlerTest.java | 19 +++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/alter/AlterOperationsTest.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/AlterOperations.java b/fe/fe-core/src/main/java/org/apache/doris/alter/AlterOperations.java index 3de8b2c6659961..db0d508bf2dbed 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/AlterOperations.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/AlterOperations.java @@ -113,7 +113,8 @@ public boolean checkBinlogConfigChange(List alterOps) { ).anyMatch(clause -> clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_ENABLE) || clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_TTL_SECONDS) || clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_BYTES) - || clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_HISTORY_NUMS)); + || clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_HISTORY_NUMS) + || clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_FORMAT)); } public boolean isBeingSynced(List alterOps) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/AlterOperationsTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/AlterOperationsTest.java new file mode 100644 index 00000000000000..60e1e04c24d2e0 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/AlterOperationsTest.java @@ -0,0 +1,41 @@ +// 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.alter; + +import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.nereids.trees.plans.commands.info.AlterOp; +import org.apache.doris.nereids.trees.plans.commands.info.ModifyTablePropertiesOp; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +public class AlterOperationsTest { + + @Test + public void testBinlogFormatUsesBinlogConfigChangePath() { + Map properties = Collections.singletonMap( + PropertyAnalyzer.PROPERTIES_BINLOG_FORMAT, "STATEMENT_AND_SNAPSHOT"); + List alterOps = Collections.singletonList(new ModifyTablePropertiesOp(properties)); + + Assert.assertTrue(new AlterOperations().checkBinlogConfigChange(alterOps)); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java index 95b4cf9145f1b5..4b9cd63334b490 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java @@ -27,6 +27,7 @@ import org.apache.doris.catalog.Table; import org.apache.doris.catalog.info.ColumnPosition; import org.apache.doris.catalog.info.IndexType; +import org.apache.doris.common.Config; import org.apache.doris.common.FeConstants; import org.apache.doris.common.jmockit.Deencapsulation; import org.apache.doris.nereids.StatementContext; @@ -165,6 +166,24 @@ private void expectException(String alterStmt, String expectedErrorMsg) { } } + @Test + public void testChangeBinlogFormatReportsBinlogError() throws Exception { + boolean originalEnableFeatureBinlog = Config.enable_feature_binlog; + try { + Config.enable_feature_binlog = true; + String tableName = "binlog_format_change"; + createTable("CREATE TABLE test." + tableName + " (k1 INT NOT NULL) " + + "DUPLICATE KEY(k1) DISTRIBUTED BY HASH(k1) BUCKETS 1 " + + "PROPERTIES('replication_num'='1','binlog.enable'='true','binlog.format'='ROW')"); + + expectException("ALTER TABLE test." + tableName + + " SET ('binlog.format'='STATEMENT_AND_SNAPSHOT')", + "not support change binlog format from ROW to STATEMENT_AND_SNAPSHOT"); + } finally { + Config.enable_feature_binlog = originalEnableFeatureBinlog; + } + } + @Test public void testWithRowBinlogSchemaChangeNoHistoricalValue() throws Exception { String tableName = "binlog_no_hist";