This is an automated email from the ASF dual-hosted git repository.
abstractdog 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 1bcf6d4b857 HIVE-28822: Concurrent INSERTs can silently lose rows or
fail with FileAlreadyExistsException on S3 (non-ACID) (#6642)
1bcf6d4b857 is described below
commit 1bcf6d4b8570d84ce6a252b39ffae445d2771991
Author: Bodor Laszlo <[email protected]>
AuthorDate: Wed Aug 19 10:48:50 2026 +0200
HIVE-28822: Concurrent INSERTs can silently lose rows or fail with
FileAlreadyExistsException on S3 (non-ACID) (#6642)
---
.../org/apache/hadoop/hive/common/FileUtils.java | 29 ++
.../java/org/apache/hadoop/hive/ql/QueryPlan.java | 33 ++
.../hadoop/hive/ql/exec/ParsedOutputFileName.java | 16 +-
.../org/apache/hadoop/hive/ql/io/AcidUtils.java | 13 +-
.../org/apache/hadoop/hive/ql/metadata/Hive.java | 114 ++++--
.../hive/ql/exec/ParsedOutputFileNameTest.java | 42 ++
.../hadoop/hive/ql/metadata/TestHiveCopyFiles.java | 86 +++-
.../hive/ql/metadata/TestHiveCopyFilesFakeS3.java | 299 ++++++++++++++
.../ql/metadata/TestInsertCopySuffixOnFakeS3.java | 442 +++++++++++++++++++++
.../hadoop/hive/ql/util/FakeS3FileSystem.java | 101 +++++
.../clientpositive/acid_convert_16hex_copy_tag.q | 53 +++
.../llap/acid_convert_16hex_copy_tag.q.out | 124 ++++++
.../metastore/TransactionalValidationListener.java | 4 +-
13 files changed, 1309 insertions(+), 47 deletions(-)
diff --git a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
index faa13a61b1b..8a802fc77d3 100644
--- a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
+++ b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
@@ -85,6 +85,35 @@ public final class FileUtils {
private static final Random random = new Random();
public static final int IO_ERROR_SLEEP_TIME = 100;
+ /**
+ * Schemes whose single-file {@link FileSystem#rename(Path, Path)} is not
atomic-if-absent and
+ * can silently overwrite an existing destination when two concurrent
writers race between an
+ * {@code exists()} probe and the rename call (object stores where rename is
client-side
+ * copy+delete). Callers use this to decide whether to switch to a
uniqueness-tag copy suffix
+ * when moving output files. The list is in code because the set of unsafe
filesystems is a
+ * property of the filesystem implementation, not something an operator
should override.
+ * <p>
+ * Note on Azure: {@code abfs}/{@code abfss} only guarantee atomic rename
when the ADLS Gen2
+ * account has hierarchical namespace enabled; without HNS they degrade to
copy+delete like
+ * {@code wasb}. Since callers cannot cheaply tell the two apart at rename
time, the Azure
+ * schemes are included unconditionally — a false positive costs only a
slightly longer
+ * filename, whereas a false negative would be silent data loss.
+ */
+ public static final Set<String> NON_ATOMIC_RENAME_SCHEMES = new HashSet<>(
+ Arrays.asList("s3a", "s3n", "s3", "gs", "abfs", "abfss", "wasb",
"wasbs"));
+
+ /**
+ * @return {@code true} when the filesystem's URI scheme is one of the known
non-atomic-rename
+ * schemes ({@link #NON_ATOMIC_RENAME_SCHEMES}); {@code false}
otherwise (including a
+ * {@code null} fs or missing scheme).
+ */
+ public static boolean isNonAtomicRenameFs(FileSystem fs) {
+ if (fs == null || fs.getUri() == null || fs.getUri().getScheme() == null) {
+ return false;
+ }
+ return
NON_ATOMIC_RENAME_SCHEMES.contains(fs.getUri().getScheme().toLowerCase());
+ }
+
public static final PathFilter HIDDEN_FILES_PATH_FILTER = new PathFilter() {
@Override
public boolean accept(Path p) {
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java
b/ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java
index 18e38fc0da1..36b01834b75 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java
@@ -37,7 +37,9 @@
import java.util.concurrent.ConcurrentHashMap;
import java.nio.charset.StandardCharsets;
+import com.google.common.base.Strings;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.metastore.api.Schema;
import org.apache.hadoop.hive.ql.ddl.DDLDesc.DDLDescWithWriteId;
import org.apache.hadoop.hive.ql.exec.ConditionalTask;
@@ -312,6 +314,37 @@ public static String makeQueryId() {
+ UUID.randomUUID().toString();
}
+ /**
+ * Compute a compact per-query uniqueness tag used by the non-ACID rename
branch of
+ * {@link Hive#mvFile} to make each concurrent writer's destination key
unique on filesystems
+ * whose {@code rename} is not atomic-if-absent. The tag becomes the copy
suffix
+ * ({@code basename_copy_<tag>}) in place of the numeric {@code _copy_N}
counter.
+ * <p>
+ * Reads {@code hive.query.id} from the passed {@link HiveConf} and
delegates to
+ * {@link QueryPlan#extractUniquenessTag(String)} for the actual UUID → hex
derivation.
+ * The shape matches {@link ParsedOutputFileName}'s copy-index group so
downstream filename
+ * parsing (taskId, attemptId, copyIndex) keeps working.
+ */
+ public static String extractUniquenessTag(HiveConf conf) {
+ String qid = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_QUERY_ID);
+ if (Strings.isNullOrEmpty(qid)) {
+ throw new IllegalStateException("hive.query.id is required to derive a
unique destination name");
+ }
+ return QueryPlan.extractUniquenessTag(qid);
+ }
+
+ /**
+ * Extract a compact per-query uniqueness tag (16 lowercase hex chars) from
a query id emitted
+ * by {@link #makeQueryId()}. The tag is the {@code
getMostSignificantBits()} half of the UUID
+ * at the tail of the id, rendered as 16-char lowercase hex. Used by callers
that need a stable
+ * per-query filename component — e.g. {@code Hive.mvFile}'s destination
naming on filesystems
+ * whose {@code rename} is not atomic-if-absent.
+ */
+ public static String extractUniquenessTag(String queryId) {
+ UUID uuid = UUID.fromString(queryId.substring(queryId.lastIndexOf('_') +
1));
+ return String.format("%016x", uuid.getMostSignificantBits());
+ }
+
/**
* generate the operator graph and operator list for the given task based on
* the operators corresponding to that task.
diff --git
a/ql/src/java/org/apache/hadoop/hive/ql/exec/ParsedOutputFileName.java
b/ql/src/java/org/apache/hadoop/hive/ql/exec/ParsedOutputFileName.java
index 64d388b9262..f9c60a2dd2e 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/ParsedOutputFileName.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/ParsedOutputFileName.java
@@ -32,8 +32,11 @@
* 00001_02
* 00001_02.gz
* 00001_02.zlib.gz
- * 00001_02_copy_1
+ * 00001_02_copy_1 (numeric copy suffix, HDFS-style)
* 00001_02_copy_1.gz
+ * 00001_02_copy_abcd1234deadbeef (per-query uniqueness tag as
copy suffix,
+ * used on non-atomic-rename
filesystems)
+ * 00001_02_copy_abcd1234deadbeef.gz
* <p>
* All the components are here:
* tmp_(taskPrefix)00001_02_copy_1.zlib.gz
@@ -42,9 +45,9 @@ public class ParsedOutputFileName {
private static final Pattern COPY_FILE_NAME_TO_TASK_ID_REGEX =
Pattern.compile(
"^(.*?)?" + // any prefix
"(\\(.*\\))?" + // taskId prefix
- "([0-9]+)" + // taskId
- "(?:_([0-9]{1,6}))?" + // _<attemptId> (limited to 6 digits)
- "(?:_copy_([0-9]{1,6}))?" + // copy file index
+ "(\\d+)" + // taskId
+ "(?:_(\\d{1,6}))?" + // _<attemptId> (limited to 6 digits)
+ "(?:_copy_(\\d{1,6}|[0-9a-fA-F]{16}))?" + // copy suffix: numeric
counter, or 16-hex uniqueness tag
"(\\..*)?$"); // any suffix/file extension
public static ParsedOutputFileName parse(String fileName) {
@@ -109,6 +112,11 @@ public boolean isCopyFile() {
return copyIndex != null;
}
+ /**
+ * @return the copy suffix: either a numeric counter (HDFS-style) or an
8-hex per-query
+ * uniqueness tag (used on non-atomic-rename filesystems), or {@code
null} when the
+ * filename has no copy suffix.
+ */
public String getCopyIndex() {
return copyIndex;
}
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
index 1acd1f71ae2..decf7f36d2b 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
@@ -241,9 +241,13 @@ private AcidUtils() {
Pattern.compile("[0-9]+_[0-9]+");
/**
* @see org.apache.hadoop.hive.ql.exec.Utilities#COPY_KEYWORD
+ *
+ * The copy suffix is either a numeric counter (HDFS/local: _copy_N) or a
+ * 16-hex per-query uniqueness tag (non-atomic-rename FS such as S3A:
+ * _copy_<queryTag>). See ParsedOutputFileName#REGEX.
*/
public static final Pattern ORIGINAL_PATTERN_COPY =
- Pattern.compile("[0-9]+_[0-9]+" + COPY_KEYWORD + "[0-9]+");
+ Pattern.compile("[0-9]+_[0-9]+" + COPY_KEYWORD +
"(?:[0-9]{1,6}|[0-9a-fA-F]{16})");
public static final PathFilter acidHiddenFileFilter = new PathFilter() {
@Override
@@ -451,8 +455,11 @@ public static BucketMetaData parse(String bucketFileName) {
return new BucketMetaData(bucketId, 0);
}
else if(ORIGINAL_PATTERN_COPY.matcher(bucketFileName).matches()) {
- int copyNumber = Integer.parseInt(
- bucketFileName.substring(bucketFileName.lastIndexOf('_') + 1));
+ String copySuffix =
bucketFileName.substring(bucketFileName.lastIndexOf('_') + 1);
+ // Copy suffix is either a numeric counter or a 16-hex per-query
uniqueness tag.
+ // Hex-tagged files are unordered peers from concurrent writers on an
+ // non-atomic-rename FS, so there is no meaningful copy number to
assign — use 0.
+ int copyNumber = StringUtils.isNumeric(copySuffix) ?
Integer.parseInt(copySuffix) : 0;
int bucketId = Integer
.parseInt(bucketFileName.substring(0,
bucketFileName.indexOf('_')));
return new BucketMetaData(bucketId, copyNumber);
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 307cc48441f..ef74914401b 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
@@ -22,6 +22,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
@@ -164,6 +165,7 @@
import org.apache.hadoop.hive.metastore.utils.RetryUtilities;
import org.apache.hadoop.hive.ql.Context;
import org.apache.hadoop.hive.ql.ErrorMsg;
+import org.apache.hadoop.hive.ql.QueryPlan;
import org.apache.hadoop.hive.ql.ddl.database.drop.DropDatabaseDesc;
import org.apache.hadoop.hive.ql.ddl.table.AlterTableType;
import org.apache.hadoop.hive.ql.exec.AbstractFileMergeOperator;
@@ -5171,6 +5173,79 @@ private static String getPathName(int taskId) {
return Utilities.replaceTaskId("000000", taskId) + "_0";
}
+ /**
+ * Picks the destination {@link Path} for {@link #mvFile}, choosing between
a per-query
+ * uniqueness-tagged name (on filesystems without atomic rename-if-absent
semantics) and the
+ * legacy {@code _copy_N} counter-based picker.
+ *
+ * <p>On file systems without atomic rename-if-absent semantics (e.g. S3),
two concurrent inserts
+ * targeting the same new dynamic partition race in the counter-based picker
below: their
+ * {@code exists()} probes both fire before either PUT commits, both rename
to the same final
+ * key, and the second PUT silently overwrites the first (last writer wins,
no error surfaces).
+ * To eliminate the collision, on such filesystems we skip the counter-based
{@code _copy_N}
+ * picker entirely and use a per-query uniqueness tag (8-hex derived from
{@code hive.query.id})
+ * as the copy suffix, so two concurrent writers rename to distinct keys.
+ *
+ * <p>The uniqueness-tag path is only taken in the non-ACID rename branch
+ * ({@code taskId == -1 && isRenameAllowed && !isOverwrite}): ACID writers
already own unique
+ * taskIds, copy/copyFromLocal do not race on the destination filename, and
overwrite explicitly
+ * clears the target first.
+ */
+ private static Path pickDestFilePath(HiveConf conf, FileSystem sourceFs,
Path sourcePath, FileSystem destFs,
+ Path destDirPath, int taskId, boolean
isOverwrite, boolean isRenameAllowed)
+ throws IOException {
+
+ final String type = FilenameUtils.getExtension(sourcePath.getName());
+
+ // Strip off the file type, if any so we don't make:
+ // 000000_0.gz -> 000000_0.gz_copy_1
+ final String fullName = sourcePath.getName();
+
+ final String name;
+ if (taskId == -1) { // non-acid
+ name = FilenameUtils.getBaseName(sourcePath.getName());
+ } else { // acid
+ name = getPathName(taskId);
+ }
+
+ // In case of ACID, the file is ORC so the extension is not relevant and
should not be inherited.
+ Path destFilePath = new Path(destDirPath, taskId == -1 ? fullName : name);
+
+ final String uniqueCopySuffix =
+ // Only apply the unique suffix in case of files, as it's supposed to
handle file name collisions.
+ // When mvFile is called with a directory, we can fall back to the
original logic.
+ (taskId == -1 && isRenameAllowed && !isOverwrite &&
sourceFs.getFileStatus(sourcePath).isFile()
+ && FileUtils.isNonAtomicRenameFs(destFs))
+ ? QueryPlan.extractUniquenessTag(conf)
+ : null;
+
+ if (uniqueCopySuffix != null && !uniqueCopySuffix.isEmpty()) {
+ // Unstable-rename FS: use `name_copy_<tag>` unconditionally as the
destination. No
+ // exists()-probe loop, no _copy_N counter — the per-query tag alone is
enough to keep
+ // concurrent writers from colliding, and ParsedOutputFileName
recognizes the shape.
+ return new Path(destDirPath, name + Utilities.COPY_KEYWORD +
uniqueCopySuffix +
+ (!type.isEmpty() ? "." + type : ""));
+ }
+
+ /*
+ * The below loop may perform bad when the destination file already exists
and it has too many _copy_
+ * files as well. A desired approach was to call listFiles() and get a
complete list of files from
+ * the destination, and check whether the file exists or not on that list.
However, millions of files
+ * could live on the destination directory, and on concurrent situations,
this can cause OOM problems.
+ *
+ * I'll leave the below loop for now until a better approach is found.
+ */
+ for (int counter = 1; destFs.exists(destFilePath); counter++) {
+ if (isOverwrite) {
+ destFs.delete(destFilePath, false);
+ break;
+ }
+ destFilePath = new Path(destDirPath, name + (Utilities.COPY_KEYWORD +
counter) +
+ ((taskId == -1 && !type.isEmpty()) ? "." + type : ""));
+ }
+ return destFilePath;
+ }
+
/**
* <p>
* Moves a file from one {@link Path} to another. If {@code
isRenameAllowed} is true then the
@@ -5200,37 +5275,8 @@ private static String getPathName(int taskId) {
private static Path mvFile(HiveConf conf, FileSystem sourceFs, Path
sourcePath, FileSystem destFs, Path destDirPath,
boolean isSrcLocal, boolean isOverwrite, boolean
isRenameAllowed,
int taskId) throws IOException {
-
- // Strip off the file type, if any so we don't make:
- // 000000_0.gz -> 000000_0.gz_copy_1
- final String fullname = sourcePath.getName();
- final String name;
- if (taskId == -1) { // non-acid
- name = FilenameUtils.getBaseName(sourcePath.getName());
- } else { // acid
- name = getPathName(taskId);
- }
- final String type = FilenameUtils.getExtension(sourcePath.getName());
-
- // Incase of ACID, the file is ORC so the extension is not relevant and
should not be inherited.
- Path destFilePath = new Path(destDirPath, taskId == -1 ? fullname : name);
-
- /*
- * The below loop may perform bad when the destination file already exists
and it has too many _copy_
- * files as well. A desired approach was to call listFiles() and get a
complete list of files from
- * the destination, and check whether the file exists or not on that list.
However, millions of files
- * could live on the destination directory, and on concurrent situations,
this can cause OOM problems.
- *
- * I'll leave the below loop for now until a better approach is found.
- */
- for (int counter = 1; destFs.exists(destFilePath); counter++) {
- if (isOverwrite) {
- destFs.delete(destFilePath, false);
- break;
- }
- destFilePath = new Path(destDirPath, name + (Utilities.COPY_KEYWORD +
counter) +
- ((taskId == -1 && !type.isEmpty()) ? "." + type : ""));
- }
+ Path destFilePath = pickDestFilePath(conf, sourceFs, sourcePath, destFs,
destDirPath, taskId, isOverwrite,
+ isRenameAllowed);
if (isRenameAllowed) {
destFs.rename(sourcePath, destFilePath);
@@ -5242,7 +5288,7 @@ private static Path mvFile(HiveConf conf, FileSystem
sourceFs, Path sourcePath,
false, // overwrite destination
conf,
new DataCopyStatistics())) {
- LOG.error("Copy failed for source: " + sourcePath + " to destination:
" + destFilePath);
+ LOG.error("Copy failed for source: {} to destination: {}", sourcePath,
destFilePath);
throw new IOException("File copy failed.");
}
@@ -5250,10 +5296,10 @@ private static Path mvFile(HiveConf conf, FileSystem
sourceFs, Path sourcePath,
// have permission to delete the files in the source path. Ignore this
failure.
try {
if (!sourceFs.delete(sourcePath, true)) {
- LOG.warn("Delete source failed for source: " + sourcePath + " during
copy to destination: " + destFilePath);
+ LOG.warn("Delete source failed for source: {} during copy to
destination: {}", sourcePath, destFilePath);
}
} catch (Exception e) {
- LOG.warn("Delete source failed for source: " + sourcePath + " during
copy to destination: " + destFilePath, e);
+ LOG.warn("Delete source failed for source: {} during copy to
destination: {}", sourcePath, destFilePath, e);
}
}
return destFilePath;
diff --git
a/ql/src/test/org/apache/hadoop/hive/ql/exec/ParsedOutputFileNameTest.java
b/ql/src/test/org/apache/hadoop/hive/ql/exec/ParsedOutputFileNameTest.java
index 38fa535eeef..342ff337dae 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/exec/ParsedOutputFileNameTest.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/exec/ParsedOutputFileNameTest.java
@@ -121,6 +121,48 @@ public void testCopyAllParts() throws Exception {
Assert.assertEquals("tmp_(prefix)00001_02_copy_4",
p.makeFilenameWithCopyIndex(4));
}
+ /**
+ * On filesystems without atomic rename-if-absent semantics (S3 etc.), the
copy suffix
+ * carries a 16-hex per-query uniqueness tag instead of the numeric counter,
so concurrent
+ * writers rename to distinct destination keys.
+ */
+ @Test
+ public void testUniquenessTagAsCopySuffix() throws Exception {
+ ParsedOutputFileName p =
ParsedOutputFileName.parse("000001_0_copy_abcd1234deadbeef");
+ Assert.assertTrue(p.matches());
+ Assert.assertEquals("000001", p.getTaskId());
+ Assert.assertEquals("0", p.getAttemptId());
+ Assert.assertEquals("abcd1234deadbeef", p.getCopyIndex());
+ Assert.assertTrue(p.isCopyFile());
+ Assert.assertNull(p.getSuffix());
+ // Numeric-index renaming (used by legacy code paths) still works and
replaces the tag.
+ Assert.assertEquals("000001_0_copy_3", p.makeFilenameWithCopyIndex(3));
+ }
+
+ @Test
+ public void testUniquenessTagAsCopySuffixWithExtension() throws Exception {
+ ParsedOutputFileName p =
ParsedOutputFileName.parse("000001_0_copy_abcd1234deadbeef.snappy.orc");
+ Assert.assertTrue(p.matches());
+ Assert.assertEquals("000001", p.getTaskId());
+ Assert.assertEquals("0", p.getAttemptId());
+ Assert.assertEquals("abcd1234deadbeef", p.getCopyIndex());
+ Assert.assertTrue(p.isCopyFile());
+ Assert.assertEquals(".snappy.orc", p.getSuffix());
+ Assert.assertEquals("000001_0_copy_3", p.makeFilenameWithCopyIndex(3));
+ }
+
+ /**
+ * The copy-index group must reject shapes that are neither a 1..6 digit
counter nor an
+ * exactly-16-hex tag (e.g. non-hex characters, or a numeric tag longer than
6 digits).
+ */
+ @Test
+ public void testUniquenessTagShapeIsStrict() {
+ // 15 chars — matches neither branch.
+
Assert.assertNull(ParsedOutputFileName.parse("000001_0_copy_abcd1234deadbee").getCopyIndex());
+ // Non-hex character in a 16-char position.
+
Assert.assertNull(ParsedOutputFileName.parse("000001_0_copy_abcd1234deadbeez").getCopyIndex());
+ }
+
@Test
public void testNoMatch() {
ParsedOutputFileName p = ParsedOutputFileName.parse("ZfsLke");
diff --git
a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java
index 0a7cab86275..5a3c844b745 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java
@@ -20,8 +20,10 @@
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.common.FileUtils;
import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.conf.HiveConfForTest;
+import org.apache.hadoop.hive.ql.QueryPlan;
import org.apache.hadoop.hive.ql.session.SessionState;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -29,14 +31,19 @@
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
-import org.mockito.Mockito;
import java.io.IOException;
import java.net.URI;
import java.util.Arrays;
import java.util.List;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
@RunWith(Parameterized.class)
@@ -159,8 +166,8 @@ public void testCopyNewFilesOnDifferentFileSystem() throws
IOException {
Path targetPath = new Path(targetFolder.getRoot().getAbsolutePath());
// Simulate different filesystems by returning a different URI
- FileSystem spyTargetFs = Mockito.spy(targetPath.getFileSystem(hiveConf));
- Mockito.when(spyTargetFs.getUri()).thenReturn(URI.create("hdfs://" +
targetPath.toUri().getPath()));
+ FileSystem spyTargetFs = spy(targetPath.getFileSystem(hiveConf));
+ when(spyTargetFs.getUri()).thenReturn(URI.create("hdfs://" +
targetPath.toUri().getPath()));
try {
Hive.copyFiles(hiveConf, sourcePath, targetPath, spyTargetFs,
isSourceLocal, NO_ACID, false, null, false, false, false,
@@ -187,8 +194,8 @@ public void testCopyExistingFilesOnDifferentFileSystem()
throws IOException {
Path targetPath = new Path(targetFolder.getRoot().getAbsolutePath());
// Simulate different filesystems by returning a different URI
- FileSystem spyTargetFs = Mockito.spy(targetPath.getFileSystem(hiveConf));
- Mockito.when(spyTargetFs.getUri()).thenReturn(URI.create("hdfs://" +
targetPath.toUri().getPath()));
+ FileSystem spyTargetFs = spy(targetPath.getFileSystem(hiveConf));
+ when(spyTargetFs.getUri()).thenReturn(URI.create("hdfs://" +
targetPath.toUri().getPath()));
try {
Hive.copyFiles(hiveConf, sourcePath, targetPath, spyTargetFs,
isSourceLocal, NO_ACID, false, null,
@@ -228,4 +235,73 @@ public void testCopyExistingFilesOnDifferentFileSystem()
throws IOException {
assertTrue(spyTargetFs.exists(new Path(targetPath, "000000_0_copy_1.gz")));
assertTrue(spyTargetFs.exists(new Path(targetPath, "000001_0_copy_1.gz")));
}
+
+ /**
+ * When two concurrent writers stage a file with the same inner filename
(e.g. {@code 000000_0})
+ * into the same destination directory on an S3-like filesystem, mvFile must
pick distinct
+ * destination keys so the second writer does not silently overwrite the
first. Both files
+ * must land under distinct {@code 000000_0_copy_<hex>} names — no plain
{@code 000000_0}, no
+ * numeric {@code _copy_N}.
+ *
+ * <p>Covers the two moving parts individually since the full rename-branch
path in
+ * {@link Hive#copyFiles} requires src and dest FileSystems to compare equal
AND the dest
+ * scheme to be flagged non-atomic-rename, which is not easily synthesizable
with
+ * LocalFileSystem in a JUnit environment:
+ * <ol>
+ * <li>{@link FileUtils#isNonAtomicRenameFs(FileSystem)} recognizes
S3-family schemes on the URI
+ * and rejects HDFS / local schemes.</li>
+ * <li>Two distinct {@code hive.query.id} values map to two distinct 8-hex
uniqueness tags
+ * — the compact per-query identifier that mvFile appends when the
destination
+ * filesystem is a non-atomic-rename one. Confirms the tag is stable
for a given
+ * queryId, and that the tag's shape (8 hex chars) matches the
copy-suffix group in
+ * {@link org.apache.hadoop.hive.ql.exec.ParsedOutputFileName}'s
regex.</li>
+ * </ol>
+ */
+ @Test
+ public void testUniquenessTagAndUnstableFsGating() throws IOException {
+ // (1) non-atomic-rename filesystem detection via URI scheme
+ FileSystem localFs = new
Path(targetFolder.getRoot().getAbsolutePath()).getFileSystem(hiveConf);
+ assertFalse("local FS is atomic-rename",
FileUtils.isNonAtomicRenameFs(localFs));
+ assertFalse("null fs is not flagged",
FileUtils.isNonAtomicRenameFs((FileSystem) null));
+
+ for (String scheme : new String[] {"s3a", "s3n", "s3", "gs", "abfs",
"abfss", "wasb", "wasbs"}) {
+ FileSystem spy = spy(localFs);
+ when(spy.getUri()).thenReturn(URI.create(scheme + ":///bucket/path"));
+ assertTrue(scheme + " must be flagged non-atomic-rename",
+ FileUtils.isNonAtomicRenameFs(spy));
+ }
+ for (String scheme : new String[] {"hdfs", "file", "ofs", "adl"}) {
+ FileSystem spy = spy(localFs);
+ when(spy.getUri()).thenReturn(URI.create(scheme + ":///whatever"));
+ assertFalse(scheme + " must not be flagged non-atomic-rename",
+ FileUtils.isNonAtomicRenameFs(spy));
+ }
+
+ // (2) uniqueness tag: the 16-hex most-significant-bits half of the UUID
at the tail of
+ // queryId (QueryPlan.makeQueryId → "<user>_<timestamp>_<uuid>"; see
+ // QueryPlan.extractUniquenessTag). Distinct UUIDs → distinct tags.
+ hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID,
+ "lbodor_20260101120000_f47ac10b-58cc-4372-a567-0e02b2c3d479");
+ String tag1 = QueryPlan.extractUniquenessTag(hiveConf);
+ hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID,
+ "lbodor_20260101120001_9c8a44f1-e2b3-4a1c-9d3e-000000000000");
+ String tag2 = QueryPlan.extractUniquenessTag(hiveConf);
+
+ assertEquals("MSB half of the UUID at the tail", "f47ac10b58cc4372", tag1);
+ assertEquals("MSB half of the UUID at the tail", "9c8a44f1e2b34a1c", tag2);
+ assertTrue("tag1 must match <16-hex>: " + tag1,
tag1.matches("[0-9a-f]{16}"));
+ assertTrue("tag2 must match <16-hex>: " + tag2,
tag2.matches("[0-9a-f]{16}"));
+ assertNotEquals("distinct queryIds must produce distinct tags", tag1,
tag2);
+
+ // Missing queryId → hard failure (mvFile's non-atomic-rename branch must
not silently
+ // fall back to a shared filename when the query state is absent).
+ hiveConf.unset(HiveConf.ConfVars.HIVE_QUERY_ID.varname);
+ try {
+ QueryPlan.extractUniquenessTag(hiveConf);
+ fail("extractUniquenessTag must throw when hive.query.id is unset");
+ } catch (IllegalStateException expected) {
+ assertTrue("exception message must mention hive.query.id: " +
expected.getMessage(),
+ expected.getMessage() != null &&
expected.getMessage().contains("hive.query.id"));
+ }
+ }
}
diff --git
a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFilesFakeS3.java
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFilesFakeS3.java
new file mode 100644
index 00000000000..b618095a2fe
--- /dev/null
+++
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFilesFakeS3.java
@@ -0,0 +1,299 @@
+/*
+ * 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 org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.hive.common.FileUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConfForTest;
+import org.apache.hadoop.hive.ql.exec.ParsedOutputFileName;
+import org.apache.hadoop.hive.ql.io.AcidUtils;
+import org.apache.hadoop.hive.ql.session.SessionState;
+import org.apache.hadoop.hive.ql.util.FakeS3FileSystem;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * End-to-end tests for {@link Hive#copyFiles} through the
non-atomic-rename-FS branch of
+ * {@link Hive}'s move logic. Registers a synthetic {@code fakes3://} scheme
backed by
+ * {@link RawLocalFileSystem} (so real files land under a JUnit {@link
TemporaryFolder})
+ * and appends it directly to {@link FileUtils#NON_ATOMIC_RENAME_SCHEMES} for
the duration of
+ * this test class so {@link FileUtils#isNonAtomicRenameFs} treats it as a
non-atomic-rename
+ * FS. The scheme is removed in {@link #tearDownClass()} so no other test sees
it.
+ *
+ * <p>What this covers that the mockito-spy tests in {@link TestHiveCopyFiles}
do not:
+ * an actual rename() call is made through the tag-suffix branch of
+ * {@link Hive#pickDestFilePath}, and the resulting on-disk layout is asserted.
+ */
+class TestHiveCopyFilesFakeS3 {
+
+ /** Scheme registered as {@code fs.fakes3.impl} for the duration of these
tests. */
+ private static final String FAKE_SCHEME = FakeS3FileSystem.SCHEME;
+
+ private static HiveConf hiveConf;
+
+ @Rule
+ public TemporaryFolder tmp = new TemporaryFolder();
+
+ @BeforeClass
+ public static void setUpClass() {
+ hiveConf = new HiveConfForTest(TestHiveCopyFilesFakeS3.class);
+ // Register the fake scheme's FileSystem impl. Cache off so each test gets
a fresh
+ // instance rooted under its own TemporaryFolder without cross-test
leakage.
+ hiveConf.setClass("fs." + FAKE_SCHEME + ".impl", FakeS3FileSystem.class,
FileSystem.class);
+ hiveConf.setBoolean("fs." + FAKE_SCHEME + ".impl.disable.cache", true);
+ // Have FileUtils.isNonAtomicRenameFs treat our fake scheme as a
non-atomic-rename FS.
+ // This is a JVM-global mutation of a production static — we undo it in
tearDownClass
+ // so no test that runs after this class sees fakes3 in the set.
+ FileUtils.NON_ATOMIC_RENAME_SCHEMES.add(FAKE_SCHEME);
+ SessionState.start(hiveConf);
+ }
+
+ @AfterClass
+ public static void tearDownClass() {
+ FileUtils.NON_ATOMIC_RENAME_SCHEMES.remove(FAKE_SCHEME);
+ }
+
+ @Before
+ public void setUp() {
+ // Every test needs a fresh hive.query.id so computeUniquenessTag produces
a real tag.
+ hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID,
+ "test_" + System.nanoTime() + "_f47ac10b-58cc-4372-a567-0e02b2c3d479");
+ }
+
+ /**
+ * Builds a Path in the {@code fakes3://} namespace that points at the given
local
+ * subdirectory of the JUnit temp root. We use the local path as the URI
path so the
+ * underlying RawLocalFileSystem writes/reads real files there.
+ */
+ private Path fakes3Path(String subdir) throws IOException {
+ java.io.File dir = tmp.newFolder(subdir);
+ return new Path(URI.create(FAKE_SCHEME + "://" + dir.getAbsolutePath()));
+ }
+
+ /**
+ * fakes3 must be recognized as a non-atomic-rename FS once
+ * {@link #setUpClass()} has appended it to
+ * {@link FileUtils#NON_ATOMIC_RENAME_SCHEMES}; a plain {@code file://}
filesystem
+ * must not be.
+ */
+ @Test
+ public void fakes3IsFlaggedNonAtomicRename() throws IOException {
+ Path fakePath = fakes3Path("gate");
+ FileSystem fakeFs = fakePath.getFileSystem(hiveConf);
+
+ assertEquals(FAKE_SCHEME, fakeFs.getUri().getScheme());
+ assertTrue("fakes3 must be non-atomic-rename",
FileUtils.isNonAtomicRenameFs(fakeFs));
+
+ FileSystem localFs = new
Path(tmp.getRoot().getAbsolutePath()).getFileSystem(hiveConf);
+ assertFalse("local FS must not be flagged",
FileUtils.isNonAtomicRenameFs(localFs));
+ }
+
+ /**
+ * A single-file rename into a fresh destination under {@code fakes3://}
must land at
+ * {@code <name>_copy_<16-hex>} (uniqueness-tag suffix), not at {@code
<name>} — even
+ * though the target directory is empty. This is the whole point of the tag
branch:
+ * skip the exists() probe and stamp the name unconditionally so concurrent
writers
+ * cannot race on the same key.
+ */
+ @Test
+ public void singleFileRenameUsesUniquenessTagSuffix() throws Exception {
+ Path srcDir = fakes3Path("src");
+ Path dstDir = fakes3Path("dst");
+ FileSystem fs = dstDir.getFileSystem(hiveConf);
+ fs.create(new Path(srcDir, "000000_0")).close();
+
+ // fakes3 is flagged non-atomic-rename, so Hive.pickDestFilePath takes the
+ // tag branch: append _copy_<16-hex> unconditionally, skipping the
exists() probe.
+ Hive.copyFiles(hiveConf, srcDir, dstDir, fs, false, false, false, null,
+ false, false, false, false);
+
+ FileStatus[] listed = fs.listStatus(dstDir);
+ assertEquals("one output file expected", 1, listed.length);
+ String name = listed[0].getPath().getName();
+ // Reuse the production regexes from AcidUtils so the test tracks whatever
+ // the readers on the other end accept, and tighten to require the 16-hex
+ // uniqueness tag (not the numeric _copy_N fallback).
+ assertHas16HexUniquenessTag(name);
+ assertFalse("no plain 000000_N leaf allowed on non-atomic-rename FS: " +
name,
+ AcidUtils.ORIGINAL_PATTERN.matcher(name).matches());
+ }
+
+ /**
+ * Two staged files that share an inner filename must land at two distinct
+ * {@code _copy_<16-hex>} keys — never at plain {@code 000000_0} vs
+ * {@code 000000_0_copy_1}. Each of the two copyFiles calls runs under its
own
+ * hive.query.id, so the two tags differ and the two on-disk keys must differ
+ * too. That's the property that guards against silent overwrite on S3A.
+ */
+ @Test
+ public void twoStagedFilesLandAtDistinctKeys() throws Exception {
+ Path srcDir = fakes3Path("src");
+ Path dstDir = fakes3Path("dst");
+ FileSystem fs = dstDir.getFileSystem(hiveConf);
+
+ // Call #1 — queryId T1 set by @Before.
+ fs.create(new Path(srcDir, "000000_0")).close();
+ Hive.copyFiles(hiveConf, srcDir, dstDir, fs, false, false, false, null,
+ false, false, false, false);
+
+ // Stage a second file with the same base name and copy again — since
srcDir was
+ // consumed by the previous move, recreate the source folder.
+ fs.mkdirs(srcDir);
+ fs.create(new Path(srcDir, "000000_0")).close();
+ // Call #2 — different queryId → different tag → distinct destination name.
+ hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID,
+ "test_" + System.nanoTime() + "_9c8a44f1-e2b3-4a1c-9d3e-000000000000");
+ Hive.copyFiles(hiveConf, srcDir, dstDir, fs, false, false, false, null,
+ false, false, false, false);
+
+ FileStatus[] listed = fs.listStatus(dstDir);
+ assertEquals("two output files expected", 2, listed.length);
+ for (FileStatus s : listed) {
+ String name = s.getPath().getName();
+ // Nothing may be named plain 000000_N — that's the silent-overwrite
failure mode.
+ assertFalse("no plain 000000_N leaf allowed on non-atomic-rename FS: " +
name,
+ AcidUtils.ORIGINAL_PATTERN.matcher(name).matches());
+ assertHas16HexUniquenessTag(name);
+ }
+ assertNotEquals("two writers must land at distinct keys",
+ listed[0].getPath().getName(), listed[1].getPath().getName());
+ }
+
+ /**
+ * Ten threads all call {@link Hive#copyFiles} concurrently into the SAME
+ * destination directory on {@code fakes3://}, each staging its own
+ * {@code 000000_0} under a per-thread source directory. Each thread runs
with
+ * its own {@link HiveConf} clone and its own {@code hive.query.id}, so
+ * {@link Hive#computeUniquenessTag} produces ten distinct 16-hex tags. The
+ * invariants:
+ * <ol>
+ * <li>Exactly 10 files end up in the destination (nothing was silently
+ * overwritten on rename).</li>
+ * <li>Every filename matches {@link AcidUtils#ORIGINAL_PATTERN_COPY}
+ * (the {@code _copy_<tag>} shape) and none matches
+ * {@link AcidUtils#ORIGINAL_PATTERN} (no plain {@code 000000_N}).</li>
+ * <li>All 10 filenames are distinct.</li>
+ * </ol>
+ * This is the actual multi-writer race the tag branch of
+ * {@link Hive#pickDestFilePath} is designed to protect against on S3A —
+ * driven from Java threads directly rather than through the planner.
+ */
+ @Test
+ public void tenConcurrentCopiesLandAtDistinctTaggedKeys() throws Exception {
+ final int threads = 10;
+ // Shared destination for every writer — this is what makes it a race.
+ final Path dstDir = fakes3Path("dst");
+ final FileSystem fs = dstDir.getFileSystem(hiveConf);
+
+ // Stage each thread's source in its own directory. All ten source files
+ // are named 000000_0 — the collision we're testing is on the DESTINATION
+ // side, where the tag branch is expected to make the ten same-named
+ // inputs land at ten distinct keys. The per-thread source dir is a
+ // mechanical necessity (rename() consumes its source, so multiple threads
+ // can't share one physical file), not a collision-avoidance measure.
+ final Path[] srcDirs = new Path[threads];
+ final HiveConf[] confs = new HiveConf[threads];
+ for (int i = 0; i < threads; i++) {
+ srcDirs[i] = fakes3Path("src" + i);
+ fs.create(new Path(srcDirs[i], "000000_0")).close();
+
+ confs[i] = new HiveConf(hiveConf);
+ confs[i].setVar(HiveConf.ConfVars.HIVE_QUERY_ID,
+ "test_concurrent_" + i + "_" + java.util.UUID.randomUUID());
+ }
+
+ final SessionState parentSession = SessionState.get();
+ ExecutorService pool = Executors.newFixedThreadPool(threads);
+ try {
+ List<Future<Void>> futures = new java.util.ArrayList<>();
+ for (int i = 0; i < threads; i++) {
+ final int idx = i;
+ futures.add(pool.submit(() -> {
+ SessionState.setCurrentSessionState(parentSession);
+ Hive.copyFiles(confs[idx], srcDirs[idx], dstDir, fs, false, false,
false, null,
+ false, false, false, false);
+ return null;
+ }));
+ }
+ for (Future<Void> f : futures) {
+ // Surface the first exception rather than swallowing it as a timeout.
+ f.get(60, TimeUnit.SECONDS);
+ }
+ } finally {
+ pool.shutdown();
+ assertTrue("executor did not terminate", pool.awaitTermination(30,
TimeUnit.SECONDS));
+ }
+
+ FileStatus[] listed = fs.listStatus(dstDir);
+ assertEquals("expected exactly " + threads + " output files, got: "
+ + java.util.Arrays.toString(listed), threads, listed.length);
+ Set<String> names = new HashSet<>();
+ for (FileStatus s : listed) {
+ String name = s.getPath().getName();
+ assertFalse("no plain 000000_N leaf allowed on non-atomic-rename FS: " +
name,
+ AcidUtils.ORIGINAL_PATTERN.matcher(name).matches());
+ assertHas16HexUniquenessTag(name);
+ assertTrue("duplicate destination filename after concurrent copyFiles: "
+ name,
+ names.add(name));
+ }
+ }
+
+ /**
+ * Asserts that {@code name} is a copy-suffixed filename whose suffix is the
+ * 16-hex per-query uniqueness tag (as opposed to the numeric {@code _copy_N}
+ * fallback that AcidUtils.ORIGINAL_PATTERN_COPY also accepts). Parses via
+ * ParsedOutputFileName so we track whatever it accepts.
+ */
+ private static void assertHas16HexUniquenessTag(String name) {
+ // Baseline: must match the copy pattern the readers accept.
+ assertTrue("must match _copy_ pattern, got: " + name,
+ AcidUtils.ORIGINAL_PATTERN_COPY.matcher(name).matches());
+ // Tighten: the suffix must be exactly 16 hex chars, not the numeric
fallback.
+ ParsedOutputFileName parsed = ParsedOutputFileName.parse(name);
+ assertTrue("ParsedOutputFileName should recognize " + name,
parsed.matches());
+ assertTrue("expected copy suffix on " + name, parsed.isCopyFile());
+ String tag = parsed.getCopyIndex();
+ assertEquals("copy suffix must be 16 chars long (uniqueness tag, not
numeric fallback), got: " + tag,
+ 16, tag.length());
+ assertTrue("copy suffix must be lowercase-or-uppercase hex, got: " + tag,
+ tag.matches("[0-9a-fA-F]{16}"));
+ }
+}
diff --git
a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestInsertCopySuffixOnFakeS3.java
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestInsertCopySuffixOnFakeS3.java
new file mode 100644
index 00000000000..f6b5e0c9242
--- /dev/null
+++
b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestInsertCopySuffixOnFakeS3.java
@@ -0,0 +1,442 @@
+/*
+ * 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 java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hive.common.FileUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.ql.TxnCommandsBaseForTests;
+import org.apache.hadoop.hive.ql.exec.AbstractFileMergeOperator;
+import org.apache.hadoop.hive.ql.exec.ParsedOutputFileName;
+import org.apache.hadoop.hive.ql.io.AcidUtils;
+import org.apache.hadoop.hive.ql.util.FakeS3FileSystem;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Driver-level end-to-end test for the copy-suffix logic in
+ * {@link Hive#mvFile} on a non-atomic-rename filesystem.
+ *
+ * <p>Runs real {@code INSERT INTO} and {@code INSERT INTO ... UNION ALL ...}
+ * queries through the {@link org.apache.hadoop.hive.ql.Driver} and inspects
+ * the resulting on-disk layout of tables whose LOCATION is a
+ * synthetic {@code fakes3://} URI. The scheme is registered as an alias for
+ * {@link RawLocalFileSystem} (so files still live under {@code test.tmp.dir}
+ * on the local disk) and appended to {@link
FileUtils#NON_ATOMIC_RENAME_SCHEMES}
+ * so {@link FileUtils#isNonAtomicRenameFs} treats it like S3A.
+ *
+ * <p>What this covers over {@link TestHiveCopyFilesFakeS3}: the whole
+ * planner/executor/MoveTask path is exercised, not just {@code Hive.copyFiles}
+ * in isolation. Anything that changes how output files land in the table
+ * directory (e.g. FileSinkOperator, MoveTask, UnionProcFactory) is on the
+ * hook here.
+ */
+class TestInsertCopySuffixOnFakeS3 extends TxnCommandsBaseForTests {
+
+ private static final String FAKE_SCHEME = FakeS3FileSystem.SCHEME;
+ private static final String TEST_DATA_DIR = new
File(System.getProperty("java.io.tmpdir")
+ + File.separator + TestInsertCopySuffixOnFakeS3.class.getCanonicalName()
+ + "-" + System.currentTimeMillis()).getPath().replaceAll("\\\\", "/");
+
+ @BeforeAll
+ static void addFakeSchemeToUnstableSet() {
+ FileUtils.NON_ATOMIC_RENAME_SCHEMES.add(FAKE_SCHEME);
+ }
+
+ @AfterAll
+ static void removeFakeSchemeFromUnstableSet() {
+ FileUtils.NON_ATOMIC_RENAME_SCHEMES.remove(FAKE_SCHEME);
+ }
+
+ @Override
+ protected String getTestDataDir() {
+ return TEST_DATA_DIR;
+ }
+
+ @Override
+ protected void initHiveConf() {
+ super.initHiveConf();
+ // Register the fake scheme's FileSystem impl for this session.
+ hiveConf.setClass("fs." + FAKE_SCHEME + ".impl", FakeS3FileSystem.class,
FileSystem.class);
+ hiveConf.setBoolean("fs." + FAKE_SCHEME + ".impl.disable.cache", true);
+ // Non-strict managed tables so we can point tables outside the warehouse.
+ HiveConf.setBoolVar(hiveConf,
HiveConf.ConfVars.HIVE_STRICT_MANAGED_TABLES, false);
+ HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.CREATE_TABLES_AS_ACID,
false);
+ HiveConf.setBoolVar(hiveConf,
HiveConf.ConfVars.HIVE_CREATE_TABLES_AS_INSERT_ONLY, false);
+ HiveConf.setVar(hiveConf, HiveConf.ConfVars.DYNAMIC_PARTITIONING_MODE,
"nonstrict");
+ // UNION-ALL: keep subdirs unflattened so we exercise the layout that
production S3 workloads see.
+ HiveConf.setBoolVar(hiveConf,
HiveConf.ConfVars.HIVE_TEZ_UNION_FLATTEN_SUBDIRECTORIES, false);
+ }
+
+ @AfterEach
+ void dropAllTestTables() throws Exception {
+ for (String t : new String[] {"insert_into_fakes3", "union_all_fakes3",
"union_all_dyn_part_fakes3",
+ "insert_only_fakes3", "full_acid_fakes3", "union_src"}) {
+ try {
+ runQuery("drop table if exists " + t);
+ } catch (Exception ignore) {
+ // don't let a residual-drop failure hide the real test failure
+ }
+ }
+ }
+
+ @Override
+ protected void setUpSchema() {
+ // Override the parent's schema — we don't need the ACID/bucketed
+ // TxnCommandsBaseForTests fixture tables; each test creates its own
+ // external table at a fakes3:// location.
+ }
+
+ @Override
+ protected void dropTables() {
+ // The parent's dropTables would try to drop the schema tables that we
+ // never created; skip.
+ }
+
+ private List<String> runQuery(String stmt) throws Exception {
+ hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID,
org.apache.hadoop.hive.ql.QueryPlan.makeQueryId());
+ d.run(stmt);
+ List<String> rs = new ArrayList<>();
+ d.getResults(rs);
+ return rs;
+ }
+
+ /**
+ * Absolute local path under the test temp dir, wrapped in a {@code
fakes3://}
+ * URI. Files land on the local disk at {@code path}, but Hive resolves the
+ * URI to our FakeS3FileSystem and applies the non-atomic-rename logic.
+ */
+ private String fakeS3Location(String subdir) {
+ return FAKE_SCHEME + "://" + TEST_DATA_DIR + "/" + subdir;
+ }
+
+ private Path fakeS3Path(String subdir) {
+ return new Path(URI.create(fakeS3Location(subdir)));
+ }
+
+ /** Collect every file under {@code root} into a flat list of relative
paths. */
+ private List<String> listFilesRelative(Path root) throws IOException {
+ FileSystem fs = root.getFileSystem(hiveConf);
+ List<String> paths = new ArrayList<>();
+ if (!fs.exists(root)) {
+ return paths;
+ }
+ RemoteIterator<LocatedFileStatus> it = fs.listFiles(root, true);
+ while (it.hasNext()) {
+ LocatedFileStatus s = it.next();
+ if (s.isFile() && !s.getPath().getName().startsWith("_") &&
!s.getPath().getName().startsWith(".")) {
+ String full = s.getPath().toUri().getPath();
+ String rootPath = root.toUri().getPath();
+ paths.add(full.startsWith(rootPath) ?
full.substring(rootPath.length()) : full);
+ }
+ }
+ return paths;
+ }
+
+ /**
+ * Simple {@code INSERT INTO} into a non-ACID external ORC table on
+ * fakes3://. Every output file must carry the 16-hex uniqueness-tag copy
+ * suffix — no plain {@code 000000_N}.
+ */
+ @Test
+ void testInsertIntoNonAcidExternalOnFakeS3() throws Exception {
+ String tbl = "insert_into_fakes3";
+ Path loc = fakeS3Path(tbl);
+
+ runQuery(
+ "create external table " + tbl + " (a int, b int) stored as orc "
+ + "location '" + fakeS3Location(tbl) + "' "
+ + "tblproperties
('transactional'='false','external.table.purge'='true')");
+
+ runQuery("insert into " + tbl + " values (1, 10), (2, 20), (3, 30)");
+
+ List<String> files = listFilesRelative(loc);
+ assertFalse(files.isEmpty(), "insert produced no files under " + loc);
+ for (String rel : files) {
+ String name = rel.substring(rel.lastIndexOf('/') + 1);
+ assertHas16HexUniquenessTag(name);
+ assertFalse(AcidUtils.ORIGINAL_PATTERN.matcher(name).matches(),
+ "no plain 000000_N leaf allowed on non-atomic-rename FS: " + rel);
+ }
+
+ assertRowCount(tbl, 3);
+
+ convertToFullAcidAndAssertRowCount(tbl, 3);
+ }
+
+ /**
+ * {@code INSERT INTO ... UNION ALL ...} on a non-ACID external ORC
+ * table. Every output file must carry the tag suffix, no two leaves may
+ * share a name (that's the anti-silent-overwrite guarantee), and the row
+ * count must be 3. The number of output files is a planner choice — one
+ * combined mapper vs one-per-branch — so we do NOT assert on it.
+ * hive.tez.union.flatten.subdirectories=false is set, so the
+ * HIVE_UNION_SUBDIR_N layout is preserved when the planner does emit it.
+ */
+ @Test
+ void testUnionAllInsertOnFakeS3() throws Exception {
+ String tbl = "union_all_fakes3";
+ Path loc = fakeS3Path(tbl);
+
+ runQuery(
+ "create external table " + tbl + " (a int, b int) stored as orc "
+ + "location '" + fakeS3Location(tbl) + "' "
+ + "tblproperties
('transactional'='false','external.table.purge'='true')");
+
+ createUnionSrc();
+ runQuery(
+ "insert into " + tbl + " "
+ + "select k as a, sum(v) as b from union_src where k = 1 group by k
union all "
+ + "select k as a, sum(v) as b from union_src where k = 2 group by k
union all "
+ + "select k as a, sum(v) as b from union_src where k = 3 group by k");
+
+ assertUnionSubdirLayoutAt(loc, /* partitionSegment */ null);
+
+ assertRowCount(tbl, 3);
+
+ // FIXME: convertToFullAcidAndAssertRowCount should pass after HIVE-29798
is fixed
+ assertThrows(Exception.class,
+ () -> convertToFullAcidAndAssertRowCount(tbl, 3),
+ "expected read-back after CONVERT TO ACID to fail until HIVE-29798 is
fixed");
+ }
+
+ /**
+ * Same 3-way UNION ALL, but this time the target is a partitioned external
+ * table and the UNION branches all land in the same new dynamic partition.
+ * This is the shape the HIVE-28822 concurrent-insert repro compressed into a
+ * single statement. Every leaf must still be tagged; no writer may clobber
+ * another.
+ */
+ @Test
+ void testUnionAllInsertToPartitionedOnFakeS3() throws Exception {
+ String tbl = "union_all_dyn_part_fakes3";
+ Path loc = fakeS3Path(tbl);
+
+ runQuery(
+ "create external table " + tbl + " (a int) partitioned by (b int)
stored as orc "
+ + "location '" + fakeS3Location(tbl) + "' "
+ + "tblproperties
('transactional'='false','external.table.purge'='true')");
+
+ createUnionSrc();
+ runQuery(
+ "insert into " + tbl + " partition (b) "
+ + "select k as a, 2 as b from union_src where k = 1 group by k union
all "
+ + "select k as a, 2 as b from union_src where k = 2 group by k union
all "
+ + "select k as a, 2 as b from union_src where k = 3 group by k");
+
+ assertUnionSubdirLayoutAt(loc, /* partitionSegment */ "b=2");
+
+ assertRowCount(tbl, 3);
+
+ convertToFullAcidAndAssertRowCount(tbl, 3);
+ }
+
+ /**
+ * Materialize a small staging table {@code union_src} so each UNION branch
+ * does a real group-by scan; that keeps the planner from constant-folding
+ * the branches into a single mapper and preserves the per-branch
+ * HIVE_UNION_SUBDIR_<N>/ layout at final-move time.
+ */
+ private void createUnionSrc() throws Exception {
+ runQuery("drop table if exists union_src");
+ runQuery("create table union_src (k int, v int) stored as orc "
+ + "tblproperties ('transactional'='false')");
+ runQuery("insert into union_src values (1, 10), (2, 20), (3, 30)");
+ }
+
+ /**
+ * Common tail of the two UNION-ALL tests. Lists every file under {@code loc}
+ * and asserts:
+ * <ol>
+ * <li>Some files were produced (union insert didn't silently no-op).</li>
+ * <li>If {@code partitionSegment} is non-null, every leaf's path contains
+ * {@code /<partitionSegment>/} (e.g. {@code /b=2/}).</li>
+ * <li>Every leaf's parent directory starts with
+ * {@link AbstractFileMergeOperator#UNION_SUDBIR_PREFIX}, and there are
+ * exactly 3 such distinct parent dirs (one per UNION branch).</li>
+ * <li>Every leaf name is a plain writer name (matches
+ * {@link AcidUtils#ORIGINAL_PATTERN}) with NO {@code _copy_<tag>}
suffix:
+ * with per-branch subdirs there is no target-side collision to defend
+ * against, so {@code pickDestFilePath} does not stamp a uniqueness tag
+ * on top. The HIVE_UNION_SUBDIR_<N>/ separation is the anti-collision
+ * mechanism here.</li>
+ * </ol>
+ */
+ private void assertUnionSubdirLayoutAt(Path loc, String partitionSegment)
throws IOException {
+ List<String> files = listFilesRelative(loc);
+ assertFalse(files.isEmpty(), "union-all insert produced no files under " +
loc);
+
+ Set<String> subdirs = new HashSet<>();
+ for (String rel : files) {
+ if (partitionSegment != null) {
+ assertTrue(rel.contains("/" + partitionSegment + "/"),
+ "output must live under " + partitionSegment + " partition: " +
rel);
+ }
+ int lastSlash = rel.lastIndexOf('/');
+ int prevSlash = rel.lastIndexOf('/', lastSlash - 1);
+ String parentDir = rel.substring(prevSlash + 1, lastSlash);
+
assertTrue(parentDir.startsWith(AbstractFileMergeOperator.UNION_SUDBIR_PREFIX),
+ "union branch's leaf must live under a HIVE_UNION_SUBDIR_<N>/ dir: "
+ rel);
+ subdirs.add(parentDir);
+
+ String name = rel.substring(lastSlash + 1);
+ assertTrue(AcidUtils.ORIGINAL_PATTERN.matcher(name).matches(),
+ "leaf must be a plain writer name (no _copy_<tag> suffix expected):
" + rel);
+ ParsedOutputFileName parsed = ParsedOutputFileName.parse(name);
+ assertTrue(parsed.matches(), "ParsedOutputFileName should recognize " +
name);
+ assertFalse(parsed.isCopyFile(),
+ "leaf must NOT carry a copy suffix — HIVE_UNION_SUBDIR_ layout
already"
+ + " isolates concurrent writers, so no uniqueness tag is
expected: " + rel);
+ }
+ assertEquals(3, subdirs.size(),
+ "each of the 3 UNION branches must have its own
HIVE_UNION_SUBDIR_<N>/: " + subdirs);
+ }
+
+ /**
+ * Insert into a micromanaged (insert-only ACID) table on fakes3://. Unlike
the
+ * non-ACID cases above, MM tables get their per-writer uniqueness from the
+ * writeId-scoped {@code delta_<writeId>_<writeId>_<stmt>/} subdirectory
rather
+ * than from a per-query filename tag: MoveTask short-circuits into the delta
+ * layout without going through {@link Hive#copyFiles} at all. So on a
non-atomic-rename
+ * FS the leaves inside the delta directory are the plain writer-emitted
names
+ * (e.g. {@code 000000_N}) — that's expected, and the delta_ dir alone keeps
+ * two concurrent MM writers from clobbering each other.
+ *
+ * <p>Two concurrent MM inserts would get separate {@code delta_<writeId>}
dirs,
+ * so a plain {@code 000000_0} inside each is safe. We can't easily
reproduce a
+ * concurrent write here, but we can pin the current single-insert layout:
+ * every leaf lives under a delta_ subdirectory, and the row count matches.
+ */
+ @Test
+ void testInsertIntoMicromanagedOnFakeS3LandsUnderDeltaSubdir() throws
Exception {
+ String tbl = "insert_only_fakes3";
+ Path loc = fakeS3Path(tbl);
+ runQuery(
+ "create table " + tbl + " (a int, b int) stored as orc "
+ + "location '" + fakeS3Location(tbl) + "' "
+ + "tblproperties
('transactional'='true','transactional_properties'='insert_only')");
+
+ runQuery("insert into " + tbl + " values (1, 10), (2, 20), (3, 30)");
+
+ List<String> files = listFilesRelative(loc);
+ assertFalse(files.isEmpty(), "MM insert produced no files under " + loc);
+
+ // Every leaf must live under a delta_* directory (that's the MM
per-writeId
+ // uniqueness scope — the analogue of the per-query tag for the non-ACID
+ // cases). The leaf name itself is the writer's plain 000000_N.
+ for (String rel : files) {
+ assertTrue(rel.contains("/" + AcidUtils.DELTA_PREFIX),
+ "MM leaf must live under a delta_* subdir: " + rel);
+ String name = rel.substring(rel.lastIndexOf('/') + 1);
+ assertTrue(AcidUtils.ORIGINAL_PATTERN.matcher(name).matches(),
+ "MM leaf must be a plain writer name (000000_N): " + rel);
+ }
+
+ assertRowCount(tbl, 3);
+
+ convertToFullAcidAndAssertRowCount(tbl, 3);
+ }
+
+ /**
+ * Insert into a full ACID (transactional=true, default
transactional_properties)
+ * managed table on fakes3://. Same story as the MM case: MoveTask
short-circuits
+ * into the {@code delta_<writeId>_<writeId>_<stmt>/} layout without going
through
+ * {@link Hive#copyFiles}, so the per-query filename tag never fires.
Full-ACID
+ * leaves are named {@code bucket_NNNNN} (see {@link
AcidUtils#BUCKET_PATTERN})
+ * — the per-writeId delta directory is what keeps concurrent writers apart.
+ */
+ @Test
+ void testInsertIntoFullAcidOnFakeS3LandsUnderDeltaSubdir() throws Exception {
+ String tbl = "full_acid_fakes3";
+ Path loc = fakeS3Path(tbl);
+ runQuery(
+ "create table " + tbl + " (a int, b int) stored as orc "
+ + "location '" + fakeS3Location(tbl) + "' "
+ + "tblproperties ('transactional'='true')");
+
+ runQuery("insert into " + tbl + " values (1, 10), (2, 20), (3, 30)");
+
+ List<String> files = listFilesRelative(loc);
+ assertFalse(files.isEmpty(), "full-ACID insert produced no files under " +
loc);
+
+ // Every leaf must live under a delta_* directory and be a bucket_NNNNN
file.
+ // No _copy_ tag: full-ACID takes the isTransactional short-circuit in
+ // Hive.loadTable, bypassing pickDestFilePath entirely.
+ for (String rel : files) {
+ assertTrue(rel.contains("/" + AcidUtils.DELTA_PREFIX),
+ "full-ACID leaf must live under a delta_* subdir: " + rel);
+ String name = rel.substring(rel.lastIndexOf('/') + 1);
+ assertTrue(AcidUtils.BUCKET_PATTERN.matcher(name).matches(),
+ "full-ACID leaf must be a bucket_NNNNN name: " + rel);
+ }
+
+ assertRowCount(tbl, 3);
+ }
+
+ private void assertRowCount(String table, int count) throws Exception {
+ List<String> rows = runQuery("select count(*) from " + table);
+ assertEquals(String.valueOf(count), rows.getFirst(), "expected " + count +
" rows total");
+ }
+
+ private void convertToFullAcidAndAssertRowCount(String tbl, int
expectedRowCount) throws Exception {
+ runQuery("alter table " + tbl + " set tblproperties ('EXTERNAL'='FALSE')");
+ runQuery("alter table " + tbl + " set tblproperties
('transactional_properties'='default')");
+ runQuery("alter table " + tbl + " set tblproperties
('transactional'='true')");
+
+ assertRowCount(tbl, expectedRowCount);
+ }
+
+ /**
+ * Asserts that {@code name} is a copy-suffixed filename whose suffix is the
+ * 16-hex per-query uniqueness tag (as opposed to the numeric {@code _copy_N}
+ * fallback that {@link AcidUtils#ORIGINAL_PATTERN_COPY} also accepts).
Parses
+ * via {@link ParsedOutputFileName} so we track whatever it accepts.
+ */
+ private static void assertHas16HexUniquenessTag(String name) {
+ // Baseline: must match the copy pattern the readers accept.
+ assertTrue(AcidUtils.ORIGINAL_PATTERN_COPY.matcher(name).matches(),
+ "must match _copy_ pattern: " + name);
+ // Tighten: the suffix must be exactly 16 hex chars, not the numeric
fallback.
+ ParsedOutputFileName parsed = ParsedOutputFileName.parse(name);
+ assertTrue(parsed.matches(), "ParsedOutputFileName should recognize " +
name);
+ assertTrue(parsed.isCopyFile(), "expected copy suffix on " + name);
+ String tag = parsed.getCopyIndex();
+ assertEquals(16, tag.length(),
+ "copy suffix must be 16 chars (uniqueness tag, not numeric fallback),
got: " + tag + " (" + name + ")");
+ assertTrue(tag.matches("[0-9a-fA-F]{16}"),
+ "copy suffix must be lowercase-or-uppercase hex, got: " + tag + " (" +
name + ")");
+ }
+}
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/util/FakeS3FileSystem.java
b/ql/src/test/org/apache/hadoop/hive/ql/util/FakeS3FileSystem.java
new file mode 100644
index 00000000000..22a1b90dc46
--- /dev/null
+++ b/ql/src/test/org/apache/hadoop/hive/ql/util/FakeS3FileSystem.java
@@ -0,0 +1,101 @@
+/*
+ * 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.util;
+
+import java.io.IOException;
+import java.net.URI;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test-only {@link RawLocalFileSystem} wrapper that advertises itself under
the
+ * synthetic {@value #SCHEME} scheme. Files still live on the local disk
(backed
+ * by {@link RawLocalFileSystem}), so tests can inspect them with plain
+ * {@link java.io.File} calls, but every {@link Path} that Hive resolves via
+ * {@link FileSystem#getFileSystem(URI, Configuration)} sees {@code
fakes3://…}.
+ *
+ * <p>Tests using this class typically add {@value #SCHEME} to
+ * {@code org.apache.hadoop.hive.common.FileUtils.NON_ATOMIC_RENAME_SCHEMES} so
+ * {@code FileUtils.isNonAtomicRenameFs} treats it like S3A and drives the
+ * non-atomic-rename branch of the move logic. Remove it again in an
+ * {@code @AfterClass}/{@code @AfterAll} hook so no other test sees the
mutation.
+ *
+ * <p>Register with:
+ * <pre>
+ * conf.setClass("fs." + FakeS3FileSystem.SCHEME + ".impl",
+ * FakeS3FileSystem.class, FileSystem.class);
+ * conf.setBoolean("fs." + FakeS3FileSystem.SCHEME + ".impl.disable.cache",
true);
+ * </pre>
+ * Disabling the FS cache is important — otherwise a per-test {@code
TemporaryFolder}
+ * root will leak between tests through the cached FS instance.
+ */
+public final class FakeS3FileSystem extends RawLocalFileSystem {
+
+ /** URI scheme this FS advertises. */
+ public static final String SCHEME = "fakes3";
+
+ private URI uri;
+
+ @Override
+ public void initialize(URI name, Configuration conf) throws IOException {
+ super.initialize(name, conf);
+ String authority = name.getAuthority() == null ? "" : name.getAuthority();
+ this.uri = URI.create(SCHEME + "://" + authority + "/");
+ }
+
+ @Override
+ public String getScheme() {
+ return SCHEME;
+ }
+
+ @Override
+ public URI getUri() {
+ return uri != null ? uri : URI.create(SCHEME + ":///");
+ }
+
+ // RawLocalFileSystem's DeprecatedRawLocalFileStatus lazy-loads permissions
via
+ // new File(getPath().toUri())
+ // and File(URI) requires scheme=="file", so it throws on every
getPermission()
+ // call for our fakes3:// URIs. Replace the returned statuses with plain
+ // FileStatus objects whose permission field is populated at construction
time,
+ // so getPermission() is a simple field read that never hits the broken
loader.
+ private static FileStatus withPermission(FileStatus s) throws IOException {
+ return new FileStatus(s.getLen(), s.isDirectory(), s.getReplication(),
s.getBlockSize(),
+ s.getModificationTime(), s.getAccessTime(), new FsPermission((short)
0644),
+ "hive", "hive", s.isSymlink() ? s.getSymlink() : null, s.getPath());
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path f) throws IOException {
+ return withPermission(super.getFileStatus(f));
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path f) throws IOException {
+ FileStatus[] arr = super.listStatus(f);
+ for (int i = 0; i < arr.length; i++) {
+ arr[i] = withPermission(arr[i]);
+ }
+ return arr;
+ }
+}
diff --git a/ql/src/test/queries/clientpositive/acid_convert_16hex_copy_tag.q
b/ql/src/test/queries/clientpositive/acid_convert_16hex_copy_tag.q
new file mode 100644
index 00000000000..e9325c6f9a9
--- /dev/null
+++ b/ql/src/test/queries/clientpositive/acid_convert_16hex_copy_tag.q
@@ -0,0 +1,53 @@
+-- Convert a non-ACID managed ORC table to full ACID after renaming the
+-- inserted file to the 16-hex per-query "uniqueness tag" copy suffix that
+-- form introduced for unstable-rename filesystems (S3A/S3N/S3/GS).
+-- Before that change, the ORIGINAL_PATTERN_COPY regex in
+-- TransactionalValidationListener only matched `_copy_[0-9]+`, so the
+-- pre-existing file would be flagged as an "unexpected data file name
+-- format" and the ALTER TABLE ... transactional=true would fail. This
+-- test locks in the widened pattern.
+
+set hive.create.as.acid=false;
+set hive.create.as.insert.only=false;
+set hive.strict.managed.tables=false;
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.mapred.mode=nonstrict;
+
+drop table if exists t_acid_convert_16hex;
+
+create table t_acid_convert_16hex (id int, name string)
+ stored as orc
+ tblproperties ('transactional'='false');
+
+insert into t_acid_convert_16hex values (1, 'a'), (2, 'b');
+
+-- What the insert produced on a stable-rename FS (local test): expect a
+-- single `000000_0` (or similar numeric) file.
+dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/t_acid_convert_16hex;
+
+-- Rename it to the 16-hex form that Hive.mvFile would have chosen on e.g. S3A
+-- This is exactly the shape the widened
TransactionalValidationListener.ORIGINAL_PATTERN_COPY has to accept.
+dfs -mv ${hiveconf:hive.metastore.warehouse.dir}/t_acid_convert_16hex/000000_0
+
${hiveconf:hive.metastore.warehouse.dir}/t_acid_convert_16hex/000000_0_copy_f0796c02aef8435d;
+
+dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/t_acid_convert_16hex;
+
+-- The conversion. This is what would blow up with
+-- IllegalStateException: Unexpected data file name format.
+-- Cannot convert default.t_acid_convert_16hex to transactional table.
+-- if ORIGINAL_PATTERN_COPY still required a numeric copy index.
+alter table t_acid_convert_16hex set tblproperties ('transactional'='true',
'transactional_properties'='default');
+
+describe formatted t_acid_convert_16hex;
+
+-- Original rows still visible after conversion.
+select id, name from t_acid_convert_16hex order by id;
+
+-- Sanity: ACID-only ops now work end-to-end.
+update t_acid_convert_16hex set name = 'B' where id = 2;
+delete from t_acid_convert_16hex where id = 1;
+select id, name from t_acid_convert_16hex order by id;
+
+drop table t_acid_convert_16hex;
diff --git
a/ql/src/test/results/clientpositive/llap/acid_convert_16hex_copy_tag.q.out
b/ql/src/test/results/clientpositive/llap/acid_convert_16hex_copy_tag.q.out
new file mode 100644
index 00000000000..59f3732f19d
--- /dev/null
+++ b/ql/src/test/results/clientpositive/llap/acid_convert_16hex_copy_tag.q.out
@@ -0,0 +1,124 @@
+PREHOOK: query: drop table if exists t_acid_convert_16hex
+PREHOOK: type: DROPTABLE
+PREHOOK: Output: database:default
+POSTHOOK: query: drop table if exists t_acid_convert_16hex
+POSTHOOK: type: DROPTABLE
+POSTHOOK: Output: database:default
+PREHOOK: query: create table t_acid_convert_16hex (id int, name string)
+ stored as orc
+ tblproperties ('transactional'='false')
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: query: create table t_acid_convert_16hex (id int, name string)
+ stored as orc
+ tblproperties ('transactional'='false')
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@t_acid_convert_16hex
+PREHOOK: query: insert into t_acid_convert_16hex values (1, 'a'), (2, 'b')
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+PREHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: query: insert into t_acid_convert_16hex values (1, 'a'), (2, 'b')
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+POSTHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: Lineage: t_acid_convert_16hex.id SCRIPT []
+POSTHOOK: Lineage: t_acid_convert_16hex.name SCRIPT []
+Found 1 items
+#### A masked pattern was here ####
+Found 1 items
+#### A masked pattern was here ####
+PREHOOK: query: alter table t_acid_convert_16hex set tblproperties
('transactional'='true', 'transactional_properties'='default')
+PREHOOK: type: ALTERTABLE_PROPERTIES
+PREHOOK: Input: default@t_acid_convert_16hex
+PREHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: query: alter table t_acid_convert_16hex set tblproperties
('transactional'='true', 'transactional_properties'='default')
+POSTHOOK: type: ALTERTABLE_PROPERTIES
+POSTHOOK: Input: default@t_acid_convert_16hex
+POSTHOOK: Output: default@t_acid_convert_16hex
+PREHOOK: query: describe formatted t_acid_convert_16hex
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@t_acid_convert_16hex
+POSTHOOK: query: describe formatted t_acid_convert_16hex
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@t_acid_convert_16hex
+# col_name data_type comment
+id int
+name string
+
+# Detailed Table Information
+Database: default
+#### A masked pattern was here ####
+Retention: 0
+#### A masked pattern was here ####
+Table Type: MANAGED_TABLE
+Table Parameters:
+ bucketing_version 2
+#### A masked pattern was here ####
+ numFiles 1
+ numRows 2
+ rawDataSize 178
+ totalSize #Masked#
+ transactional true
+ transactional_properties default
+#### A masked pattern was here ####
+
+# Storage Information
+SerDe Library: org.apache.hadoop.hive.ql.io.orc.OrcSerde
+InputFormat: org.apache.hadoop.hive.ql.io.orc.OrcInputFormat
+OutputFormat: org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat
+Compressed: No
+Num Buckets: -1
+Bucket Columns: []
+Sort Columns: []
+PREHOOK: query: select id, name from t_acid_convert_16hex order by id
+PREHOOK: type: QUERY
+PREHOOK: Input: default@t_acid_convert_16hex
+#### A masked pattern was here ####
+POSTHOOK: query: select id, name from t_acid_convert_16hex order by id
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@t_acid_convert_16hex
+#### A masked pattern was here ####
+1 a
+2 b
+PREHOOK: query: update t_acid_convert_16hex set name = 'B' where id = 2
+PREHOOK: type: QUERY
+PREHOOK: Input: default@t_acid_convert_16hex
+PREHOOK: Output: default@t_acid_convert_16hex
+PREHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: query: update t_acid_convert_16hex set name = 'B' where id = 2
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@t_acid_convert_16hex
+POSTHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: Lineage: t_acid_convert_16hex.id SIMPLE []
+POSTHOOK: Lineage: t_acid_convert_16hex.name SIMPLE []
+PREHOOK: query: delete from t_acid_convert_16hex where id = 1
+PREHOOK: type: QUERY
+PREHOOK: Input: default@t_acid_convert_16hex
+PREHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: query: delete from t_acid_convert_16hex where id = 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@t_acid_convert_16hex
+POSTHOOK: Output: default@t_acid_convert_16hex
+PREHOOK: query: select id, name from t_acid_convert_16hex order by id
+PREHOOK: type: QUERY
+PREHOOK: Input: default@t_acid_convert_16hex
+#### A masked pattern was here ####
+POSTHOOK: query: select id, name from t_acid_convert_16hex order by id
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@t_acid_convert_16hex
+#### A masked pattern was here ####
+2 B
+PREHOOK: query: drop table t_acid_convert_16hex
+PREHOOK: type: DROPTABLE
+PREHOOK: Input: default@t_acid_convert_16hex
+PREHOOK: Output: database:default
+PREHOOK: Output: default@t_acid_convert_16hex
+POSTHOOK: query: drop table t_acid_convert_16hex
+POSTHOOK: type: DROPTABLE
+POSTHOOK: Input: default@t_acid_convert_16hex
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@t_acid_convert_16hex
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
index 37a10625ce5..44532570f53 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
@@ -448,8 +448,10 @@ private String validateTransactionalProperties(String
transactionalProperties) {
/**
* see org.apache.hadoop.hive.ql.exec.Utilities#COPY_KEYWORD
*/
+ // Copy suffix is either a numeric counter (HDFS/local: _copy_N) or a 16-hex
per-query
+ // uniqueness tag (non-atomic-rename FS such as S3A: _copy_<queryTag>).
private static final Pattern ORIGINAL_PATTERN_COPY =
- Pattern.compile("[0-9]+_[0-9]+" + "_copy_" + "[0-9]+");
+ Pattern.compile("[0-9]+_[0-9]+" + "_copy_" +
"(?:[0-9]{1,6}|[0-9a-fA-F]{16})");
/**
* It's assumed everywhere that original data files are named according to
