This is an automated email from the ASF dual-hosted git repository.
difin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 5ae5a709996 HIVE-29744: Concurrent INSERT INTO on new dynamic
partition causes data loss (non-ACID) (#6613)
5ae5a709996 is described below
commit 5ae5a709996c6950f2103dd4e252f224ac20e1a9
Author: Dmitriy Fingerman <[email protected]>
AuthorDate: Fri Jul 17 13:52:17 2026 -0400
HIVE-29744: Concurrent INSERT INTO on new dynamic partition causes data
loss (non-ACID) (#6613)
* HIVE-29744: Concurrent INSERT INTO on new dynamic partition causes data
loss (non-ACID)
---------
Co-authored-by: Dmitriy Fingerman <[email protected]>
Co-authored-by: Dmitriy Fingerman <[email protected]>
---
.../org/apache/hadoop/hive/ql/metadata/Hive.java | 4 +-
.../TestHiveLoadPartitionKeepExisting.java | 225 +++++++++++++++++++++
2 files changed, 227 insertions(+), 2 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
index 3a1b23e9f17..d7eb7281ecc 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
@@ -2898,8 +2898,8 @@ private Partition loadPartitionInternal(Path loadPath,
Table tbl, Map<String, St
}
boolean isManaged = tbl.getTableType() == TableType.MANAGED_TABLE;
- // TODO: why is "&& !isAcidIUDoperation" needed here?
- if (!isTxnTable && ((loadFileType == LoadFileType.REPLACE_ALL) ||
(oldPart == null && !isAcidIUDoperation))) {
+ if (!isTxnTable && ((loadFileType == LoadFileType.REPLACE_ALL)
+ || (oldPart == null && loadFileType !=
LoadFileType.KEEP_EXISTING))) {
//for fullAcid tables we don't delete files for commands with
OVERWRITE - we create a new
// base_x. (there is Insert Overwrite and Load Data Overwrite)
boolean isSkipTrash =
MetaStoreUtils.isSkipTrash(tbl.getParameters());
diff --git
a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveLoadPartitionKeepExisting.java
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveLoadPartitionKeepExisting.java
new file mode 100644
index 00000000000..a84f40ac3ce
--- /dev/null
+++
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveLoadPartitionKeepExisting.java
@@ -0,0 +1,225 @@
+/*
+ * 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.hadoop.hive.ql.metadata;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.OutputStream;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConfForTest;
+import org.apache.hadoop.hive.metastore.TableType;
+import org.apache.hadoop.hive.metastore.Warehouse;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat;
+import org.apache.hadoop.hive.ql.plan.LoadTableDesc.LoadFileType;
+import org.apache.hadoop.hive.ql.session.SessionState;
+import org.apache.hadoop.mapred.TextInputFormat;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Regression tests for {@link Hive#loadPartition} commit semantics on dynamic
partitions.
+ *
+ * <p>MoveTask calls {@code loadPartition} with {@link
LoadFileType#KEEP_EXISTING} after
+ * {@code INSERT INTO} (append) or {@link LoadFileType#REPLACE_ALL} after
{@code INSERT OVERWRITE}.
+ * These tests exercise that commit path directly — not a full SQL {@code
INSERT} statement.
+ */
+public class TestHiveLoadPartitionKeepExisting {
+
+ private static HiveConf hiveConf;
+ private static Hive hive;
+
+ private String tableName;
+
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+ hiveConf = new HiveConfForTest(TestHiveLoadPartitionKeepExisting.class);
+ // hive-site.xml defaults to SQLStdHiveAuthorizerFactoryForTest
(hive-it-util); use the ql
+ // module factory so this test runs from the ql module alone (same as
TestHive).
+ hiveConf.setVar(HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER,
+
"org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory");
+ hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_IN_TEST, true);
+ MetastoreConf.setBoolVar(hiveConf, MetastoreConf.ConfVars.HIVE_IN_TEST,
true);
+ SessionState.start(hiveConf);
+ hive = Hive.get(hiveConf);
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ tableName = "test_load_partition_keep_existing_" + System.nanoTime();
+ createExternalPartitionedTable(tableName);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ hive.dropTable(Warehouse.DEFAULT_DATABASE_NAME, tableName, true, true,
true);
+ }
+
+ /**
+ * {@link LoadFileType#KEEP_EXISTING} when partition data exists on disk but
HMS has no entry yet.
+ */
+ @Test
+ public void testKeepExistingAppendsWhenPartitionDirExistsButNotInHms()
throws Exception {
+ Table table = hive.getTable(tableName);
+ Map<String, String> partSpec = partitionSpec("2026-06-14");
+ Path partPath = new Path(table.getDataLocation(),
Warehouse.makePartPath(partSpec));
+ FileSystem fs = partPath.getFileSystem(hiveConf);
+
+ Path existingFile = new Path(partPath, "000000_0_writer_a");
+ fs.mkdirs(partPath);
+ try (OutputStream out = fs.create(existingFile)) {
+ out.write("writer-a\n".getBytes());
+ }
+
+ Path staging = createStagingDir(fs, table.getDataLocation(), "writer_b");
+ Path stagingFile = new Path(staging, "000000_0_writer_b");
+ try (OutputStream out = fs.create(stagingFile)) {
+ out.write("writer-b\n".getBytes());
+ }
+
+ hive.loadPartition(staging, table, partSpec, LoadFileType.KEEP_EXISTING,
true, false,
+ false, false, false, false, null, 0, false, false);
+
+ assertTrue("Existing partition file should survive KEEP_EXISTING
loadPartition (append)",
+ fs.exists(new Path(partPath, "000000_0_writer_a")));
+ assertTrue("Staged file should be moved in by KEEP_EXISTING loadPartition",
+ fs.exists(new Path(partPath, "000000_0_writer_b")));
+ }
+
+ @Test
+ public void testReplaceAllReplacesExistingPartitionData() throws Exception {
+ Table table = hive.getTable(tableName);
+ Map<String, String> partSpec = partitionSpec("2026-06-15");
+ Path partPath = new Path(table.getDataLocation(),
Warehouse.makePartPath(partSpec));
+ FileSystem fs = partPath.getFileSystem(hiveConf);
+
+ fs.mkdirs(partPath);
+ try (OutputStream out = fs.create(new Path(partPath, "000000_0_old"))) {
+ out.write("old-data\n".getBytes());
+ }
+
+ Path staging = createStagingDir(fs, table.getDataLocation(), "overwrite");
+ try (OutputStream out = fs.create(new Path(staging, "000000_0_new"))) {
+ out.write("new-data\n".getBytes());
+ }
+
+ hive.loadPartition(staging, table, partSpec, LoadFileType.REPLACE_ALL,
true, false,
+ false, false, false, false, null, 0, true, false);
+
+ assertFalse("REPLACE_ALL loadPartition should remove prior partition data",
+ fs.exists(new Path(partPath, "000000_0_old")));
+ assertTrue("REPLACE_ALL loadPartition should publish staged data",
+ fs.exists(new Path(partPath, "000000_0_new")));
+ }
+
+ @Test
+ public void testConcurrentKeepExistingAppendsBothStagedLoads() throws
Exception {
+ Table table = hive.getTable(tableName);
+ Map<String, String> partSpec = partitionSpec("2026-06-16");
+ FileSystem fs = table.getDataLocation().getFileSystem(hiveConf);
+
+ Path stagingA = createStagingDir(fs, table.getDataLocation(), "writer_a");
+ try (OutputStream out = fs.create(new Path(stagingA,
"000000_0_writer_a"))) {
+ out.write("writer-a\n".getBytes());
+ }
+
+ Path stagingB = createStagingDir(fs, table.getDataLocation(), "writer_b");
+ try (OutputStream out = fs.create(new Path(stagingB,
"000000_0_writer_b"))) {
+ out.write("writer-b\n".getBytes());
+ }
+
+ CyclicBarrier barrier = new CyclicBarrier(2);
+ SessionState parentSession = SessionState.get();
+ ExecutorService executor = Executors.newFixedThreadPool(2);
+ try {
+ Future<?> writerA = executor.submit(() -> {
+ SessionState.setCurrentSessionState(parentSession);
+ barrierAwait(barrier);
+ hive.loadPartition(stagingA, table, partSpec,
LoadFileType.KEEP_EXISTING, true, false,
+ false, false, false, false, null, 0, false, false);
+ return null;
+ });
+
+ Future<?> writerB = executor.submit(() -> {
+ SessionState.setCurrentSessionState(parentSession);
+ barrierAwait(barrier);
+ hive.loadPartition(stagingB, table, partSpec,
LoadFileType.KEEP_EXISTING, true, false,
+ false, false, false, false, null, 1, false, false);
+ return null;
+ });
+ writerA.get(2, TimeUnit.MINUTES);
+ writerB.get(2, TimeUnit.MINUTES);
+ } finally {
+ executor.shutdownNow();
+ }
+
+ Path partPath = new Path(table.getDataLocation(),
Warehouse.makePartPath(partSpec));
+ assertTrue("First KEEP_EXISTING loadPartition should publish its staged
file",
+ fs.exists(new Path(partPath, "000000_0_writer_a")));
+ assertTrue("Second KEEP_EXISTING loadPartition should append its staged
file",
+ fs.exists(new Path(partPath, "000000_0_writer_b")));
+ }
+
+ private static void barrierAwait(CyclicBarrier barrier) {
+ try {
+ barrier.await(1, TimeUnit.MINUTES);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private void createExternalPartitionedTable(String name) throws
HiveException {
+ hive.dropTable(Warehouse.DEFAULT_DATABASE_NAME, name, true, true, true);
+ hive.createTable(name,
+ Arrays.asList("value"),
+ Arrays.asList("load_date"),
+ TextInputFormat.class,
+ HiveIgnoreKeyTextOutputFormat.class);
+ Table table = hive.getTable(name);
+ table.setTableType(TableType.EXTERNAL_TABLE);
+ table.getParameters().put("EXTERNAL", "TRUE");
+ table.getParameters().put("transactional", "false");
+ hive.alterTable(table, false, null, false);
+ }
+
+ private static Map<String, String> partitionSpec(String loadDate) {
+ Map<String, String> partSpec = new LinkedHashMap<>();
+ partSpec.put("load_date", loadDate);
+ return partSpec;
+ }
+
+ private static Path createStagingDir(FileSystem fs, Path tablePath, String
suffix) throws Exception {
+ Path staging = new Path(tablePath, "_staging_" + suffix + "_" +
System.nanoTime());
+ fs.mkdirs(staging);
+ return staging;
+ }
+}