From 3645aba3febc8e861b8605b3fbd7972fa5076448 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Thu, 19 Mar 2026 18:20:02 -0700 Subject: [PATCH 01/10] HIVE-29517: Make schema upgrade scripts idempotent and rerunnable --- .../tools/schematool/DbErrorCodes.java | 145 ++++++++++++++++ .../tools/schematool/HiveSchemaHelper.java | 114 ++++++++----- .../schematool/IdempotentDDLExecutor.java | 86 ++++++++++ .../tools/schematool/MetastoreSchemaTool.java | 41 ++--- .../schematool/SchemaToolCommandLine.java | 4 +- .../TestSchemaToolForMetastore.java | 156 ++++++++++++++++++ 6 files changed, 475 insertions(+), 71 deletions(-) create mode 100644 standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java create mode 100644 standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java new file mode 100644 index 000000000000..6285795ab2c4 --- /dev/null +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java @@ -0,0 +1,145 @@ +/* + * 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.metastore.tools.schematool; + +import com.google.common.collect.ImmutableSet; + +import java.sql.SQLException; +import java.util.Set; + +/** + * Per-database ignorable DDL error codes for idempotent schema upgrades. + * This is used by {@link IdempotentDDLExecutor} to determine whether a given {@link SQLException} + * can be safely ignored (e.g. because it indicates an object already exists or is already gone). + *

+ * Use {@link #forDbType(String)} to get the right instance. + */ +public interface DbErrorCodes { + + /** @return true if the exception can be safely ignored (object already exists or already gone). */ + boolean isIgnorable(SQLException e); + + /** + * Checks both {@link SQLException#getSQLState()} and {@code String.valueOf(getErrorCode())} + * against {@code duplicateCodes} and {@code missingCodes}. + */ + class Codes implements DbErrorCodes { + private final Set duplicateCodes; + private final Set missingCodes; + + Codes(Set duplicateCodes, Set missingCodes) { + this.duplicateCodes = duplicateCodes; + this.missingCodes = missingCodes; + } + + @Override + public boolean isIgnorable(SQLException e) { + String state = e.getSQLState(); + String code = String.valueOf(e.getErrorCode()); + return duplicateCodes.contains(state) || duplicateCodes.contains(code) + || missingCodes.contains(state) || missingCodes.contains(code); + } + } + + DbErrorCodes POSTGRES = new Codes( + ImmutableSet.of( + "42P07", // duplicate table + "42701", // duplicate column + "42710" // duplicate object (e.g. constraint, index) + ), + ImmutableSet.of( + "42P01", // undefined table + "42703", // undefined column + "42704" // undefined object + ) + ); + + DbErrorCodes DERBY = new Codes( + ImmutableSet.of( + "X0Y32", // table/view already exists + "X0Y68", // index already exists + "42Z93" // duplicate constraint (same column set already constrained) + ), + ImmutableSet.of( + "42Y55", // table/view does not exist + "42X14", // column does not exist + "42X65", // index does not exist + "42X86" // constraint does not exist on table (ALTER TABLE DROP CONSTRAINT) + ) + ); + + DbErrorCodes MYSQL = new Codes( + ImmutableSet.of( + "1050", // ER_TABLE_EXISTS_ERROR: table already exists + "1060", // ER_DUP_FIELDNAME: duplicate column name + "1061" // ER_DUP_KEYNAME: duplicate key name + ), + ImmutableSet.of( + "1051", // ER_BAD_TABLE_ERROR: unknown table (DROP TABLE) + "1054", // ER_BAD_FIELD_ERROR: unknown column (CHANGE COLUMN on already-renamed column) + "1091" // ER_CANT_DROP_FIELD_OR_KEY: column or index does not exist (DROP COLUMN/INDEX) + ) + ); + + DbErrorCodes ORACLE = new Codes( + ImmutableSet.of( + "955", // ORA-00955: name already used by an existing object + "957", // ORA-00957: duplicate column name (RENAME COLUMN target already exists) + "1430", // ORA-01430: column being added already exists in table + "2261" // ORA-02261: unique or primary key already exists in the table + ), + ImmutableSet.of( + "942", // ORA-00942: table or view does not exist + "904", // ORA-00904: invalid identifier (column does not exist) + "1418", // ORA-01418: specified index does not exist + "2443" // ORA-02443: cannot drop constraint - nonexistent constraint + ) + ); + + DbErrorCodes MSSQL = new Codes( + ImmutableSet.of( + "2714", // There is already an object named '...' in the database + "2705", // Column names in each table must be unique (duplicate column) + "1913" // There is already an index named '...' on table '...' + ), + ImmutableSet.of( + "3701", // Cannot drop object because it does not exist or you do not have permission + "3728", // DROP CONSTRAINT: not a constraint + "4924", // ALTER TABLE DROP COLUMN: column does not exist + "15248" // sp_rename: parameter @objname is ambiguous or @objtype is wrong (column already renamed) + ) + ); + + /** No-op instance for unrecognized database types; never ignores any error. */ + DbErrorCodes NOOP = e -> false; + + /** Returns the {@link DbErrorCodes} for the given db-type string, or {@link #NOOP} if unrecognized. */ + static DbErrorCodes forDbType(String dbType) { + if (dbType == null) { + return NOOP; + } + return switch (dbType.toLowerCase()) { + case "postgres" -> POSTGRES; + case "derby", "derby.clean" -> DERBY; + case "mysql", "mariadb" -> MYSQL; + case "oracle" -> ORACLE; + case "mssql" -> MSSQL; + default -> NOOP; + }; + } +} diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java index 47000cc41b20..542ea5a2f3fc 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java @@ -42,7 +42,7 @@ public class HiveSchemaHelper { public static final String DB_HIVE = "hive"; public static final String DB_MSSQL = "mssql"; public static final String DB_MYSQL = "mysql"; - public static final String DB_POSTGRACE = "postgres"; + public static final String DB_POSTGRES = "postgres"; public static final String DB_ORACLE = "oracle"; public static final String EMBEDDED_HS2_URL = "jdbc:hive2://?hive.conf.restricted.list=;hive.security.authorization.sqlstd.confwhitelist=.*;" @@ -186,6 +186,18 @@ String buildCommand(String scriptDir, String scriptFile) */ String buildCommand(String scriptDir, String scriptFile, boolean fixQuotes) throws IllegalFormatException, IOException; + + /** + * Parse the script and return a list of individual, executable SQL commands. + */ + List getExecutableCommands(String scriptDir, String scriptFile) + throws IllegalFormatException, IOException; + + /** + * Parse the script and return a list of individual, executable SQL commands. + */ + List getExecutableCommands(String scriptDir, String scriptFile, boolean fixQuotes) + throws IllegalFormatException, IOException; } /** @@ -254,58 +266,74 @@ public boolean needsQuotedIdentifier() { } @Override - public String buildCommand( - String scriptDir, String scriptFile) throws IllegalFormatException, IOException { - return buildCommand(scriptDir, scriptFile, false); + public List getExecutableCommands(String scriptDir, String scriptFile) throws IllegalFormatException, IOException { + return getExecutableCommands(scriptDir, scriptFile, false); } @Override - public String buildCommand( - String scriptDir, String scriptFile, boolean fixQuotes) throws IllegalFormatException, IOException { - BufferedReader bfReader = - new BufferedReader(new FileReader(scriptDir + File.separatorChar + scriptFile)); - String currLine; - StringBuilder sb = new StringBuilder(); - String currentCommand = null; - while ((currLine = bfReader.readLine()) != null) { - currLine = currLine.trim(); + public List getExecutableCommands(String scriptDir, String scriptFile, boolean fixQuotes) throws IllegalFormatException, IOException { + List commands = new java.util.ArrayList<>(); - if (fixQuotes && !getQuoteCharacter().equals(DEFAULT_QUOTE)) { - currLine = currLine.replace("\\\"", getQuoteCharacter()); - } + try (BufferedReader bfReader = + new BufferedReader(new FileReader(scriptDir + File.separatorChar + scriptFile))) { + String currLine; + String currentCommand = null; - if (currLine.isEmpty()) { - continue; // skip empty lines - } + while ((currLine = bfReader.readLine()) != null) { + currLine = currLine.trim(); - if (currentCommand == null) { - currentCommand = currLine; - } else { - currentCommand = currentCommand + " " + currLine; - } - if (isPartialCommand(currLine)) { - // if its a partial line, continue collecting the pieces - continue; - } + if (fixQuotes && !getQuoteCharacter().equals(DEFAULT_QUOTE)) { + currLine = currLine.replace("\\\"", getQuoteCharacter()); + } - // if this is a valid executable command then add it to the buffer - if (!isNonExecCommand(currentCommand)) { - currentCommand = cleanseCommand(currentCommand); - if (isNestedScript(currentCommand)) { - // if this is a nested sql script then flatten it - String currScript = getScriptName(currentCommand); - sb.append(buildCommand(scriptDir, currScript)); + if (currLine.isEmpty()) { + continue; + } + + if (currentCommand == null) { + currentCommand = currLine; } else { - // Now we have a complete statement, process it - // write the line to buffer - sb.append(currentCommand); - if (usingSqlLine) sb.append(";"); - sb.append(System.getProperty("line.separator")); + currentCommand = currentCommand + " " + currLine; + } + + if (isPartialCommand(currLine)) { + continue; + } + + if (!isNonExecCommand(currentCommand)) { + currentCommand = cleanseCommand(currentCommand); + if (isNestedScript(currentCommand)) { + String currScript = getScriptName(currentCommand); + commands.addAll(getExecutableCommands(scriptDir, currScript, fixQuotes)); + } else { + commands.add(currentCommand.trim()); + } } + currentCommand = null; + } + } + return commands; + } + + @Override + public String buildCommand( + String scriptDir, String scriptFile) throws IllegalFormatException, IOException { + return buildCommand(scriptDir, scriptFile, false); + } + + @Override + public String buildCommand( + String scriptDir, String scriptFile, boolean fixQuotes) throws IllegalFormatException, IOException { + List commands = getExecutableCommands(scriptDir, scriptFile, fixQuotes); + StringBuilder sb = new StringBuilder(); + for (String cmd : commands) { + sb.append(cmd); + if (usingSqlLine) { + sb.append(";"); } - currentCommand = null; + sb.append(System.lineSeparator()); } - bfReader.close(); + return sb.toString(); } @@ -581,7 +609,7 @@ public static NestedScriptParser getDbCommandParser(String dbName, return new MSSQLCommandParser(dbOpts, msUsername, msPassword, conf, usingSqlLine); } else if (dbName.equalsIgnoreCase(DB_MYSQL)) { return new MySqlCommandParser(dbOpts, msUsername, msPassword, conf, usingSqlLine); - } else if (dbName.equalsIgnoreCase(DB_POSTGRACE)) { + } else if (dbName.equalsIgnoreCase(DB_POSTGRES)) { return new PostgresCommandParser(dbOpts, msUsername, msPassword, conf, usingSqlLine); } else if (dbName.equalsIgnoreCase(DB_ORACLE)) { return new OracleCommandParser(dbOpts, msUsername, msPassword, conf, usingSqlLine); diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java new file mode 100644 index 000000000000..15d4e5004ba3 --- /dev/null +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java @@ -0,0 +1,86 @@ +/* + * 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.metastore.tools.schematool; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.List; + +public class IdempotentDDLExecutor { + private static final Logger LOG = LoggerFactory.getLogger(IdempotentDDLExecutor.class); + + private final Connection conn; + private final DbErrorCodes errorCodes; + private final HiveSchemaHelper.NestedScriptParser parser; + private final boolean verbose; + + public IdempotentDDLExecutor( + Connection conn, String dbType, HiveSchemaHelper.NestedScriptParser parser, boolean verbose) { + this.conn = conn; + this.errorCodes = DbErrorCodes.forDbType(dbType); + this.parser = parser; + this.verbose = verbose; + } + + public void executeScript(String scriptFile) throws SQLException, IOException { + LOG.info("Executing script line-by-line via IdempotentDDLExecutor: {}", scriptFile); + conn.setAutoCommit(true); + + File file = new File(scriptFile); + List commands = parser.getExecutableCommands(file.getParent(), file.getName()); + + for (String sqlStmt : commands) { + if (!sqlStmt.isEmpty()) { + executeStatement(sqlStmt); + } + } + if (verbose) { + System.out.println("Completed successfully."); + } + } + + private void executeStatement(String sqlStmt) throws SQLException { + if (verbose) { + System.out.println("Executing: " + sqlStmt); + } else if (LOG.isDebugEnabled()) { + LOG.debug("Executing: {}", sqlStmt); + } + + try (Statement stmt = conn.createStatement()) { + stmt.execute(sqlStmt); + } catch (SQLException e) { + if (errorCodes.isIgnorable(e)) { + String msg = String.format("Object already exists or was already dropped. " + + "Statement: %s, ErrorCode: %d, SQLState: %s", sqlStmt, e.getErrorCode(), e.getSQLState()); + if (verbose) { + System.out.println(msg); + } + LOG.info(msg); + } else { + LOG.error("SQL Error executing: {}. Error Code: {}, SQLState: {}", sqlStmt, e.getErrorCode(), e.getSQLState()); + throw e; + } + } + } +} \ No newline at end of file diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java index 2dc07cb319bf..305f426b0a31 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java @@ -305,33 +305,22 @@ protected void execSql(String scriptDir, String scriptFile) throws IOException, execSql(scriptDir + File.separatorChar + scriptFile); } - // Generate the beeline args per hive conf and execute the given script + /** + * Executes the given SQL script file against the metastore database via {@link IdempotentDDLExecutor}. + * Each statement in the script is executed individually over a direct JDBC connection with + * auto-commit enabled. Errors that indicate an object already exists or is already gone are + * silently ignored according to the per-database {@link DbErrorCodes}; all other SQL errors are + * rethrown as {@link IOException}. + */ protected void execSql(String sqlScriptFile) throws IOException { - CommandBuilder builder = - new CommandBuilder(conf, url, driver, userName, passWord, sqlScriptFile) - .setVerbose(verbose); - - // run the script using SqlLine - SqlLine sqlLine = new SqlLine(); - ByteArrayOutputStream outputForLog = null; - if (!verbose) { - OutputStream out; - if (LOG.isDebugEnabled()) { - out = outputForLog = new ByteArrayOutputStream(); - } else { - out = new NullOutputStream(); - } - sqlLine.setOutputStream(new PrintStream(out)); - System.setProperty("sqlline.silent", "true"); - } - LOG.info("Going to run command <" + builder.buildToLog() + ">"); - SqlLine.Status status = sqlLine.begin(builder.buildToRun(), null, false); - if (LOG.isDebugEnabled() && outputForLog != null) { - LOG.debug("Received following output from Sqlline:"); - LOG.debug(outputForLog.toString("UTF-8")); - } - if (status != SqlLine.Status.OK) { - throw new IOException("Schema script failed, errorcode " + status); + LOG.info("Going to run script <{}> via Idempotent JDBC Executor", sqlScriptFile); + try (Connection conn = getConnectionToMetastore(true)) { + NestedScriptParser parser = getDbCommandParser(dbType, metaDbType); + IdempotentDDLExecutor idempotentExecutor = new IdempotentDDLExecutor(conn, dbType, parser, verbose); + idempotentExecutor.executeScript(sqlScriptFile); + LOG.info("Script executed successfully."); + } catch (Exception e) { + throw new IOException("Schema script failed, error: " + e.getMessage(), e); } } diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/SchemaToolCommandLine.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/SchemaToolCommandLine.java index 72c1e1ac4884..f41be648a941 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/SchemaToolCommandLine.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/SchemaToolCommandLine.java @@ -240,10 +240,10 @@ private CommandLine getCommandLine(String[] args) throws ParseException { private static final Set VALID_DB_TYPES = ImmutableSet.of(HiveSchemaHelper.DB_DERBY, HiveSchemaHelper.DB_HIVE, HiveSchemaHelper.DB_MSSQL, HiveSchemaHelper.DB_MYSQL, - HiveSchemaHelper.DB_POSTGRACE, HiveSchemaHelper.DB_ORACLE); + HiveSchemaHelper.DB_POSTGRES, HiveSchemaHelper.DB_ORACLE); private static final Set VALID_META_DB_TYPES = ImmutableSet.of(HiveSchemaHelper.DB_DERBY, - HiveSchemaHelper.DB_MSSQL, HiveSchemaHelper.DB_MYSQL, HiveSchemaHelper.DB_POSTGRACE, + HiveSchemaHelper.DB_MSSQL, HiveSchemaHelper.DB_MYSQL, HiveSchemaHelper.DB_POSTGRES, HiveSchemaHelper.DB_ORACLE); private void validate() throws ParseException { diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java index e302d24a95e9..0b5352765465 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java @@ -486,6 +486,162 @@ public void testMetastoreDbPropertiesAfterUpgrade() throws HiveMetaException, IO validateMetastoreDbPropertiesTable(); } + /** + * Runs {@code ddlScripts} twice. The first run applies all DDL normally. The second run + * re-runs the same script from scratch — exactly as a re-run of a failed upgrade would — + * and must not throw, because {@link IdempotentDDLExecutor} swallows "already exists" and + * "already gone" errors via {@link DbErrorCodes}. + */ + private void executeWithIdempotencyCheck(String testName, String[] ddlScripts) throws Exception { + File scriptFile = generateTestScript(ddlScripts); + try { + schemaTool.execSql(scriptFile.getPath()); + } catch (Exception e) { + Assert.fail("[" + testName + "] First run failed for " + dbms.getDbType() + ": " + e.getMessage()); + } + try { + schemaTool.execSql(scriptFile.getPath()); + } catch (IOException e) { + Assert.fail("[" + testName + "] Idempotent retry failed for " + dbms.getDbType() + + " — DbErrorCodes did not cover the error: " + e.getMessage()); + } + } + + @Test + public void testIdempotentTableOperations() throws Exception { + String[] createScripts = new String[]{ + "create table TEST_A (ID int);", + "create table TEST_B (ID int primary key, NAME varchar(50));" + }; + String[] dropScripts = new String[]{ + "drop table TEST_A;", + "drop table TEST_B;" + }; + executeWithIdempotencyCheck("TableOperations-create", createScripts); + executeWithIdempotencyCheck("TableOperations-drop", dropScripts); + } + + @Test + public void testIdempotentAddColumnOperations() throws Exception { + String addColumnStmt = switch (dbms.getDbType()) { + case "oracle" -> "alter table TEST_C add (NEW_COL integer);"; + case "mssql" -> "alter table TEST_C add NEW_COL int;"; + default -> "alter table TEST_C add column NEW_COL int;"; + }; + String[] addScripts = new String[]{ + "create table TEST_C (ID int);", + addColumnStmt + }; + String[] dropScripts = new String[]{ + "alter table TEST_C drop column NEW_COL;" + }; + executeWithIdempotencyCheck("AddColumnOperations-add", addScripts); + executeWithIdempotencyCheck("AddColumnOperations-drop", dropScripts); + } + + @Test + public void testIdempotentIndexOperations() throws Exception { + String[] createScripts = new String[]{ + "create table TEST_D (ID int, VAL int);", + "create index TEST_IDX on TEST_D (ID);" + }; + String dropIndexStmt = switch (dbms.getDbType()) { + case "derby" -> "drop index \"APP\".\"TEST_IDX\";"; + case "oracle", + "postgres" -> "drop index TEST_IDX;"; + default -> "drop index TEST_IDX on TEST_D;"; + }; + String[] dropScripts = new String[]{dropIndexStmt}; + executeWithIdempotencyCheck("IndexOperations-create", createScripts); + executeWithIdempotencyCheck("IndexOperations-drop", dropScripts); + } + + @Test + public void testIdempotentConstraintOperations() throws Exception { + String[] createScripts; + String[] dropScripts; + switch (dbms.getDbType()) { + case "mysql" -> { + createScripts = new String[]{ + "create table TEST_E (ID int primary key, FK_COL int, " + + "constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID));" + }; + dropScripts = new String[]{ + "alter table TEST_E drop foreign key TEST_E_FK;", + "alter table TEST_E drop key TEST_E_FK;", + "alter table TEST_E drop column FK_COL;" + }; + } + case "mssql" -> { + createScripts = new String[]{ + "create table TEST_E (ID int primary key, FK_COL int, VAL int);", + "alter table TEST_E add constraint TEST_E_UQ unique (VAL);", + "alter table TEST_E add constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID);" + }; + dropScripts = new String[]{ + "alter table TEST_E drop constraint TEST_E_FK;", + "alter table TEST_E drop constraint TEST_E_UQ;" + }; + } + default -> { + createScripts = new String[]{ + "create table TEST_E (ID int, VAL int);", + "alter table TEST_E add constraint TEST_E_UQ unique (ID);" + }; + dropScripts = new String[]{ + "alter table TEST_E drop constraint TEST_E_UQ;" + }; + } + } + executeWithIdempotencyCheck("ConstraintOperations-add", createScripts); + executeWithIdempotencyCheck("ConstraintOperations-drop", dropScripts); + } + + /** + * Tests ALTER COLUMN type change and RENAME COLUMN — the same concept across all DBs + * with DB-specific syntax, matching heavy use of MODIFY/ALTER COLUMN/SET DATA TYPE + * and RENAME COLUMN in real upgrade scripts. + */ + @Test + public void testIdempotentAlterColumnOperations() throws Exception { + String[] alterScripts; + String[] dropScripts = new String[]{"alter table TEST_F drop column COL_RENAMED;"}; + switch (dbms.getDbType()) { + case "derby" -> { + alterScripts = new String[]{ + "create table \"TEST_F\" (\"ID\" int, \"COL_MOD\" varchar(10), \"COL_RENAME\" varchar(10));", + "alter table \"TEST_F\" alter \"COL_MOD\" set data type varchar(50);", + "rename column \"APP\".\"TEST_F\".\"COL_RENAME\" to \"COL_RENAMED\";" + }; + dropScripts = new String[]{"alter table \"TEST_F\" drop column \"COL_RENAMED\";"}; + } + case "mssql" -> alterScripts = new String[]{ + "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F alter column COL_MOD varchar(50) not null;", + "exec sp_rename 'TEST_F.COL_RENAME', 'COL_RENAMED', 'COLUMN';" + }; + case "oracle" -> alterScripts = new String[]{ + "create table TEST_F (ID integer, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F modify (COL_MOD varchar(50));", + "alter table TEST_F rename column COL_RENAME to COL_RENAMED;" + }; + case "postgres" -> alterScripts = new String[]{ + "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F alter column COL_MOD type varchar(50);", + "alter table TEST_F rename column COL_RENAME to COL_RENAMED;" + }; + default -> // mysql: CHANGE COLUMN renames and redefines in one statement + alterScripts = new String[]{ + "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F modify column COL_MOD varchar(50);", + "alter table TEST_F change column COL_RENAME COL_RENAMED varchar(10);" + }; + } + executeWithIdempotencyCheck("AlterColumnOperations-alter", alterScripts); + executeWithIdempotencyCheck("AlterColumnOperations-drop", dropScripts); + } + + private File generateTestScript(String [] stmts) throws IOException { File testScriptFile = File.createTempFile("schematest", ".sql"); testScriptFile.deleteOnExit(); From ec5a34dae542026865d0c2d6b9c1059183725571 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Fri, 27 Mar 2026 11:34:43 -0700 Subject: [PATCH 02/10] Restrict ignoreable errors to DDLs only as a safeguard --- .../schematool/IdempotentDDLExecutor.java | 49 ++++++++++++++++++- .../TestSchemaToolForMetastore.java | 3 +- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java index 15d4e5004ba3..85f2151b1a13 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java @@ -26,9 +26,13 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.List; +import java.util.Locale; +import java.util.Set; public class IdempotentDDLExecutor { private static final Logger LOG = LoggerFactory.getLogger(IdempotentDDLExecutor.class); + private static final Set DDL_FIRST_TOKENS = Set.of( + "CREATE", "ALTER", "DROP", "RENAME", "TRUNCATE", "COMMENT"); private final Connection conn; private final DbErrorCodes errorCodes; @@ -70,7 +74,7 @@ private void executeStatement(String sqlStmt) throws SQLException { try (Statement stmt = conn.createStatement()) { stmt.execute(sqlStmt); } catch (SQLException e) { - if (errorCodes.isIgnorable(e)) { + if (isDdlStatement(sqlStmt) && errorCodes.isIgnorable(e)) { String msg = String.format("Object already exists or was already dropped. " + "Statement: %s, ErrorCode: %d, SQLState: %s", sqlStmt, e.getErrorCode(), e.getSQLState()); if (verbose) { @@ -83,4 +87,47 @@ private void executeStatement(String sqlStmt) throws SQLException { } } } + + private boolean isDdlStatement(String sqlStmt) { + if (sqlStmt == null) { + return false; + } + + String trimmed = sqlStmt.trim(); + if (trimmed.isEmpty()) { + return false; + } + + // We only inspect the first one or two keywords for statement type classification. + String[] tokens = trimmed.split("\\s+", 3); + if (tokens.length == 0) { + return false; + } + + String first = normalizeToken(tokens[0]).toUpperCase(Locale.ROOT); + if (DDL_FIRST_TOKENS.contains(first)) { + return true; + } + + if (("EXEC".equals(first) || "EXECUTE".equals(first)) && tokens.length > 1) { + String second = normalizeToken(tokens[1]); + return "SP_RENAME".equalsIgnoreCase(second) || isDropHelperProcedure(second); + } + + return false; + } + + private String normalizeToken(String token) { + if (token == null || token.isEmpty()) { + return ""; + } + if (token.endsWith(";")) { + return token.substring(0, token.length() - 1); + } + return token; + } + + private boolean isDropHelperProcedure(String token) { + return token != null && token.toUpperCase(Locale.ROOT).startsWith("#DROP_"); + } } \ No newline at end of file diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java index 0b5352765465..b0e7fadbea9b 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java @@ -579,7 +579,8 @@ public void testIdempotentConstraintOperations() throws Exception { "alter table TEST_E add constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID);" }; dropScripts = new String[]{ - "alter table TEST_E drop constraint TEST_E_FK;", + "create procedure #DROP_FK_HELPER as begin alter table TEST_E drop constraint TEST_E_FK end;", + "exec #DROP_FK_HELPER;", "alter table TEST_E drop constraint TEST_E_UQ;" }; } From 55529c4d01c7b0592abf5dc83ec1eef767c7e6f3 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Tue, 31 Mar 2026 12:40:20 -0700 Subject: [PATCH 03/10] Pass metaDbType to schema parser and align MetastoreSchemaTool parser mode with JDBC execution --- .../hive/metastore/tools/schematool/MetastoreSchemaTool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java index 305f426b0a31..4c6590395735 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java @@ -258,7 +258,7 @@ public Connection getConnectionToMetastore(boolean printInfo) throws HiveMetaExc protected NestedScriptParser getDbCommandParser(String dbType, String metaDbType) { return HiveSchemaHelper.getDbCommandParser(dbType, dbOpts, userName, - passWord, conf, null, true); + passWord, conf, metaDbType, false); } protected MetaStoreConnectionInfo getConnectionInfo(boolean printInfo) { From 5ef4c6313223619da9c0ad80949788b32ee79712 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Tue, 31 Mar 2026 13:41:44 -0700 Subject: [PATCH 04/10] set READ_COMMITTED for JDBC schema execution --- .../hive/metastore/tools/schematool/MetastoreSchemaTool.java | 1 + 1 file changed, 1 insertion(+) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java index 4c6590395735..14408a840bb2 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java @@ -315,6 +315,7 @@ protected void execSql(String scriptDir, String scriptFile) throws IOException, protected void execSql(String sqlScriptFile) throws IOException { LOG.info("Going to run script <{}> via Idempotent JDBC Executor", sqlScriptFile); try (Connection conn = getConnectionToMetastore(true)) { + conn.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED); NestedScriptParser parser = getDbCommandParser(dbType, metaDbType); IdempotentDDLExecutor idempotentExecutor = new IdempotentDDLExecutor(conn, dbType, parser, verbose); idempotentExecutor.executeScript(sqlScriptFile); From 6afe6a8523004d46e1464643bfa55d06be5db582 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Tue, 31 Mar 2026 14:07:56 -0700 Subject: [PATCH 05/10] Fail fast on unterminated SQL at EOF --- .../hive/metastore/tools/schematool/HiveSchemaHelper.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java index 542ea5a2f3fc..15ea5575e07a 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java @@ -311,6 +311,10 @@ public List getExecutableCommands(String scriptDir, String scriptFile, b } currentCommand = null; } + + if (currentCommand != null && !isNonExecCommand(currentCommand)) { + throw new IllegalArgumentException("Unterminated SQL statement at end of script: " + scriptFile); + } } return commands; } From e52133e4f0874a2c3dca540a4e7a2107c8e7c19e Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Tue, 31 Mar 2026 14:20:13 -0700 Subject: [PATCH 06/10] Use logger instead of stdout in idempotent executor --- .../tools/schematool/IdempotentDDLExecutor.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java index 85f2151b1a13..2b3013697269 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java @@ -60,13 +60,13 @@ public void executeScript(String scriptFile) throws SQLException, IOException { } } if (verbose) { - System.out.println("Completed successfully."); + LOG.info("Completed successfully."); } } private void executeStatement(String sqlStmt) throws SQLException { if (verbose) { - System.out.println("Executing: " + sqlStmt); + LOG.info("Executing: {}", sqlStmt); } else if (LOG.isDebugEnabled()) { LOG.debug("Executing: {}", sqlStmt); } @@ -75,12 +75,8 @@ private void executeStatement(String sqlStmt) throws SQLException { stmt.execute(sqlStmt); } catch (SQLException e) { if (isDdlStatement(sqlStmt) && errorCodes.isIgnorable(e)) { - String msg = String.format("Object already exists or was already dropped. " + - "Statement: %s, ErrorCode: %d, SQLState: %s", sqlStmt, e.getErrorCode(), e.getSQLState()); - if (verbose) { - System.out.println(msg); - } - LOG.info(msg); + LOG.info("Object already exists or was already dropped. Statement: {}, ErrorCode: {}, SQLState: {}", + sqlStmt, e.getErrorCode(), e.getSQLState()); } else { LOG.error("SQL Error executing: {}. Error Code: {}, SQLState: {}", sqlStmt, e.getErrorCode(), e.getSQLState()); throw e; From 638437a439a11ef5e1465870494aa0d3b6c33450 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Tue, 31 Mar 2026 15:00:47 -0700 Subject: [PATCH 07/10] Improve idempotency test failure diagnostics and exception handling --- .../tools/schematool/TestSchemaToolForMetastore.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java index b0e7fadbea9b..715c8f37d748 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java @@ -501,9 +501,10 @@ private void executeWithIdempotencyCheck(String testName, String[] ddlScripts) t } try { schemaTool.execSql(scriptFile.getPath()); - } catch (IOException e) { + } catch (Exception e) { Assert.fail("[" + testName + "] Idempotent retry failed for " + dbms.getDbType() - + " — DbErrorCodes did not cover the error: " + e.getMessage()); + + " — possible missing DbErrorCodes mapping or underlying SQL/configuration issue. " + + "Exception: " + e + ", cause: " + e.getCause()); } } From 8ed889fac5a946b9409402d79034ab4846dcbdcd Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Fri, 20 Mar 2026 16:01:55 -0700 Subject: [PATCH 08/10] Fix failing tests because of missing semicolon in some of the queries --- .../tools/schematool/TestSchemaToolForMetastore.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java index 715c8f37d748..ac17dccec268 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java @@ -220,7 +220,7 @@ public void testValidateNullValues() throws Exception { // Test invalid case String[] scripts = new String[] { - "update TBLS set SD_ID=null" + "update TBLS set SD_ID=null;" }; File scriptFile = generateTestScript(scripts); schemaTool.execSql(scriptFile.getPath()); @@ -308,7 +308,7 @@ public void testValidateSchemaVersions() throws Exception { boolean isValid = validator.validateSchemaVersions(); // Test an invalid case with multiple versions String[] scripts = new String[] { - "insert into VERSION values(100, '2.2.0', 'Hive release version 2.2.0')" + "insert into VERSION values(100, '2.2.0', 'Hive release version 2.2.0');" }; File scriptFile = generateTestScript(scripts); schemaTool.execSql(scriptFile.getPath()); @@ -316,7 +316,7 @@ public void testValidateSchemaVersions() throws Exception { Assert.assertFalse(isValid); scripts = new String[] { - "delete from VERSION where VER_ID = 100" + "delete from VERSION where VER_ID = 100;" }; scriptFile = generateTestScript(scripts); schemaTool.execSql(scriptFile.getPath()); @@ -325,7 +325,7 @@ public void testValidateSchemaVersions() throws Exception { // Test an invalid case without version scripts = new String[] { - "delete from VERSION" + "delete from VERSION;" }; scriptFile = generateTestScript(scripts); schemaTool.execSql(scriptFile.getPath()); From 28a7f9798acd3bde052cbcdbda812dad4d57976b Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Fri, 20 Mar 2026 17:19:57 -0700 Subject: [PATCH 09/10] Address sonar issues --- .../tools/schematool/DbErrorCodes.java | 14 +- .../tools/schematool/HiveSchemaHelper.java | 49 ++++--- .../schematool/IdempotentDDLExecutor.java | 5 +- .../tools/schematool/MetastoreSchemaTool.java | 6 - .../TestSchemaToolForMetastore.java | 131 +++++++++--------- 5 files changed, 99 insertions(+), 106 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java index 6285795ab2c4..2dc79c14497c 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/DbErrorCodes.java @@ -50,7 +50,7 @@ class Codes implements DbErrorCodes { @Override public boolean isIgnorable(SQLException e) { String state = e.getSQLState(); - String code = String.valueOf(e.getErrorCode()); + String code = String.valueOf(e.getErrorCode()); return duplicateCodes.contains(state) || duplicateCodes.contains(code) || missingCodes.contains(state) || missingCodes.contains(code); } @@ -134,12 +134,12 @@ static DbErrorCodes forDbType(String dbType) { return NOOP; } return switch (dbType.toLowerCase()) { - case "postgres" -> POSTGRES; - case "derby", "derby.clean" -> DERBY; - case "mysql", "mariadb" -> MYSQL; - case "oracle" -> ORACLE; - case "mssql" -> MSSQL; - default -> NOOP; + case "postgres" -> POSTGRES; + case "derby", "derby.clean" -> DERBY; + case "mysql", "mariadb" -> MYSQL; + case "oracle" -> ORACLE; + case "mssql" -> MSSQL; + default -> NOOP; }; } } diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java index 15ea5575e07a..b4563c1a4098 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/HiveSchemaHelper.java @@ -266,12 +266,14 @@ public boolean needsQuotedIdentifier() { } @Override - public List getExecutableCommands(String scriptDir, String scriptFile) throws IllegalFormatException, IOException { + public List getExecutableCommands(String scriptDir, String scriptFile) + throws IllegalFormatException, IOException { return getExecutableCommands(scriptDir, scriptFile, false); } @Override - public List getExecutableCommands(String scriptDir, String scriptFile, boolean fixQuotes) throws IllegalFormatException, IOException { + public List getExecutableCommands(String scriptDir, String scriptFile, boolean fixQuotes) + throws IllegalFormatException, IOException { List commands = new java.util.ArrayList<>(); try (BufferedReader bfReader = @@ -280,36 +282,26 @@ public List getExecutableCommands(String scriptDir, String scriptFile, b String currentCommand = null; while ((currLine = bfReader.readLine()) != null) { - currLine = currLine.trim(); - - if (fixQuotes && !getQuoteCharacter().equals(DEFAULT_QUOTE)) { - currLine = currLine.replace("\\\"", getQuoteCharacter()); - } + currLine = fixQuotesFromCurrentLine(fixQuotes, currLine.trim()); if (currLine.isEmpty()) { continue; } - if (currentCommand == null) { - currentCommand = currLine; - } else { - currentCommand = currentCommand + " " + currLine; - } - - if (isPartialCommand(currLine)) { - continue; - } - - if (!isNonExecCommand(currentCommand)) { - currentCommand = cleanseCommand(currentCommand); - if (isNestedScript(currentCommand)) { - String currScript = getScriptName(currentCommand); - commands.addAll(getExecutableCommands(scriptDir, currScript, fixQuotes)); - } else { - commands.add(currentCommand.trim()); + currentCommand = currentCommand == null ? currLine : currentCommand + " " + currLine; + + if (!isPartialCommand(currLine)) { + if (!isNonExecCommand(currentCommand)) { + currentCommand = cleanseCommand(currentCommand); + if (isNestedScript(currentCommand)) { + String currScript = getScriptName(currentCommand); + commands.addAll(getExecutableCommands(scriptDir, currScript, fixQuotes)); + } else { + commands.add(currentCommand.trim()); + } } + currentCommand = null; } - currentCommand = null; } if (currentCommand != null && !isNonExecCommand(currentCommand)) { @@ -319,6 +311,13 @@ public List getExecutableCommands(String scriptDir, String scriptFile, b return commands; } + private String fixQuotesFromCurrentLine(boolean fixQuotes, String currLine) { + if (fixQuotes && !getQuoteCharacter().equals(DEFAULT_QUOTE)) { + currLine = currLine.replace("\\\"", getQuoteCharacter()); + } + return currLine; + } + @Override public String buildCommand( String scriptDir, String scriptFile) throws IllegalFormatException, IOException { diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java index 2b3013697269..556491797585 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java @@ -67,7 +67,8 @@ public void executeScript(String scriptFile) throws SQLException, IOException { private void executeStatement(String sqlStmt) throws SQLException { if (verbose) { LOG.info("Executing: {}", sqlStmt); - } else if (LOG.isDebugEnabled()) { + } + if (LOG.isDebugEnabled()) { LOG.debug("Executing: {}", sqlStmt); } @@ -126,4 +127,4 @@ private String normalizeToken(String token) { private boolean isDropHelperProcedure(String token) { return token != null && token.toUpperCase(Locale.ROOT).startsWith("#DROP_"); } -} \ No newline at end of file +} diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java index 14408a840bb2..9767885b33e5 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java @@ -21,7 +21,6 @@ import org.apache.commons.cli.OptionGroup; import org.apache.commons.cli.ParseException; import org.apache.commons.io.FileUtils; -import org.apache.commons.io.output.NullOutputStream; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -37,16 +36,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sqlline.SqlLine; - import java.io.BufferedReader; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileReader; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; -import java.io.PrintStream; import java.net.URI; import java.sql.Connection; import java.sql.SQLException; diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java index ac17dccec268..98228232a198 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/schematool/TestSchemaToolForMetastore.java @@ -525,9 +525,9 @@ public void testIdempotentTableOperations() throws Exception { @Test public void testIdempotentAddColumnOperations() throws Exception { String addColumnStmt = switch (dbms.getDbType()) { - case "oracle" -> "alter table TEST_C add (NEW_COL integer);"; - case "mssql" -> "alter table TEST_C add NEW_COL int;"; - default -> "alter table TEST_C add column NEW_COL int;"; + case "oracle" -> "alter table TEST_C add (NEW_COL integer);"; + case "mssql" -> "alter table TEST_C add NEW_COL int;"; + default -> "alter table TEST_C add column NEW_COL int;"; }; String[] addScripts = new String[]{ "create table TEST_C (ID int);", @@ -547,10 +547,9 @@ public void testIdempotentIndexOperations() throws Exception { "create index TEST_IDX on TEST_D (ID);" }; String dropIndexStmt = switch (dbms.getDbType()) { - case "derby" -> "drop index \"APP\".\"TEST_IDX\";"; - case "oracle", - "postgres" -> "drop index TEST_IDX;"; - default -> "drop index TEST_IDX on TEST_D;"; + case "derby" -> "drop index \"APP\".\"TEST_IDX\";"; + case "oracle", "postgres" -> "drop index TEST_IDX;"; + default -> "drop index TEST_IDX on TEST_D;"; }; String[] dropScripts = new String[]{dropIndexStmt}; executeWithIdempotencyCheck("IndexOperations-create", createScripts); @@ -562,38 +561,38 @@ public void testIdempotentConstraintOperations() throws Exception { String[] createScripts; String[] dropScripts; switch (dbms.getDbType()) { - case "mysql" -> { - createScripts = new String[]{ - "create table TEST_E (ID int primary key, FK_COL int, " - + "constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID));" - }; - dropScripts = new String[]{ - "alter table TEST_E drop foreign key TEST_E_FK;", - "alter table TEST_E drop key TEST_E_FK;", - "alter table TEST_E drop column FK_COL;" - }; - } - case "mssql" -> { - createScripts = new String[]{ - "create table TEST_E (ID int primary key, FK_COL int, VAL int);", - "alter table TEST_E add constraint TEST_E_UQ unique (VAL);", - "alter table TEST_E add constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID);" - }; - dropScripts = new String[]{ - "create procedure #DROP_FK_HELPER as begin alter table TEST_E drop constraint TEST_E_FK end;", + case "mysql" -> { + createScripts = new String[] { + "create table TEST_E (ID int primary key, FK_COL int, " + + "constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID));" + }; + dropScripts = new String[] { + "alter table TEST_E drop foreign key TEST_E_FK;", + "alter table TEST_E drop key TEST_E_FK;", + "alter table TEST_E drop column FK_COL;" + }; + } + case "mssql" -> { + createScripts = new String[] { + "create table TEST_E (ID int primary key, FK_COL int, VAL int);", + "alter table TEST_E add constraint TEST_E_UQ unique (VAL);", + "alter table TEST_E add constraint TEST_E_FK foreign key (FK_COL) references TEST_E(ID);" + }; + dropScripts = new String[] { + "create procedure #DROP_FK_HELPER as begin alter table TEST_E drop constraint TEST_E_FK end;", "exec #DROP_FK_HELPER;", - "alter table TEST_E drop constraint TEST_E_UQ;" - }; - } - default -> { - createScripts = new String[]{ - "create table TEST_E (ID int, VAL int);", - "alter table TEST_E add constraint TEST_E_UQ unique (ID);" - }; - dropScripts = new String[]{ - "alter table TEST_E drop constraint TEST_E_UQ;" - }; - } + "alter table TEST_E drop constraint TEST_E_UQ;" + }; + } + default -> { + createScripts = new String[] { + "create table TEST_E (ID int, VAL int);", + "alter table TEST_E add constraint TEST_E_UQ unique (ID);" + }; + dropScripts = new String[] { + "alter table TEST_E drop constraint TEST_E_UQ;" + }; + } } executeWithIdempotencyCheck("ConstraintOperations-add", createScripts); executeWithIdempotencyCheck("ConstraintOperations-drop", dropScripts); @@ -609,35 +608,35 @@ public void testIdempotentAlterColumnOperations() throws Exception { String[] alterScripts; String[] dropScripts = new String[]{"alter table TEST_F drop column COL_RENAMED;"}; switch (dbms.getDbType()) { - case "derby" -> { - alterScripts = new String[]{ - "create table \"TEST_F\" (\"ID\" int, \"COL_MOD\" varchar(10), \"COL_RENAME\" varchar(10));", - "alter table \"TEST_F\" alter \"COL_MOD\" set data type varchar(50);", - "rename column \"APP\".\"TEST_F\".\"COL_RENAME\" to \"COL_RENAMED\";" - }; - dropScripts = new String[]{"alter table \"TEST_F\" drop column \"COL_RENAMED\";"}; - } - case "mssql" -> alterScripts = new String[]{ - "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", - "alter table TEST_F alter column COL_MOD varchar(50) not null;", - "exec sp_rename 'TEST_F.COL_RENAME', 'COL_RENAMED', 'COLUMN';" - }; - case "oracle" -> alterScripts = new String[]{ - "create table TEST_F (ID integer, COL_MOD varchar(10), COL_RENAME varchar(10));", - "alter table TEST_F modify (COL_MOD varchar(50));", - "alter table TEST_F rename column COL_RENAME to COL_RENAMED;" - }; - case "postgres" -> alterScripts = new String[]{ - "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", - "alter table TEST_F alter column COL_MOD type varchar(50);", - "alter table TEST_F rename column COL_RENAME to COL_RENAMED;" + case "derby" -> { + alterScripts = new String[] { + "create table \"TEST_F\" (\"ID\" int, \"COL_MOD\" varchar(10), \"COL_RENAME\" varchar(10));", + "alter table \"TEST_F\" alter \"COL_MOD\" set data type varchar(50);", + "rename column \"APP\".\"TEST_F\".\"COL_RENAME\" to \"COL_RENAMED\";" }; - default -> // mysql: CHANGE COLUMN renames and redefines in one statement - alterScripts = new String[]{ - "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", - "alter table TEST_F modify column COL_MOD varchar(50);", - "alter table TEST_F change column COL_RENAME COL_RENAMED varchar(10);" - }; + dropScripts = new String[] {"alter table \"TEST_F\" drop column \"COL_RENAMED\";"}; + } + case "mssql" -> alterScripts = new String[] { + "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F alter column COL_MOD varchar(50) not null;", + "exec sp_rename 'TEST_F.COL_RENAME', 'COL_RENAMED', 'COLUMN';" + }; + case "oracle" -> alterScripts = new String[] { + "create table TEST_F (ID integer, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F modify (COL_MOD varchar(50));", + "alter table TEST_F rename column COL_RENAME to COL_RENAMED;" + }; + case "postgres" -> alterScripts = new String[] { + "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F alter column COL_MOD type varchar(50);", + "alter table TEST_F rename column COL_RENAME to COL_RENAMED;" + }; + default -> // mysql: CHANGE COLUMN renames and redefines in one statement + alterScripts = new String[] { + "create table TEST_F (ID int, COL_MOD varchar(10), COL_RENAME varchar(10));", + "alter table TEST_F modify column COL_MOD varchar(50);", + "alter table TEST_F change column COL_RENAME COL_RENAMED varchar(10);" + }; } executeWithIdempotencyCheck("AlterColumnOperations-alter", alterScripts); executeWithIdempotencyCheck("AlterColumnOperations-drop", dropScripts); From 1459b18b3023fa8d4ea7c058207660a5129b4fc3 Mon Sep 17 00:00:00 2001 From: Soumyakanti Das Date: Tue, 31 Mar 2026 16:31:57 -0700 Subject: [PATCH 10/10] Enable fixQuotes in idempotent executor --- .../metastore/tools/schematool/IdempotentDDLExecutor.java | 7 +++++-- .../metastore/tools/schematool/MetastoreSchemaTool.java | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java index 556491797585..47bf6b14ece7 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/IdempotentDDLExecutor.java @@ -37,13 +37,16 @@ public class IdempotentDDLExecutor { private final Connection conn; private final DbErrorCodes errorCodes; private final HiveSchemaHelper.NestedScriptParser parser; + private final boolean fixQuotes; private final boolean verbose; public IdempotentDDLExecutor( - Connection conn, String dbType, HiveSchemaHelper.NestedScriptParser parser, boolean verbose) { + Connection conn, String dbType, HiveSchemaHelper.NestedScriptParser parser, + boolean fixQuotes, boolean verbose) { this.conn = conn; this.errorCodes = DbErrorCodes.forDbType(dbType); this.parser = parser; + this.fixQuotes = fixQuotes; this.verbose = verbose; } @@ -52,7 +55,7 @@ public void executeScript(String scriptFile) throws SQLException, IOException { conn.setAutoCommit(true); File file = new File(scriptFile); - List commands = parser.getExecutableCommands(file.getParent(), file.getName()); + List commands = parser.getExecutableCommands(file.getParent(), file.getName(), fixQuotes); for (String sqlStmt : commands) { if (!sqlStmt.isEmpty()) { diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java index 9767885b33e5..a631310033ff 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/schematool/MetastoreSchemaTool.java @@ -311,7 +311,8 @@ protected void execSql(String sqlScriptFile) throws IOException { try (Connection conn = getConnectionToMetastore(true)) { conn.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED); NestedScriptParser parser = getDbCommandParser(dbType, metaDbType); - IdempotentDDLExecutor idempotentExecutor = new IdempotentDDLExecutor(conn, dbType, parser, verbose); + IdempotentDDLExecutor idempotentExecutor = + new IdempotentDDLExecutor(conn, dbType, parser, metaDbType != null, verbose); idempotentExecutor.executeScript(sqlScriptFile); LOG.info("Script executed successfully."); } catch (Exception e) {