jerryshao commented on code in PR #13351: URL: https://github.com/apache/gravitino/pull/13351#discussion_r4059764897
########## catalogs/hive-metastore3-libs/src/main/java/org/apache/gravitino/hive/client/hive3/HiveShimV3.java: ########## @@ -0,0 +1,583 @@ +/* + * 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.gravitino.hive.client.hive3; + +import static org.apache.gravitino.hive.client.HiveClientClassLoader.HiveVersion.HIVE3; +import static org.apache.gravitino.hive.client.Util.updateConfigurationFromProperties; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.hive.HivePartition; +import org.apache.gravitino.hive.HiveSchema; +import org.apache.gravitino.hive.HiveTable; +import org.apache.gravitino.hive.client.HiveExceptionConverter; +import org.apache.gravitino.hive.client.HiveExceptionConverter.ExceptionTarget; +import org.apache.gravitino.hive.client.HiveShim; +import org.apache.gravitino.hive.converter.HiveColumnDefaultValueConverter; +import org.apache.gravitino.hive.converter.HiveDatabaseConverter; +import org.apache.gravitino.hive.converter.HiveTableConverter; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.utils.RandomNameUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient; +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.api.Catalog; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.hadoop.hive.metastore.api.DefaultConstraintsRequest; +import org.apache.hadoop.hive.metastore.api.NotNullConstraintsRequest; +import org.apache.hadoop.hive.metastore.api.Partition; +import org.apache.hadoop.hive.metastore.api.SQLDefaultConstraint; +import org.apache.hadoop.hive.metastore.api.SQLNotNullConstraint; +import org.apache.hadoop.hive.metastore.api.Table; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Hive 3.x metastore shim. Hive 3.x supports multiple metastore catalogs and NOT NULL / DEFAULT + * column constraints, so every operation below is catalog-aware and column constraints are kept in + * sync with the metastore. + */ +public class HiveShimV3 extends HiveShim { + + private static final Logger LOG = LoggerFactory.getLogger(HiveShimV3.class); + + // Keeps generated constraint names well within the metastore's 400 character limit + private static final int MAX_CONSTRAINT_PREFIX = 128; + // Constraints are recorded for metadata only: enabled and not validated against existing data. + // rely_cstr is deliberately false: Gravitino never validates existing data against newly added + // constraints, so telling the query optimizer to rely on them could produce incorrect results. + private static final boolean CONSTRAINT_ENABLE = true; + private static final boolean CONSTRAINT_VALIDATE = false; + private static final boolean CONSTRAINT_RELY = false; + + public HiveShimV3(Properties properties) { + super(HIVE3, properties); + } + + @Override + public IMetaStoreClient createMetaStoreClient(Properties properties) { + try { + Configuration conf = new Configuration(); + updateConfigurationFromProperties(properties, conf); + return RetryingMetaStoreClient.getProxy(conf, false); + } catch (Exception e) { + throw HiveExceptionConverter.toGravitinoException( + e, ExceptionTarget.other("MetaStoreClient")); + } + } + + @Override + public void createDatabase(HiveSchema database) { + Database db = HiveDatabaseConverter.toHiveDb(database); + db.setCatalogName(database.catalogName()); + invoke(ExceptionTarget.schema(database.name()), () -> client.createDatabase(db)); + } + + @Override + public List<String> getAllDatabases(String catalogName) { + return invoke(ExceptionTarget.catalog(catalogName), () -> client.getAllDatabases(catalogName)); + } + + @Override + public HiveSchema getDatabase(String catalogName, String databaseName) { + Database db = + invoke( + ExceptionTarget.schema(databaseName), + () -> client.getDatabase(catalogName, databaseName)); + return HiveDatabaseConverter.fromHiveDB(db); + } + + @Override + public void alterDatabase(String catalogName, String databaseName, HiveSchema database) { + Database db = HiveDatabaseConverter.toHiveDb(database); + db.setCatalogName(catalogName); + invoke( + ExceptionTarget.schema(databaseName), + () -> client.alterDatabase(catalogName, databaseName, db)); + } + + @Override + public void dropDatabase(String catalogName, String databaseName, boolean cascade) { + invoke( + ExceptionTarget.schema(databaseName), + () -> client.dropDatabase(catalogName, databaseName, true, false, cascade)); + } + + @Override + public List<String> getAllTables(String catalogName, String databaseName) { + return invoke( + ExceptionTarget.schema(databaseName), () -> client.getAllTables(catalogName, databaseName)); + } + + @Override + public List<String> listTablesByType( + String catalogName, String databaseName, String tablePattern, String tableType) { + TableType hiveTableType = TableType.valueOf(tableType); + return invoke( + ExceptionTarget.schema(databaseName), + () -> client.getTables(catalogName, databaseName, tablePattern, hiveTableType)); + } + + @Override + public List<String> listTableNamesByFilter( + String catalogName, String databaseName, String filter, short pageSize) { + return invoke( + ExceptionTarget.schema(databaseName), + () -> client.listTableNamesByFilter(catalogName, databaseName, filter, pageSize)); + } + + @Override + public HiveTable getTable(String catalogName, String databaseName, String tableName) { + Table tb = + invoke( + ExceptionTarget.table(tableName), + () -> client.getTable(catalogName, databaseName, tableName)); + ColumnConstraints constraints = loadColumnConstraints(catalogName, databaseName, tableName); Review Comment: [Nit] `getTable` loads NOT NULL and DEFAULT constraints for every metastore object, including views. `HiveViewCatalogOperations.loadHiveView` goes through the same call (`catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveViewCatalogOperations.java:435`), and `viewExists` (`HiveViewCatalogOperations.java:417-429`) delegates to it, so on Hive 3 every view load and every view existence check now costs two extra metastore round-trips for constraints a `VIRTUAL_VIEW` cannot carry. The same reasoning you just applied to `getTableObjectsByName` a few lines below fits here: skipping `loadColumnConstraints` when the returned table's type is `VIRTUAL_VIEW` would keep view paths at one RPC. Verified by: read `loadHiveView`/`viewExists` in this checkout and traced both through `HiveClientImpl.getTable` (`catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientImpl.java:111`) into this method. ########## catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive3IT.java: ########## @@ -39,4 +52,185 @@ protected void startNecessaryContainer() { containerSuite.getHiveContainer().getContainerIpAddress(), HiveContainer.HIVE_METASTORE_PORT); } + + /** Hive 3.x metastores support NOT NULL and DEFAULT constraints, which must round-trip to HMS. */ + @Override + protected void checkColumnConstraintsOnCreate( + NameIdentifier nameIdentifier, Map<String, String> properties) { + NameIdentifier constraintsIdent = + NameIdentifier.of(schemaName, nameIdentifier.name() + "_constraints"); + Column notNullColumn = + Column.of("not_null_column", Types.StringType.get(), "not null column", false, false, null); + Column defaultColumn = + Column.of( + "default_column", + Types.IntegerType.get(), + "default column", + true, + false, + Literals.integerLiteral(42)); + Column defaultStringColumn = + Column.of( + "default_string_column", + Types.StringType.get(), + null, + false, + false, + Literals.stringLiteral("it's")); + Column plainColumn = Column.of("plain_column", Types.StringType.get(), "plain column"); + + catalog + .asTableCatalog() + .createTable( + constraintsIdent, + new Column[] {notNullColumn, defaultColumn, defaultStringColumn, plainColumn}, + TABLE_COMMENT, + properties, + Transforms.EMPTY_TRANSFORM); + + Table loaded = catalog.asTableCatalog().loadTable(constraintsIdent); + assertColumnConstraints(loaded.columns()); + // Read back straight from HMS to make sure the constraints were persisted + assertColumnConstraints(loadHiveTableColumns(schemaName, constraintsIdent.name())); + + // Property-only alters must leave the constraints untouched + catalog.asTableCatalog().alterTable(constraintsIdent, TableChange.setProperty("k1", "v1")); + assertColumnConstraints(loadHiveTableColumns(schemaName, constraintsIdent.name())); + + // Constraints follow the table when it is renamed + NameIdentifier renamedIdent = + NameIdentifier.of(schemaName, constraintsIdent.name() + "_renamed"); + catalog.asTableCatalog().alterTable(constraintsIdent, TableChange.rename(renamedIdent.name())); + assertColumnConstraints(catalog.asTableCatalog().loadTable(renamedIdent).columns()); + assertColumnConstraints(loadHiveTableColumns(schemaName, renamedIdent.name())); + + catalog.asTableCatalog().dropTable(renamedIdent); + + NameIdentifier partitionedIdent = + NameIdentifier.of(schemaName, nameIdentifier.name() + "_partition_constraints"); + Column valueColumn = Column.of("value_column", Types.StringType.get()); + Column partitionColumn = + Column.of( + "partition_column", Types.StringType.get(), "partition column", false, false, null); + catalog + .asTableCatalog() + .createTable( + partitionedIdent, + new Column[] {valueColumn, partitionColumn}, + TABLE_COMMENT, + properties, + new Transform[] {Transforms.identity(partitionColumn.name())}); + Assertions.assertFalse( + findColumn(catalog.asTableCatalog().loadTable(partitionedIdent), partitionColumn.name()) + .nullable()); + Assertions.assertFalse( + findColumn( + loadHiveTableColumns(schemaName, partitionedIdent.name()), partitionColumn.name()) + .nullable()); + catalog.asTableCatalog().dropTable(partitionedIdent); + + NameIdentifier hiveDdlIdent = + NameIdentifier.of(schemaName, nameIdentifier.name() + "_hive_default"); + executeHiveSql( + String.format( + "CREATE TABLE %s.%s (quoted_string STRING DEFAULT 'it''s')", + schemaName, hiveDdlIdent.name())); + try { + Assertions.assertEquals( + Literals.stringLiteral("it's"), + findColumn(catalog.asTableCatalog().loadTable(hiveDdlIdent), "quoted_string") + .defaultValue()); + } finally { + executeHiveSql(String.format("DROP TABLE %s.%s", schemaName, hiveDdlIdent.name())); + } + } + + private void executeHiveSql(String sql) { + HiveContainer hiveContainer = containerSuite.getHiveContainer(); + if (hiveContainer == null) { + hiveContainer = containerSuite.getHiveContainerWithS3(); Review Comment: [Nit] This fallback is unreachable, and would be wrong if it were reached. `startNecessaryContainer` already dereferences `containerSuite.getHiveContainer()` unconditionally at line 52, so the test cannot get this far with a null container; and `getHiveContainerWithS3()` (`integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java:747`) is a different container from the Hive 3 one this class starts, so running the DDL there would silently exercise the wrong metastore rather than fail loudly. `containerSuite.getHiveContainer()` on its own is enough. While here: this helper is otherwise identical to `CatalogHiveViewIT.executeHiveSql` (`catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveViewIT.java:537-546`) and could be shared. Verified by: read `startNecessaryContainer` in this file and both container accessors in `ContainerSuite`. ########## catalogs/hive-metastore3-libs/src/main/java/org/apache/gravitino/hive/client/hive3/HiveShimV3.java: ########## @@ -0,0 +1,583 @@ +/* + * 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.gravitino.hive.client.hive3; + +import static org.apache.gravitino.hive.client.HiveClientClassLoader.HiveVersion.HIVE3; +import static org.apache.gravitino.hive.client.Util.updateConfigurationFromProperties; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.hive.HivePartition; +import org.apache.gravitino.hive.HiveSchema; +import org.apache.gravitino.hive.HiveTable; +import org.apache.gravitino.hive.client.HiveExceptionConverter; +import org.apache.gravitino.hive.client.HiveExceptionConverter.ExceptionTarget; +import org.apache.gravitino.hive.client.HiveShim; +import org.apache.gravitino.hive.converter.HiveColumnDefaultValueConverter; +import org.apache.gravitino.hive.converter.HiveDatabaseConverter; +import org.apache.gravitino.hive.converter.HiveTableConverter; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.utils.RandomNameUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient; +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.api.Catalog; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.hadoop.hive.metastore.api.DefaultConstraintsRequest; +import org.apache.hadoop.hive.metastore.api.NotNullConstraintsRequest; +import org.apache.hadoop.hive.metastore.api.Partition; +import org.apache.hadoop.hive.metastore.api.SQLDefaultConstraint; +import org.apache.hadoop.hive.metastore.api.SQLNotNullConstraint; +import org.apache.hadoop.hive.metastore.api.Table; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Hive 3.x metastore shim. Hive 3.x supports multiple metastore catalogs and NOT NULL / DEFAULT + * column constraints, so every operation below is catalog-aware and column constraints are kept in + * sync with the metastore. + */ +public class HiveShimV3 extends HiveShim { + + private static final Logger LOG = LoggerFactory.getLogger(HiveShimV3.class); + + // Keeps generated constraint names well within the metastore's 400 character limit + private static final int MAX_CONSTRAINT_PREFIX = 128; + // Constraints are recorded for metadata only: enabled and not validated against existing data. + // rely_cstr is deliberately false: Gravitino never validates existing data against newly added + // constraints, so telling the query optimizer to rely on them could produce incorrect results. + private static final boolean CONSTRAINT_ENABLE = true; + private static final boolean CONSTRAINT_VALIDATE = false; + private static final boolean CONSTRAINT_RELY = false; + + public HiveShimV3(Properties properties) { + super(HIVE3, properties); + } + + @Override + public IMetaStoreClient createMetaStoreClient(Properties properties) { + try { + Configuration conf = new Configuration(); + updateConfigurationFromProperties(properties, conf); + return RetryingMetaStoreClient.getProxy(conf, false); + } catch (Exception e) { + throw HiveExceptionConverter.toGravitinoException( + e, ExceptionTarget.other("MetaStoreClient")); + } + } + + @Override + public void createDatabase(HiveSchema database) { + Database db = HiveDatabaseConverter.toHiveDb(database); + db.setCatalogName(database.catalogName()); + invoke(ExceptionTarget.schema(database.name()), () -> client.createDatabase(db)); + } + + @Override + public List<String> getAllDatabases(String catalogName) { + return invoke(ExceptionTarget.catalog(catalogName), () -> client.getAllDatabases(catalogName)); + } + + @Override + public HiveSchema getDatabase(String catalogName, String databaseName) { + Database db = + invoke( + ExceptionTarget.schema(databaseName), + () -> client.getDatabase(catalogName, databaseName)); + return HiveDatabaseConverter.fromHiveDB(db); + } + + @Override + public void alterDatabase(String catalogName, String databaseName, HiveSchema database) { + Database db = HiveDatabaseConverter.toHiveDb(database); + db.setCatalogName(catalogName); + invoke( + ExceptionTarget.schema(databaseName), + () -> client.alterDatabase(catalogName, databaseName, db)); + } + + @Override + public void dropDatabase(String catalogName, String databaseName, boolean cascade) { + invoke( + ExceptionTarget.schema(databaseName), + () -> client.dropDatabase(catalogName, databaseName, true, false, cascade)); + } + + @Override + public List<String> getAllTables(String catalogName, String databaseName) { + return invoke( + ExceptionTarget.schema(databaseName), () -> client.getAllTables(catalogName, databaseName)); + } + + @Override + public List<String> listTablesByType( + String catalogName, String databaseName, String tablePattern, String tableType) { + TableType hiveTableType = TableType.valueOf(tableType); + return invoke( + ExceptionTarget.schema(databaseName), + () -> client.getTables(catalogName, databaseName, tablePattern, hiveTableType)); + } + + @Override + public List<String> listTableNamesByFilter( + String catalogName, String databaseName, String filter, short pageSize) { + return invoke( + ExceptionTarget.schema(databaseName), + () -> client.listTableNamesByFilter(catalogName, databaseName, filter, pageSize)); + } + + @Override + public HiveTable getTable(String catalogName, String databaseName, String tableName) { + Table tb = + invoke( + ExceptionTarget.table(tableName), + () -> client.getTable(catalogName, databaseName, tableName)); + ColumnConstraints constraints = loadColumnConstraints(catalogName, databaseName, tableName); + return HiveTableConverter.fromHiveTable( + tb, notNullColumns(constraints), defaultValues(constraints)); + } + + @Override + public void alterTable( + String catalogName, + String databaseName, + String tableName, + HiveTable alteredHiveTable, + boolean skipStatsUpdate) { + Table tb = HiveTableConverter.toHiveTable(alteredHiveTable); + tb.setCatName(catalogName); + if (skipStatsUpdate) { + // Property-only and comment-only alters cannot change columns, so the constraints stay as + // they are. Instruct the metastore not to recompute statistics for this alter, so it does + // not access the table's storage location. + invoke( + ExceptionTarget.table(tableName), + () -> + client.alter_table( + catalogName, databaseName, tableName, tb, doNotUpdateStatsContext())); + return; + } + + // The Thrift Table passed to alter_table carries no constraint information, so constraints + // are rewritten around it: the existing ones are dropped first (while their columns and the + // old table name still exist) and the desired set is added after the alter (once new columns + // and names are in place). The desired set is built before the first metastore write so that + // an unconvertible default value leaves the table untouched. + ColumnConstraints existing = loadColumnConstraints(catalogName, databaseName, tableName); + ColumnConstraints desired = + buildColumnConstraints( + catalogName, + alteredHiveTable.databaseName(), + alteredHiveTable.name(), + alteredHiveTable.columns(), + existing); + try { + dropColumnConstraints(catalogName, databaseName, tableName, existing); + invoke( + ExceptionTarget.table(tableName), + () -> client.alter_table(catalogName, databaseName, tableName, tb)); + } catch (RuntimeException e) { + // The table is unchanged, so any constraints dropped before the failure can be put back. + restoreColumnConstraints(catalogName, databaseName, tableName, existing, e); + throw e; + } + try { + addColumnConstraints(desired); + } catch (RuntimeException e) { + String message = + String.format( + "Table %s.%s was altered but its column constraints %s were dropped and could not " + + "be re-created; the NOT NULL and DEFAULT constraints must be re-applied " + + "manually", + alteredHiveTable.databaseName(), alteredHiveTable.name(), desired.names); Review Comment: [Nit] The message names `desired.names`, but what was actually dropped is `existing.names`. After an alter that *adds* a NOT NULL to a column that did not have one, the operator is told to manually re-apply a constraint that never existed; after an alter that *removes* one, the constraint that really was dropped is not named at all. Since this message is the operator's only instruction for manual recovery, printing `existing.names` (or both sets) would be accurate. Verified by: read `alterTable` in this checkout — `existing` is what `dropColumnConstraints` consumes (line 198), while `desired` is built from the new column set (lines 190-196), and the two only coincide when the alter changes no constraints. ########## catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/converter/HiveColumnDefaultValueConverter.java: ########## @@ -0,0 +1,213 @@ +/* + * 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.gravitino.hive.converter; + +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; + +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import org.apache.gravitino.rel.expressions.Expression; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.literals.Literal; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Converts column default values between Gravitino {@link Expression}s and the SQL expression + * strings stored by the Hive Metastore in {@code SQLDefaultConstraint}. Literals and zero-argument + * functions are modeled; any other expression is carried verbatim as an {@link UnparsedExpression} + * so it round-trips unchanged. + */ +public class HiveColumnDefaultValueConverter { + + private static final Logger LOG = LoggerFactory.getLogger(HiveColumnDefaultValueConverter.class); + private static final String NULL = "NULL"; + private static final DateTimeFormatter DATE_TIME_FORMATTER = + new DateTimeFormatterBuilder() + .appendPattern("yyyy-MM-dd HH:mm:ss") + .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true) + .toFormatter(); + + private HiveColumnDefaultValueConverter() {} + + /** + * Renders a Gravitino default value as a Hive SQL expression string. + * + * @param defaultValue The Gravitino default value expression. + * @return The Hive SQL expression, or {@code null} when no default value is set. + */ + public static String fromGravitino(Expression defaultValue) { + if (defaultValue == null || DEFAULT_VALUE_NOT_SET.equals(defaultValue)) { + return null; + } + + if (defaultValue instanceof UnparsedExpression) { + return ((UnparsedExpression) defaultValue).unparsedExpression(); + } + + if (defaultValue instanceof FunctionExpression) { + FunctionExpression function = (FunctionExpression) defaultValue; + if (function.arguments().length == 0) { + // Hive requires parentheses for functions such as CURRENT_TIMESTAMP(); the trailing "()" + // is also what toGravitino keys on to recognize a function + return function.functionName().toUpperCase() + "()"; + } + throw new IllegalArgumentException( + "Hive catalog does not support function default value with arguments: " + defaultValue); + } + + if (defaultValue instanceof Literal) { + Literal<?> literal = (Literal<?>) defaultValue; + Type type = literal.dataType(); + if (literal.value() == null || type.name() == Type.Name.NULL) { + return NULL; + } + if (type instanceof Type.NumericType || type instanceof Types.BooleanType) { + return literal.value().toString(); + } + if (type instanceof Types.TimestampType && literal.value() instanceof LocalDateTime) { + return quote(((LocalDateTime) literal.value()).format(DATE_TIME_FORMATTER)); + } + if (type instanceof Types.StringType + || type instanceof Types.VarCharType + || type instanceof Types.FixedCharType + || type instanceof Types.DateType) { + return quote(literal.value().toString()); + } + throw new IllegalArgumentException( + "Hive catalog does not support column default value literal of type " + + type + + ": " + + defaultValue); + } + + throw new IllegalArgumentException("Not a supported column default value: " + defaultValue); + } + + /** + * Parses a Hive SQL default value expression into a Gravitino {@link Expression}. + * + * @param type The Gravitino type of the column that owns the default value. + * @param defaultValue The Hive SQL expression string. + * @return The Gravitino default value expression. + */ + public static Expression toGravitino(Type type, String defaultValue) { + return toGravitino(type, defaultValue, null); + } + + /** + * Parses a Hive SQL default value expression into a Gravitino {@link Expression}. + * + * @param type The Gravitino type of the column that owns the default value. + * @param defaultValue The Hive SQL expression string. + * @param columnName The name of the column that owns the default value, used only to give context + * in log messages; may be {@code null} when unknown. + * @return The Gravitino default value expression. + */ + public static Expression toGravitino(Type type, String defaultValue, String columnName) { + if (defaultValue == null) { + return DEFAULT_VALUE_NOT_SET; + } + String value = defaultValue.trim(); + if (value.equalsIgnoreCase(NULL)) { + return Literals.NULL; + } + if (value.endsWith("()") && value.length() > 2) { + return FunctionExpression.of(value.substring(0, value.length() - 2).toLowerCase()); + } + boolean quoted = value.length() >= 2 && value.startsWith("'") && value.endsWith("'"); + if (quoted) { + value = unquote(value); + } + + Type.Name typeName = type.name(); + try { + switch (typeName) { + case BOOLEAN: + // Boolean.valueOf() maps anything but "true" to false, so only accept the two literals + if (value.equalsIgnoreCase("true") || value.equalsIgnoreCase("false")) { + return Literals.booleanLiteral(Boolean.valueOf(value)); + } + break; + case BYTE: + return Literals.byteLiteral(Byte.valueOf(value)); + case SHORT: + return Literals.shortLiteral(Short.valueOf(value)); + case INTEGER: + return Literals.integerLiteral(Integer.valueOf(value)); + case LONG: + return Literals.longLiteral(Long.valueOf(value)); + case FLOAT: + return Literals.floatLiteral(Float.valueOf(value)); + case DOUBLE: + return Literals.doubleLiteral(Double.valueOf(value)); + case DECIMAL: + Types.DecimalType decimalType = (Types.DecimalType) type; + return Literals.decimalLiteral( + Decimal.of(value, decimalType.precision(), decimalType.scale())); + case DATE: + return Literals.dateLiteral(LocalDate.parse(value)); + case TIMESTAMP: + return Literals.timestampLiteral(LocalDateTime.parse(value, DATE_TIME_FORMATTER)); + case STRING: + case VARCHAR: + case FIXEDCHAR: + // An unquoted value is an expression such as CAST(...) rather than a string literal + if (quoted) { + return Literals.of(value, type); + } + break; + default: + break; + } + } catch (IllegalArgumentException | DateTimeParseException e) { + // The metastore stores arbitrary SQL text; whatever cannot be modeled as a literal of the + // column type is deliberately kept verbatim instead of failing the table load. + LOG.warn( + "Cannot parse Hive default value '{}' as a literal of type {} for column '{}'; keeping " + + "it as an unparsed expression", + defaultValue, + type, + columnName == null ? "<unknown>" : columnName, + e); + } + return UnparsedExpression.of(defaultValue); + } + + private static String quote(String value) { + return "'" + value.replace("\\", "\\\\").replace("'", "\\'") + "'"; Review Comment: [Question] `unquote` now accepts both `\'` and `''`, but `quote` still emits only the backslash form, so every DEFAULT Gravitino writes is stored as e.g. `'it\'s'`. The new IT proves Gravitino can read what Hive writes; nothing proves Hive can read what Gravitino writes — the metastore keeps the constraint text verbatim, and it is Hive's own parser that evaluates it when a row is inserted. Have you confirmed this against a real Hive 3 `INSERT` into a column whose DEFAULT contains a quote? If not, emitting the SQL-standard doubled form (`value.replace("'", "''")`) would be the safer default, and `unquote` already round-trips it after this PR. Verified by: read `quote`/`unquote` here and the Hive-DDL round-trip added at `catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive3IT.java:132-145`; found no test that inserts a row against a Gravitino-written default. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
