Copilot commented on code in PR #61358:
URL: https://github.com/apache/doris/pull/61358#discussion_r2937939466
##########
docker/thirdparties/docker-compose/iceberg/scripts/create_preinstalled_scripts/iceberg/run23.sql:
##########
@@ -0,0 +1,40 @@
+
+CREATE DATABASE IF NOT EXISTS demo.wap_test;
+
+
+USE demo.wap_test;
+
+
+DROP TABLE IF EXISTS orders_wap;
+
+-- WAP-enabled orders table
+CREATE TABLE orders_wap (
+ order_id INT,
+ customer_id INT,
+ amount DECIMAL(10, 2),
+ order_date STRING
+)
+USING iceberg;
+ALTER TABLE wap_test.orders_wap SET TBLPROPERTIES ('write.wap.enabled'='true');
+
+SET spark.wap.id = test_wap_001;
+
+
+
+INSERT INTO orders_wap VALUES
+ (1, 103, 150.00, '2025-12-03'),
+ (2, 104, 320.25, '2025-12-04');
+
Review Comment:
`SET spark.wap.id = test_wap_001` is session-scoped. Because the docker
entrypoint concatenates/sources all Iceberg SQL files into a single spark-sql
session, this setting can leak into subsequent scripts and accidentally stage
writes. Please reset/unset `spark.wap.id` after the WAP insert (e.g., `RESET
spark.wap.id`) to avoid cross-script side effects.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergPublishChangesAction.java:
##########
@@ -0,0 +1,128 @@
+// 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.datasource.iceberg.action;
+
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.TableIf;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.common.ArgumentParsers;
+import org.apache.doris.common.UserException;
+import org.apache.doris.datasource.ExternalTable;
+import org.apache.doris.datasource.iceberg.IcebergExternalTable;
+import org.apache.doris.info.PartitionNamesInfo;
Review Comment:
IcebergPublishChangesAction imports PartitionNamesInfo from
`org.apache.doris.info.PartitionNamesInfo`, but the factory and other Iceberg
actions use
`org.apache.doris.nereids.trees.plans.commands.info.PartitionNamesInfo`. This
mismatch will cause a compilation error or type incompatibility when
constructing the action. Please align the import/package to the Nereids
`PartitionNamesInfo` used elsewhere in this module.
##########
regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy:
##########
@@ -546,4 +546,97 @@ suite("test_iceberg_optimize_actions_ddl",
"p0,external,doris,external_docker,ex
"""
exception "Action 'expire_snapshots' does not support partition
specification"
}
+
+ //
=====================================================================================
+// Test Case 6: publish_changes action with WAP (Write-Audit-Publish) pattern
+// Simplified workflow:
+//
+// - Main branch is initially empty (0 rows)
+// - A WAP snapshot exists with wap.id = "test_wap_001" and 2 rows
+// - publish_changes should cherry-pick the WAP snapshot into the main branch
+//
=====================================================================================
+
+logger.info("Starting simplified WAP (Write-Audit-Publish) workflow
verification test")
+
+// WAP test database and table
+String wap_db = "wap_test"
+String wap_table = "orders_wap"
+
+// Step 1: Verify no data is visible before publish_changes
+logger.info("Step 1: Verifying table is empty before publish_changes")
+qt_wap_before_publish """
+ SELECT order_id, customer_id, amount, order_date
+ FROM ${catalog_name}.${wap_db}.${wap_table}
+ ORDER BY order_id
+"""
+
+// Step 2: Publish the WAP changes with wap_id = "test_wap_001"
+logger.info("Step 2: Publishing WAP changes with wap_id=test_wap_001")
+sql """
+ ALTER TABLE ${catalog_name}.${wap_db}.${wap_table}
+ EXECUTE publish_changes("wap_id" = "test_wap_001")
+"""
+logger.info("Publish changes executed successfully")
+
+// Step 3: Verify WAP data is visible after publish_changes
+logger.info("Step 3: Verifying WAP data is visible after publish_changes")
+qt_wap_after_publish """
+ SELECT order_id, customer_id, amount, order_date
+ FROM ${catalog_name}.${wap_db}.${wap_table}
+ ORDER BY order_id
+"""
+
+logger.info("Simplified WAP (Write-Audit-Publish) workflow verification
completed successfully")
+
+// Negative tests for publish_changes
+
+// publish_changes on table without write.wap.enabled = true (should fail)
+test {
+ String nonWapDb = "wap_test"
+ String nonWapTable = "orders_non_wap"
+
+ sql """
+ ALTER TABLE ${catalog_name}.${nonWapDb}.${nonWapTable}
+ EXECUTE publish_changes("wap_id" = "test_wap_001")
+ """
+ exception "Cannot find snapshot with wap.id = test_wap_001"
+}
+
+
+// publish_changes with missing wap_id (should fail)
+test {
+ sql """
+ ALTER TABLE ${catalog_name}.${db_name}.${table_name}
+ EXECUTE publish_changes ()
+ """
+ exception "Missing required argument: wap_id"
+}
+
+// publish_changes with invalid wap_id (should fail)
+test {
+ sql """
+ ALTER TABLE ${catalog_name}.${wap_db}.${wap_table}
+ EXECUTE publish_changes("wap_id" = "non_existing_wap_id")
+ """
+ exception "Cannot find snapshot with wap.id = non_existing_wap_id"
+}
+
+// publish_changes with partition specification (should fail)
+test {
+ sql """
+ ALTER TABLE ${catalog_name}.${db_name}.${table_name}
+ EXECUTE publish_changes ("wap_id" = "test_wap_001") PARTITIONS (part1)
+ """
+ exception "Action 'publish_changes' does not support partition
specification"
+}
+
+// publish_changes with WHERE condition (should fail)
+test {
+ sql """
+ ALTER TABLE ${catalog_name}.${db_name}.${table_name}
+ EXECUTE publish_changes ("wap_id" = "test_wap_001") WHERE id > 0
+ """
+ exception "Action 'publish_changes' does not support WHERE condition"
+}
Review Comment:
The newly added WAP test block is not indented consistently with the rest of
the suite body (comments and statements start at column 0 while surrounding
code is indented inside the `suite { ... }`). This makes the test harder to
read and maintain; please indent this block to match the file’s existing
formatting.
--
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]
