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 6c47d3bc1693..1b20778e6ab5 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java @@ -311,6 +311,18 @@ public static String makeQueryId() { + UUID.randomUUID().toString(); } + /** + * 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 713c7e56f848..45257d31f50d 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 @@ -31,8 +31,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 *

* All the components are here: * tmp_(taskPrefix)00001_02_copy_1.zlib.gz @@ -41,9 +44,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}))?" + // _ (limited to 6 digits) - "(?:_copy_([0-9]{1,6}))?" + // copy file index + "(\\d+)" + // taskId + "(?:_(\\d{1,6}))?" + // _ (limited to 6 digits) + "(?:_copy_(\\d{1,6}|[\\da-fA-F]{16}))?" + // copy suffix: numeric counter, or 16-hex uniqueness tag "(\\..*)?$"); // any suffix/file extension public static ParsedOutputFileName parse(String fileName) { @@ -108,6 +111,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 47877ed97a1d..4f7265ae92ce 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 @@ -240,9 +240,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 @@ -450,8 +454,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 = (copySuffix.length() == 16) ? 0 : Integer.parseInt(copySuffix); 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 d7eb7281eccc..bc31a918c302 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 @@ -21,6 +21,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; @@ -163,6 +164,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; @@ -272,6 +274,23 @@ public class Hive implements AutoCloseable { static final private Logger LOG = LoggerFactory.getLogger("hive.ql.metadata.Hive"); private final String CLASS_NAME = Hive.class.getName(); + /** + * 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 + * in {@link #mvFile}. The list is in code because the set of unsafe filesystems is a property + * of the filesystem implementation, not something an operator should override. + *

+ * 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 {@code mvFile} 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 NON_ATOMIC_RENAME_SCHEMES = new HashSet<>( + Arrays.asList("s3a", "s3n", "s3", "gs", "abfs", "abfss", "wasb", "wasbs")); + private HiveConf conf = null; private IMetaStoreClient metaStoreClient; private UserGroupInformation owner; @@ -5170,6 +5189,110 @@ private static String getPathName(int taskId) { return Utilities.replaceTaskId("000000", taskId) + "_0"; } + /** + * Compute a compact per-query uniqueness tag used by the non-ACID rename branch of + * {@link #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_}) in place of the numeric {@code _copy_N} counter. + *

+ * 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. + */ + static String computeUniquenessTag(HiveConf conf) { + String qid = HiveConf.getVar(conf, 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); + } + + /** + * @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). + */ + 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()); + } + + /** + * 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. + * + *

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. + * + *

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() + && isNonAtomicRenameFs(destFs)) + ? computeUniquenessTag(conf) + : null; + + if (uniqueCopySuffix != null && !uniqueCopySuffix.isEmpty()) { + // Unstable-rename FS: use `name_copy_` 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; + } + /** *

* Moves a file from one {@link Path} to another. If {@code isRenameAllowed} is true then the @@ -5199,37 +5322,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); @@ -5241,7 +5335,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."); } @@ -5249,10 +5343,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 e09a5ecc3c33..ac222dc224b5 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 @@ -120,6 +120,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 2ef7bfcbccdd..e05b14bcd885 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 @@ -28,14 +28,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) @@ -158,8 +163,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, @@ -186,8 +191,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, @@ -227,4 +232,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_} names — no plain {@code 000000_0}, no + * numeric {@code _copy_N}. + * + *

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: + *

    + *
  1. {@link Hive#isNonAtomicRenameFs(FileSystem)} recognizes S3-family schemes on the URI + * and rejects HDFS / local schemes.
    2. + *
  2. 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.
    3. + *
+ */ + @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", Hive.isNonAtomicRenameFs(localFs)); + assertFalse("null fs is not flagged", Hive.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", + Hive.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", + Hive.isNonAtomicRenameFs(spy)); + } + + // (2) uniqueness tag: the 16-hex most-significant-bits half of the UUID at the tail of + // queryId (QueryPlan.makeQueryId → "__"; see + // QueryPlan.extractUniquenessTag). Distinct UUIDs → distinct tags. + hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID, + "lbodor_20260101120000_f47ac10b-58cc-4372-a567-0e02b2c3d479"); + String tag1 = Hive.computeUniquenessTag(hiveConf); + hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID, + "lbodor_20260101120001_9c8a44f1-e2b3-4a1c-9d3e-000000000000"); + String tag2 = Hive.computeUniquenessTag(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 { + Hive.computeUniquenessTag(hiveConf); + fail("computeUniquenessTag 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 000000000000..b1dbf57d8ea3 --- /dev/null +++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFilesFakeS3.java @@ -0,0 +1,298 @@ +/* + * 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.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 Hive#NON_ATOMIC_RENAME_SCHEMES} for the duration of + * this test class so {@link Hive#isNonAtomicRenameFs} treats it as a non-atomic-rename + * FS. The scheme is removed in {@link #tearDownClass()} so no other test sees it. + * + *

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 Hive.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. + Hive.NON_ATOMIC_RENAME_SCHEMES.add(FAKE_SCHEME); + SessionState.start(hiveConf); + } + + @AfterClass + public static void tearDownClass() { + Hive.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 Hive#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", Hive.isNonAtomicRenameFs(fakeFs)); + + FileSystem localFs = new Path(tmp.getRoot().getAbsolutePath()).getFileSystem(hiveConf); + assertFalse("local FS must not be flagged", Hive.isNonAtomicRenameFs(localFs)); + } + + /** + * A single-file rename into a fresh destination under {@code fakes3://} must land at + * {@code _copy_<16-hex>} (uniqueness-tag suffix), not at {@code } — 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: + *

    + *
  1. Exactly 10 files end up in the destination (nothing was silently + * overwritten on rename).
    2. + *
  2. Every filename matches {@link AcidUtils#ORIGINAL_PATTERN_COPY} + * (the {@code _copy_} shape) and none matches + * {@link AcidUtils#ORIGINAL_PATTERN} (no plain {@code 000000_N}).
    3. + *
  3. All 10 filenames are distinct.
    4. + *
+ * 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> 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 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 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 000000000000..53c5a06c3cfe --- /dev/null +++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestInsertCopySuffixOnFakeS3.java @@ -0,0 +1,441 @@ +/* + * 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.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. + * + *

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 Hive#NON_ATOMIC_RENAME_SCHEMES} + * so {@link Hive#isNonAtomicRenameFs} treats it like S3A. + * + *

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() { + Hive.NON_ATOMIC_RENAME_SCHEMES.add(FAKE_SCHEME); + } + + @AfterAll + static void removeFakeSchemeFromUnstableSet() { + Hive.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 runQuery(String stmt) throws Exception { + hiveConf.setVar(HiveConf.ConfVars.HIVE_QUERY_ID, org.apache.hadoop.hive.ql.QueryPlan.makeQueryId()); + d.run(stmt); + List 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 listFilesRelative(Path root) throws IOException { + FileSystem fs = root.getFileSystem(hiveConf); + List paths = new ArrayList<>(); + if (!fs.exists(root)) { + return paths; + } + RemoteIterator 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 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_/ 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: + *

    + *
  1. Some files were produced (union insert didn't silently no-op).
    2. + *
  2. If {@code partitionSegment} is non-null, every leaf's path contains + * {@code //} (e.g. {@code /b=2/}).
    3. + *
  3. 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).
    4. + *
  4. Every leaf name is a plain writer name (matches + * {@link AcidUtils#ORIGINAL_PATTERN}) with NO {@code _copy_} 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_/ separation is the anti-collision + * mechanism here.
    5. + *
+ */ + private void assertUnionSubdirLayoutAt(Path loc, String partitionSegment) throws IOException { + List files = listFilesRelative(loc); + assertFalse(files.isEmpty(), "union-all insert produced no files under " + loc); + + Set 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_/ 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_ 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_/: " + 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___/} 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. + * + *

Two concurrent MM inserts would get separate {@code delta_} 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 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___/} 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 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 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 000000000000..7948f8728368 --- /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://…}. + * + *

Tests using this class typically add {@value #SCHEME} to + * {@code org.apache.hadoop.hive.ql.metadata.Hive.NON_ATOMIC_RENAME_SCHEMES} so + * {@code Hive.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. + * + *

Register with: + * 

+ *   conf.setClass("fs." + FakeS3FileSystem.SCHEME + ".impl",
+ *                 FakeS3FileSystem.class, FileSystem.class);
+ *   conf.setBoolean("fs." + FakeS3FileSystem.SCHEME + ".impl.disable.cache", true);
+ *
+ * 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 000000000000..e9325c6f9a9b --- /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 000000000000..59f3732f19d0 --- /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 16404a9ca9c8..11dc4b75f76e 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 @@ -447,8 +447,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_). 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