diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java index 9248897bf745..03935bcf53c5 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java @@ -341,7 +341,8 @@ private VectorizedOrcAcidRowBatchReader(JobConf conf, OrcSplit orcSplit, Reporte if (deleteEventRegistry.isEmpty() && !rowIdProjected) { Path parent = orcSplit.getPath().getParent(); while (parent != null && !rootPath.equals(parent)) { - if (parent.getName().startsWith(AcidUtils.BASE_PREFIX)) { + String parentName = parent.getName(); + if (parentName.startsWith(AcidUtils.BASE_PREFIX)) { /** * The assumption here is that any base_x is filtered out by * {@link AcidUtils#getAcidState(Path, Configuration, ValidWriteIdList)} @@ -351,7 +352,8 @@ private VectorizedOrcAcidRowBatchReader(JobConf conf, OrcSplit orcSplit, Reporte */ readerOptions.includeAcidColumns(false); break; - } else { + } else if (parentName.startsWith(AcidUtils.DELTA_PREFIX) + || parentName.startsWith(AcidUtils.DELETE_DELTA_PREFIX)) { ParsedDeltaLight pd = ParsedDeltaLight.parse(parent); if (validWriteIdList.isWriteIdRangeValid(pd.getMinWriteId(), pd.getMaxWriteId()) == ValidWriteIdList.RangeResponse.ALL) { @@ -361,6 +363,11 @@ private VectorizedOrcAcidRowBatchReader(JobConf conf, OrcSplit orcSplit, Reporte break; } } + // Any other directory name (e.g. HIVE_UNION_SUBDIR_/ produced by + // an INSERT ... UNION ALL against a table that was later converted + // to full ACID) is not an ACID container — skip it and keep walking + // up. Feeding such a name into ParsedDeltaLight.parse would throw + // NumberFormatException. parent = parent.getParent(); } } diff --git a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestUnionAllToAcidConversion.java b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestUnionAllToAcidConversion.java new file mode 100644 index 000000000000..4681cd22c770 --- /dev/null +++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestUnionAllToAcidConversion.java @@ -0,0 +1,446 @@ +/* + * 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.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.ql.TxnCommandsBaseForTests; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Covers the non-ACID→full-ACID conversion of a table whose pre-conversion + * data was written by {@code INSERT ... UNION ALL ...} — i.e. the branches + * materialize under {@code HIVE_UNION_SUBDIR_/000000_0}. + * + *

Both DDL paths — the dedicated {@code ALTER TABLE ... CONVERT TO ACID} + * and the historical {@code ALTER TABLE ... SET TBLPROPERTIES + * ('transactional'='true')} — are metadata-only flips: they preserve the + * pre-conversion on-disk layout, including the {@code HIVE_UNION_SUBDIR_/} + * subdirs. Existing callers depend on this (see + * {@code TestTxnNoBuckets.testToAcidConversionMultiBucket}, which verifies + * both the preserved layout and specific ROW__ID assignments encoded per + * subdir). A subsequent {@code SELECT} must therefore read the pre-conversion + * layout cleanly — which requires the ACID reader's parent-walk to skip + * non-delta directory names rather than feeding them to + * {@code ParsedDeltaLight.parse}. That reader-side guard is what these tests + * exercise end-to-end. + */ +class TestUnionAllToAcidConversion extends TxnCommandsBaseForTests { + + private static final String TEST_DATA_DIR = new File(System.getProperty("java.io.tmpdir") + + File.separator + TestUnionAllToAcidConversion.class.getCanonicalName() + + "-" + System.currentTimeMillis()).getPath().replaceAll("\\\\", "/"); + + @Override + protected String getTestDataDir() { + return TEST_DATA_DIR; + } + + @Override + protected void initHiveConf() { + super.initHiveConf(); + 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); + // Keep the UNION-ALL branches in their own HIVE_UNION_SUBDIR_/ intermediate + // directories so the conversion sees the multi-level layout. + HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_TEZ_UNION_FLATTEN_SUBDIRECTORIES, false); + } + + @Override + protected void setUpSchema() { + // Skip the parent's ACID/bucketed fixture tables — each test creates its own. + } + + @Override + protected void dropTables() { + // Match setUpSchema override: nothing to drop for the parent's fixtures. + } + + @AfterEach + public void dropAllTestTables() throws Exception { + for (String t : new String[] { + "union_all_repro", + "union_all_repro_setprops", + "union_all_repro_part", + "union_all_repro_part_setprops", + "union_src"}) { + try { + runQuery("drop table if exists " + t); + } catch (Exception ignore) { + // don't let a residual-drop failure hide the real test failure + } + } + } + + private void insertUnionAllInto(String tbl) throws Exception { + // Real staging table + per-branch group-by, so the planner can't + // constant-fold the branches into one mapper. This is what surfaces the + // HIVE_UNION_SUBDIR_/ layout at final-move time. + runQuery("create table if not exists union_src (k int, v int) stored as orc " + + "tblproperties ('transactional'='false')"); + runQuery("insert into union_src values (1, 10), (2, 20), (3, 30)"); + + 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"); + } + + /** + * Same 3-way UNION ALL pattern as {@link #insertUnionAllInto(String)}, but + * targeted at a static partition {@code p='X'} of a partitioned table. + */ + private void insertUnionAllIntoPartition(String tbl, String partValue) throws Exception { + runQuery("create table if not exists union_src (k int, v int) stored as orc " + + "tblproperties ('transactional'='false')"); + runQuery("insert into union_src values (1, 10), (2, 20), (3, 30)"); + + runQuery( + "insert into " + tbl + " partition (p='" + partValue + "') " + + "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"); + } + + /** + * Return the table's on-disk layout as a sorted list of warehouse-relative + * paths (i.e. including the table directory name as the top-level segment, + * one level higher than the table root itself), so tests can assert against + * an exact expected literal. + */ + private List layoutOf(String tbl) throws Exception { + org.apache.hadoop.fs.Path loc = new org.apache.hadoop.fs.Path( + hiveConf.get("hive.metastore.warehouse.dir") + "/" + tbl); + org.apache.hadoop.fs.FileSystem fs = loc.getFileSystem(hiveConf); + List paths = new ArrayList<>(); + if (fs.exists(loc)) { + org.apache.hadoop.fs.RemoteIterator it = fs.listFiles(loc, true); + // Strip the *parent* of the table location (the warehouse dir), so the + // returned paths start with "//…" — one level higher than + // just the table root. + String warehousePath = loc.getParent().toUri().getPath(); + while (it.hasNext()) { + org.apache.hadoop.fs.LocatedFileStatus s = it.next(); + String full = s.getPath().toUri().getPath(); + paths.add(full.startsWith(warehousePath) ? full.substring(warehousePath.length()) : full); + } + } + Collections.sort(paths); + return paths; + } + + 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; + } + + /** + * {@code ALTER TABLE ... CONVERT TO ACID} on a table previously loaded via + * UNION ALL is a metadata-only flip: the on-disk layout is unchanged, and + * the subsequent SELECT returns the expected row count. + */ + @Test + void testUnionAllInsertThenConvertToAcid() throws Exception { + String tbl = "union_all_repro"; + runQuery("create table " + tbl + " (a int, b int) stored as orc " + + "tblproperties ('transactional'='false')"); + + insertUnionAllInto(tbl); + + // The UNION-ALL insert materializes each branch under its own + // HIVE_UNION_SUBDIR_/000000_0. Assert the exact layout so a reader of + // this test can see what CONVERT TO ACID is going to be run against. + List expectedLayout = List.of( + "/union_all_repro/HIVE_UNION_SUBDIR_1/000000_0", + "/union_all_repro/HIVE_UNION_SUBDIR_2/000000_0", + "/union_all_repro/HIVE_UNION_SUBDIR_3/000000_0"); + List beforeLayout = layoutOf(tbl); + assertEquals(expectedLayout, beforeLayout, "pre-conversion layout"); + + runQuery("alter table " + tbl + " convert to acid"); + + // CONVERT TO ACID is a metadata-only flip: the on-disk layout is unchanged. + List afterLayout = layoutOf(tbl); + assertEquals(expectedLayout, afterLayout, + "CONVERT TO ACID should be a metadata-only flip and preserve the pre-conversion layout"); + + List rows = runQuery("select count(*) from " + tbl); + assertEquals("3", rows.getFirst(), "expected 3 rows after UNION-ALL insert + CONVERT TO ACID"); + } + + /** + * {@code ALTER TABLE ... SET TBLPROPERTIES ('transactional'='true')} — the + * path {@code UpgradeTool} emits — is likewise a metadata-only flip. The + * subsequent SELECT returns the expected row count. + */ + @Test + void testUnionAllInsertThenSetTblpropertiesAcid() throws Exception { + String tbl = "union_all_repro_setprops"; + runQuery("create table " + tbl + " (a int, b int) stored as orc " + + "tblproperties ('transactional'='false')"); + + insertUnionAllInto(tbl); + + // Same shape of pre-conversion layout as the CONVERT TO ACID case above, + // rooted under this test's own table directory. + List expectedLayout = List.of( + "/union_all_repro_setprops/HIVE_UNION_SUBDIR_1/000000_0", + "/union_all_repro_setprops/HIVE_UNION_SUBDIR_2/000000_0", + "/union_all_repro_setprops/HIVE_UNION_SUBDIR_3/000000_0"); + List beforeLayout = layoutOf(tbl); + assertEquals(expectedLayout, beforeLayout, "pre-conversion layout"); + + runQuery("alter table " + tbl + " set tblproperties ('transactional'='true')"); + + List afterLayout = layoutOf(tbl); + assertEquals(expectedLayout, afterLayout, + "SET TBLPROPERTIES ('transactional'='true') should preserve the pre-conversion layout"); + + List rows = runQuery("select count(*) from " + tbl); + assertEquals("3", rows.getFirst(), + "expected 3 rows after UNION-ALL insert + SET TBLPROPERTIES ACID conversion"); + } + + /** + * Partitioned-table variant of {@link #testUnionAllInsertThenConvertToAcid()} + * — the UNION-ALL insert materializes each branch under {@code p=x/HIVE_UNION_SUBDIR_/000000_0} + * inside the partition directory. CONVERT TO ACID is still a metadata-only + * flip, and the subsequent SELECT returns the expected row count. + */ + @Test + void testPartitionedUnionAllInsertThenConvertToAcid() throws Exception { + String tbl = "union_all_repro_part"; + runQuery("create table " + tbl + " (a int, b int) partitioned by (p string) " + + "stored as orc tblproperties ('transactional'='false')"); + + insertUnionAllIntoPartition(tbl, "x"); + + List expectedLayout = List.of( + "/union_all_repro_part/p=x/HIVE_UNION_SUBDIR_1/000000_0", + "/union_all_repro_part/p=x/HIVE_UNION_SUBDIR_2/000000_0", + "/union_all_repro_part/p=x/HIVE_UNION_SUBDIR_3/000000_0"); + List beforeLayout = layoutOf(tbl); + assertEquals(expectedLayout, beforeLayout, "pre-conversion layout"); + + runQuery("alter table " + tbl + " convert to acid"); + + List afterLayout = layoutOf(tbl); + assertEquals(expectedLayout, afterLayout, + "CONVERT TO ACID should be a metadata-only flip and preserve the pre-conversion layout"); + + List rows = runQuery("select count(*) from " + tbl); + assertEquals("3", rows.getFirst(), + "expected 3 rows after partitioned UNION-ALL insert + CONVERT TO ACID"); + } + + /** + * Partitioned-table variant of + * {@link #testUnionAllInsertThenSetTblpropertiesAcid()} — the + * {@code UpgradeTool}-style path preserves the pre-conversion partition + * layout, and the subsequent SELECT returns the expected row count. + */ + @Test + void testPartitionedUnionAllInsertThenSetTblpropertiesAcid() throws Exception { + String tbl = "union_all_repro_part_setprops"; + runQuery("create table " + tbl + " (a int, b int) partitioned by (p string) " + + "stored as orc tblproperties ('transactional'='false')"); + + insertUnionAllIntoPartition(tbl, "x"); + + List expectedLayout = List.of( + "/union_all_repro_part_setprops/p=x/HIVE_UNION_SUBDIR_1/000000_0", + "/union_all_repro_part_setprops/p=x/HIVE_UNION_SUBDIR_2/000000_0", + "/union_all_repro_part_setprops/p=x/HIVE_UNION_SUBDIR_3/000000_0"); + List beforeLayout = layoutOf(tbl); + assertEquals(expectedLayout, beforeLayout, "pre-conversion layout"); + + runQuery("alter table " + tbl + " set tblproperties ('transactional'='true')"); + + List afterLayout = layoutOf(tbl); + assertEquals(expectedLayout, afterLayout, + "SET TBLPROPERTIES ('transactional'='true') should preserve the pre-conversion layout"); + + List rows = runQuery("select count(*) from " + tbl); + assertEquals("3", rows.getFirst(), + "expected 3 rows after partitioned UNION-ALL insert + SET TBLPROPERTIES ACID conversion"); + } + + // --------------------------------------------------------------------------- + // flatten=true variants + // + // hive.tez.union.flatten.subdirectories=true asks MoveTask to hoist the + // HIVE_UNION_SUBDIR_/000000_0 leaves into the target directory at write + // time. MoveTask.flattenUnionSubdirectories folds the source subdir index + // into the filename to avoid collisions, producing _000000_0 (e.g. + // "1_000000_0"). That name has three numeric parts / two underscores and + // therefore does NOT match the metastore's ORIGINAL_PATTERN ([0-9]+_[0-9]+) + // — so a subsequent ACID conversion (either CONVERT TO ACID or the + // SET TBLPROPERTIES flip) is rejected by + // TransactionalValidationListener.validateTableStructureForPath. + // + // These four tests pin that current behavior: pre-conversion the layout is + // _000000_0 files (partitioned or not), and the conversion attempt + // fails at metastore-side validation. When the flatten filename scheme is + // aligned with ORIGINAL_PATTERN (or the validator is relaxed for this + // shape), these tests should be updated to assert successful conversion + + // 3-row read-back. + // --------------------------------------------------------------------------- + + /** + * Enable the write-time flatten and re-run the given block. Restores the + * previous value on exit. + */ + private void withUnionFlattenAtWriteTime(ThrowingRunnable body) throws Exception { + boolean previous = HiveConf.getBoolVar(hiveConf, HiveConf.ConfVars.HIVE_TEZ_UNION_FLATTEN_SUBDIRECTORIES); + HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_TEZ_UNION_FLATTEN_SUBDIRECTORIES, true); + try { + body.run(); + } finally { + HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_TEZ_UNION_FLATTEN_SUBDIRECTORIES, previous); + } + } + + @FunctionalInterface + private interface ThrowingRunnable { + void run() throws Exception; + } + + /** + * flatten=true + unpartitioned + CONVERT TO ACID. Pins the current + * behavior: MoveTask hoists the union-subdir leaves into + * {@code _000000_0} files at the table root, and the subsequent + * CONVERT TO ACID is rejected by the metastore's file-name validator. + */ + @Test + void testUnionAllInsertWithFlattenThenConvertToAcid() throws Exception { + String tbl = "union_all_repro"; + withUnionFlattenAtWriteTime(() -> { + runQuery("create table " + tbl + " (a int, b int) stored as orc " + + "tblproperties ('transactional'='false')"); + + insertUnionAllInto(tbl); + + List expectedLayout = List.of( + "/union_all_repro/1_000000_0", + "/union_all_repro/2_000000_0", + "/union_all_repro/3_000000_0"); + assertEquals(expectedLayout, layoutOf(tbl), "pre-conversion layout (write-time flatten on)"); + + Exception thrown = assertThrows(Exception.class, + () -> runQuery("alter table " + tbl + " convert to acid"), + "flatten=true names don't match ORIGINAL_PATTERN → metastore rejects the conversion"); + assertTrue(thrown.getMessage().contains("Unexpected data file name format"), + "expected metastore-side name-validation failure, got: " + thrown.getMessage()); + }); + } + + /** + * flatten=true + unpartitioned + SET TBLPROPERTIES ACID conversion. Same + * failure as the CONVERT TO ACID variant: the metastore validator rejects + * the {@code _000000_0} filenames. + */ + @Test + void testUnionAllInsertWithFlattenThenSetTblpropertiesAcid() throws Exception { + String tbl = "union_all_repro_setprops"; + withUnionFlattenAtWriteTime(() -> { + runQuery("create table " + tbl + " (a int, b int) stored as orc " + + "tblproperties ('transactional'='false')"); + + insertUnionAllInto(tbl); + + List expectedLayout = List.of( + "/union_all_repro_setprops/1_000000_0", + "/union_all_repro_setprops/2_000000_0", + "/union_all_repro_setprops/3_000000_0"); + assertEquals(expectedLayout, layoutOf(tbl), "pre-conversion layout (write-time flatten on)"); + + Exception thrown = assertThrows(Exception.class, + () -> runQuery("alter table " + tbl + " set tblproperties ('transactional'='true')"), + "flatten=true names don't match ORIGINAL_PATTERN → metastore rejects the conversion"); + assertTrue(thrown.getMessage().contains("Unexpected data file name format"), + "expected metastore-side name-validation failure, got: " + thrown.getMessage()); + }); + } + + /** + * flatten=true + partitioned + CONVERT TO ACID. Same failure as the + * unpartitioned variant, inside the partition directory. + */ + @Test + void testPartitionedUnionAllInsertWithFlattenThenConvertToAcid() throws Exception { + String tbl = "union_all_repro_part"; + withUnionFlattenAtWriteTime(() -> { + runQuery("create table " + tbl + " (a int, b int) partitioned by (p string) " + + "stored as orc tblproperties ('transactional'='false')"); + + insertUnionAllIntoPartition(tbl, "x"); + + List expectedLayout = List.of( + "/union_all_repro_part/p=x/1_000000_0", + "/union_all_repro_part/p=x/2_000000_0", + "/union_all_repro_part/p=x/3_000000_0"); + assertEquals(expectedLayout, layoutOf(tbl), "pre-conversion layout (write-time flatten on)"); + + Exception thrown = assertThrows(Exception.class, + () -> runQuery("alter table " + tbl + " convert to acid"), + "flatten=true names don't match ORIGINAL_PATTERN → metastore rejects the conversion"); + assertTrue(thrown.getMessage().contains("Unexpected data file name format"), + "expected metastore-side name-validation failure, got: " + thrown.getMessage()); + }); + } + + /** + * flatten=true + partitioned + SET TBLPROPERTIES ACID conversion. + */ + @Test + void testPartitionedUnionAllInsertWithFlattenThenSetTblpropertiesAcid() throws Exception { + String tbl = "union_all_repro_part_setprops"; + withUnionFlattenAtWriteTime(() -> { + runQuery("create table " + tbl + " (a int, b int) partitioned by (p string) " + + "stored as orc tblproperties ('transactional'='false')"); + + insertUnionAllIntoPartition(tbl, "x"); + + List expectedLayout = List.of( + "/union_all_repro_part_setprops/p=x/1_000000_0", + "/union_all_repro_part_setprops/p=x/2_000000_0", + "/union_all_repro_part_setprops/p=x/3_000000_0"); + assertEquals(expectedLayout, layoutOf(tbl), "pre-conversion layout (write-time flatten on)"); + + Exception thrown = assertThrows(Exception.class, + () -> runQuery("alter table " + tbl + " set tblproperties ('transactional'='true')"), + "flatten=true names don't match ORIGINAL_PATTERN → metastore rejects the conversion"); + assertTrue(thrown.getMessage().contains("Unexpected data file name format"), + "expected metastore-side name-validation failure, got: " + thrown.getMessage()); + }); + } +}