This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 207972fe44 [#14523] fix(postgresql): handle native identifiers and
generated indexes safely (#14527)
207972fe44 is described below
commit 207972fe44a95a42ebbffe68d12cdc51d2649230
Author: Qi Yu <[email protected]>
AuthorDate: Mon Nov 2 22:36:55 2026 +0800
[#14523] fix(postgresql): handle native identifiers and generated indexes
safely (#14527)
### What changes were proposed in this pull request?
Bind schema-comment queries, quote table/column/constraint identifiers
consistently, escape exact JDBC metadata patterns, enforce PostgreSQL
UTF-8 name limits and let PostgreSQL name unnamed constraints. Route
existing native table deletion through provider-specific validation
while preserving strict creation validation and other catalogs'
defaults.
### Why are the changes needed?
Existing quote/backslash/wildcard names must remain data or identifiers,
overlong names must not truncate silently, and unnamed indexes must not
collide under a literal null name.
Fixes #14523
Fixes #14488
Fixes #14494
Consolidates #14489, #14495 into this PR.
### Does this PR introduce _any_ user-facing change?
Safe native-name table load/alter/drop and exact schema/table metadata
matching; actionable UTF-8 and index conflict errors. Adds a default
existing-name capability method. PostgreSQL purge, arbitrary native-name
schema CREATE/DROP and general column-name validation expansion remain
outside scope.
### How was this patch tested?
Full coreUnitTest: 2,691 cases, 2 skipped; PostgreSQL: 88, Docker
enabled, no failures. Combined regressions cover quoted native names
with unnamed constraints, original bound parameters plus escaped
metadata filters, and strict creation/default-provider validation.
Mutation/restored-green verification and independent cumulative review
completed. Deployed REST/Trino/Spark end-to-end is not claimed.
---
catalogs/catalog-jdbc-postgresql/build.gradle.kts | 1 +
.../postgresql/PostgreSqlCatalogCapability.java | 35 +++-
.../operation/PostgreSqlSchemaOperations.java | 26 ++-
.../operation/PostgreSqlTableOperations.java | 224 +++++++++++----------
.../TestPostgreSqlCatalogCapability.java | 39 ++++
.../operation/TestPostgreSqlSchemaOperations.java | 26 +++
...estPostgreSqlSchemaOperationsSqlGeneration.java | 46 +++++
.../operation/TestPostgreSqlTableOperations.java | 136 +++++++++++++
...TestPostgreSqlTableOperationsSqlGeneration.java | 75 +++++++
.../catalog/TableNormalizeDispatcher.java | 15 +-
.../gravitino/connector/capability/Capability.java | 15 ++
.../catalog/TestTableNormalizeDispatcher.java | 51 +++++
12 files changed, 567 insertions(+), 122 deletions(-)
diff --git a/catalogs/catalog-jdbc-postgresql/build.gradle.kts
b/catalogs/catalog-jdbc-postgresql/build.gradle.kts
index 3bb6e1d801..c202c23585 100644
--- a/catalogs/catalog-jdbc-postgresql/build.gradle.kts
+++ b/catalogs/catalog-jdbc-postgresql/build.gradle.kts
@@ -52,6 +52,7 @@ dependencies {
testImplementation(libs.commons.dbcp2)
testImplementation(libs.junit.jupiter.api)
testImplementation(libs.junit.jupiter.params)
+ testImplementation(libs.mockito.core)
testImplementation(libs.mysql.driver)
testImplementation(libs.postgresql.driver)
testImplementation(libs.testcontainers)
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSqlCatalogCapability.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSqlCatalogCapability.java
index 3d671565c2..07f5c1e144 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSqlCatalogCapability.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSqlCatalogCapability.java
@@ -18,7 +18,10 @@
*/
package org.apache.gravitino.catalog.postgresql;
+import java.nio.charset.StandardCharsets;
+import java.util.Locale;
import java.util.Set;
+import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
@@ -40,14 +43,40 @@ public class PostgreSqlCatalogCapability implements
Capability {
private static final Set<String> POSTGRESQL_RESERVED_WORDS =
Set.of("pg_catalog", "information_schema");
+ /** {@inheritDoc} */
+ @Override
+ public CapabilityResult specificationOnExistingName(Scope scope, String
name) {
+ if (scope != Scope.TABLE && scope != Scope.SCHEMA) {
+ return specificationOnName(scope, name);
+ }
+ if (MetadataObjects.METADATA_OBJECT_RESERVED_NAME.equals(name)) {
+ return CapabilityResult.unsupported("The metadata wildcard name is
reserved");
+ }
+ if (name.isEmpty()
+ || name.indexOf('\0') >= 0
+ || name.getBytes(StandardCharsets.UTF_8).length > 63) {
+ return CapabilityResult.unsupported(
+ "PostgreSQL native names must contain 1 to 63 UTF-8 bytes and no
NUL");
+ }
+ if (scope == Scope.SCHEMA
+ && POSTGRESQL_RESERVED_WORDS.contains(name.toLowerCase(Locale.ROOT))) {
+ return CapabilityResult.unsupported("PostgreSQL system schema names are
reserved");
+ }
+ return CapabilityResult.SUPPORTED;
+ }
+
@Override
public CapabilityResult specificationOnName(Scope scope, String name) {
- if (!name.matches(POSTGRESQL_NAME_PATTERN)) {
+ if (!name.matches(POSTGRESQL_NAME_PATTERN)
+ || name.getBytes(StandardCharsets.UTF_8).length > 63) {
return CapabilityResult.unsupported(
- String.format("The %s name '%s' is illegal.", scope, name));
+ String.format(
+ "The %s name '%s' is illegal. Names must use supported
characters and contain at most 63 UTF-8 bytes.",
+ scope, name));
}
- if (scope == Scope.SCHEMA &&
POSTGRESQL_RESERVED_WORDS.contains(name.toLowerCase())) {
+ if (scope == Scope.SCHEMA
+ && POSTGRESQL_RESERVED_WORDS.contains(name.toLowerCase(Locale.ROOT))) {
return CapabilityResult.unsupported(
String.format("The %s name '%s' is reserved and cannot be used.",
scope, name));
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
index 3996297b01..57d62c7457 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
@@ -131,7 +131,15 @@ public class PostgreSqlSchemaOperations extends
JdbcDatabaseOperations {
*/
private ResultSet getSchema(Connection connection, String schemaName) throws
SQLException {
final DatabaseMetaData metaData = connection.getMetaData();
- return metaData.getSchemas(connection.getCatalog(), schemaName);
+ String escape = metaData.getSearchStringEscape();
+ String pattern =
+ schemaName == null
+ ? null
+ : schemaName
+ .replace(escape, escape + escape)
+ .replace("%", escape + "%")
+ .replace("_", escape + "_");
+ return metaData.getSchemas(connection.getCatalog(), pattern);
}
@Override
@@ -154,7 +162,8 @@ public class PostgreSqlSchemaOperations extends
JdbcDatabaseOperations {
@Override
protected String generateDatabaseExistSql(String databaseName) {
return String.format(
- "SELECT n.datname FROM pg_catalog.pg_database n where n.datname='%s'",
databaseName);
+ "SELECT n.datname FROM pg_catalog.pg_database n where n.datname='%s'",
+ databaseName.replace("'", "''"));
}
@Override
@@ -178,17 +187,16 @@ public class PostgreSqlSchemaOperations extends
JdbcDatabaseOperations {
return false;
}
- private String getShowSchemaCommentSql(String schema) {
- return String.format(
- "SELECT obj_description(n.oid, 'pg_namespace') AS comment\n"
- + "FROM pg_catalog.pg_namespace n\n"
- + "WHERE n.nspname = '%s';\n",
- schema);
+ private String getShowSchemaCommentSql() {
+ return ("SELECT obj_description(n.oid, 'pg_namespace') AS comment\n"
+ + "FROM pg_catalog.pg_namespace n\n"
+ + "WHERE n.nspname = ?;\n");
}
private String getSchemaComment(String schema, Connection connection) throws
SQLException {
try (PreparedStatement preparedStatement =
- connection.prepareStatement(getShowSchemaCommentSql(schema))) {
+ connection.prepareStatement(getShowSchemaCommentSql())) {
+ preparedStatement.setString(1, schema);
try (ResultSet resultSet = preparedStatement.executeQuery()) {
if (resultSet.next()) {
return resultSet.getString("comment");
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
index 07d59dbcc7..01c8438503 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
@@ -23,6 +23,7 @@ import static
org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeS
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import java.nio.charset.StandardCharsets;
import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
@@ -33,6 +34,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
import javax.sql.DataSource;
import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.ArrayUtils;
@@ -48,9 +50,11 @@ import
org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
import org.apache.gravitino.catalog.jdbc.operation.DatabaseOperation;
import org.apache.gravitino.catalog.jdbc.operation.JdbcTableOperations;
import org.apache.gravitino.catalog.jdbc.operation.RequireDatabaseOperation;
+import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils;
import org.apache.gravitino.exceptions.NoSuchColumnException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
@@ -171,16 +175,14 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
StringBuilder sqlBuilder = new StringBuilder();
sqlBuilder
.append("CREATE TABLE ")
- .append(PG_QUOTE)
- .append(tableName)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(tableName))
.append(" (")
.append(NEW_LINE);
// Add columns
for (int i = 0; i < columns.length; i++) {
JdbcColumn column = columns[i];
- sqlBuilder.append("
").append(PG_QUOTE).append(column.name()).append(PG_QUOTE);
+ sqlBuilder.append(" ").append(quotedIdentifier(column.name()));
appendColumnDefinition(column, sqlBuilder);
// Add a comma for the next column, unless it's the last one
@@ -203,9 +205,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
sqlBuilder
.append(NEW_LINE)
.append(TABLE_COMMENT)
- .append(PG_QUOTE)
- .append(tableName)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(tableName))
.append(IS)
.append(escapeSqlLiteral(comment, '\''))
.append("';");
@@ -217,13 +217,9 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
sqlBuilder
.append(NEW_LINE)
.append(COLUMN_COMMENT)
- .append(PG_QUOTE)
- .append(tableName)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(tableName))
.append(".")
- .append(PG_QUOTE)
- .append(jdbcColumn.name())
- .append(PG_QUOTE)
+ .append(quotedIdentifier(jdbcColumn.name()))
.append(IS)
.append(escapeSqlLiteral(jdbcColumn.comment(), '\''))
.append("';"));
@@ -238,18 +234,19 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
@VisibleForTesting
static void appendIndexesSql(Index[] indexes, StringBuilder sqlBuilder) {
for (Index index : indexes) {
+ validateIndexName(index.name());
String fieldStr = getIndexFieldStr(index.fieldNames());
sqlBuilder.append(",").append(NEW_LINE);
switch (index.type()) {
case PRIMARY_KEY:
if (StringUtils.isNotEmpty(index.name())) {
- sqlBuilder.append("CONSTRAINT
").append(PG_QUOTE).append(index.name()).append(PG_QUOTE);
+ sqlBuilder.append("CONSTRAINT
").append(quotedIdentifier(index.name()));
}
sqlBuilder.append(" PRIMARY KEY (").append(fieldStr).append(")");
break;
case UNIQUE_KEY:
if (StringUtils.isNotEmpty(index.name())) {
- sqlBuilder.append("CONSTRAINT
").append(PG_QUOTE).append(index.name()).append(PG_QUOTE);
+ sqlBuilder.append("CONSTRAINT
").append(quotedIdentifier(index.name()));
}
sqlBuilder.append(" UNIQUE (").append(fieldStr).append(")");
break;
@@ -267,7 +264,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
throw new IllegalArgumentException(
"Index does not support complex fields in PostgreSQL");
}
- return PG_QUOTE + colNames[0] + PG_QUOTE;
+ return quotedIdentifier(colNames[0]);
})
.collect(Collectors.joining(", "));
}
@@ -300,18 +297,14 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
@Override
protected String generateRenameTableSql(String oldTableName, String
newTableName) {
return ALTER_TABLE
- + PG_QUOTE
- + oldTableName
- + PG_QUOTE
+ + quotedIdentifier(oldTableName)
+ " RENAME TO "
- + PG_QUOTE
- + newTableName
- + PG_QUOTE;
+ + quotedIdentifier(newTableName);
}
@Override
protected String generateDropTableSql(String tableName) {
- return "DROP TABLE " + PG_QUOTE + tableName + PG_QUOTE;
+ return "DROP TABLE " + quotedIdentifier(tableName);
}
@Override
@@ -380,14 +373,10 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
&&
Column.DEFAULT_VALUE_NOT_SET.equals(pendingColumn.defaultValue())) {
alterSql.add(
ALTER_TABLE
- + PG_QUOTE
- + tableName
- + PG_QUOTE
+ + quotedIdentifier(tableName)
+ " "
+ ALTER_COLUMN
- + PG_QUOTE
- + updateColumnNullability.fieldName()[0]
- + PG_QUOTE
+ + quotedIdentifier(updateColumnNullability.fieldName()[0])
+ " DROP DEFAULT;");
}
} else if (change instanceof TableChange.AddIndex) {
@@ -427,7 +416,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
return String.format(
"ALTER TABLE %s %s %s %s;",
- PG_QUOTE + tableName + PG_QUOTE, ALTER_COLUMN, PG_QUOTE + fieldName +
PG_QUOTE, action);
+ quotedIdentifier(tableName), ALTER_COLUMN,
quotedIdentifier(fieldName), action);
}
@VisibleForTesting
@@ -435,28 +424,20 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
// Gravitino creates primary and unique indexes as constraints. Dropping
the constraint
// also removes its backing index, so a second DROP INDEX would fail after
a successful drop.
return String.format(
- "ALTER TABLE %s%s%s DROP CONSTRAINT %s%s%s%s;",
- PG_QUOTE,
- tableName,
- PG_QUOTE,
+ "ALTER TABLE %s DROP CONSTRAINT %s%s;",
+ quotedIdentifier(tableName),
deleteIndex.isIfExists() ? "IF EXISTS " : "",
- PG_QUOTE,
- deleteIndex.getName(),
- PG_QUOTE);
+ quotedIdentifier(deleteIndex.getName()));
}
@VisibleForTesting
static String addIndexDefinition(String tableName, TableChange.AddIndex
addIndex) {
+ validateIndexName(addIndex.getName());
StringBuilder sqlBuilder = new StringBuilder();
- sqlBuilder
- .append("ALTER TABLE ")
- .append(PG_QUOTE)
- .append(tableName)
- .append(PG_QUOTE)
- .append(" ADD CONSTRAINT ")
- .append(PG_QUOTE)
- .append(addIndex.getName())
- .append(PG_QUOTE);
+ sqlBuilder.append("ALTER TABLE
").append(quotedIdentifier(tableName)).append(" ADD");
+ if (StringUtils.isNotEmpty(addIndex.getName())) {
+ sqlBuilder.append(" CONSTRAINT
").append(quotedIdentifier(addIndex.getName()));
+ }
switch (addIndex.getType()) {
case PRIMARY_KEY:
sqlBuilder.append(" PRIMARY KEY ");
@@ -479,25 +460,17 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
String col = updateColumnNullability.fieldName()[0];
if (updateColumnNullability.nullable()) {
return ALTER_TABLE
- + PG_QUOTE
- + tableName
- + PG_QUOTE
+ + quotedIdentifier(tableName)
+ " "
+ ALTER_COLUMN
- + PG_QUOTE
- + col
- + PG_QUOTE
+ + quotedIdentifier(col)
+ " DROP NOT NULL;";
} else {
return ALTER_TABLE
- + PG_QUOTE
- + tableName
- + PG_QUOTE
+ + quotedIdentifier(tableName)
+ " "
+ ALTER_COLUMN
- + PG_QUOTE
- + col
- + PG_QUOTE
+ + quotedIdentifier(col)
+ " SET NOT NULL;";
}
}
@@ -515,9 +488,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
}
}
return TABLE_COMMENT
- + PG_QUOTE
- + jdbcTable.name()
- + PG_QUOTE
+ + quotedIdentifier(jdbcTable.name())
+ IS
+ escapeSqlLiteral(newComment, '\'')
+ "';";
@@ -539,13 +510,9 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
}
}
return ALTER_TABLE
- + PG_QUOTE
- + table.name()
- + PG_QUOTE
+ + quotedIdentifier(table.name())
+ " DROP COLUMN "
- + PG_QUOTE
- + deleteColumn.fieldName()[0]
- + PG_QUOTE
+ + quotedIdentifier(deleteColumn.fieldName()[0])
+ ";";
}
@@ -565,14 +532,11 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
throw new NoSuchColumnException("Column %s does not exist.", col);
}
- StringBuilder sqlBuilder =
- new StringBuilder(ALTER_TABLE + PG_QUOTE + jdbcTable.name() +
PG_QUOTE);
+ StringBuilder sqlBuilder = new StringBuilder(ALTER_TABLE +
quotedIdentifier(jdbcTable.name()));
sqlBuilder
.append("\n")
.append(ALTER_COLUMN)
- .append(PG_QUOTE)
- .append(col)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(col))
.append(" SET DEFAULT ")
.append(
columnDefaultValueConverter.fromGravitino(
@@ -595,23 +559,18 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
if (null == column) {
throw new NoSuchColumnException("Column %s does not exist.", col);
}
- StringBuilder sqlBuilder =
- new StringBuilder(ALTER_TABLE + PG_QUOTE + jdbcTable.name() +
PG_QUOTE);
+ StringBuilder sqlBuilder = new StringBuilder(ALTER_TABLE +
quotedIdentifier(jdbcTable.name()));
sqlBuilder
.append("\n")
.append(ALTER_COLUMN)
- .append(PG_QUOTE)
- .append(col)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(col))
.append(" SET DATA TYPE ")
.append(typeConverter.fromGravitino(updateColumnType.getNewDataType()));
if (!column.nullable()) {
sqlBuilder
.append(",\n")
.append(ALTER_COLUMN)
- .append(PG_QUOTE)
- .append(col)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(col))
.append(" SET NOT NULL");
}
return sqlBuilder.append(";").toString();
@@ -623,19 +582,13 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
throw new
UnsupportedOperationException(POSTGRESQL_NOT_SUPPORT_NESTED_COLUMN_MSG);
}
return ALTER_TABLE
- + PG_QUOTE
- + tableName
- + PG_QUOTE
+ + quotedIdentifier(tableName)
+ " RENAME COLUMN "
- + PG_QUOTE
- + renameColumn.fieldName()[0]
- + PG_QUOTE
+ + quotedIdentifier(renameColumn.fieldName()[0])
+ SPACE
+ "TO"
+ SPACE
- + PG_QUOTE
- + renameColumn.getNewName()
- + PG_QUOTE
+ + quotedIdentifier(renameColumn.getNewName())
+ ";";
}
@@ -650,14 +603,10 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
StringBuilder columnDefinition = new StringBuilder();
columnDefinition
.append(ALTER_TABLE)
- .append(PG_QUOTE)
- .append(lazyLoadTable.name())
- .append(PG_QUOTE)
+ .append(quotedIdentifier(lazyLoadTable.name()))
.append(SPACE)
.append("ADD COLUMN ")
- .append(PG_QUOTE)
- .append(col)
- .append(PG_QUOTE)
+ .append(quotedIdentifier(col))
.append(SPACE)
.append(typeConverter.fromGravitino(addColumn.getDataType()))
.append(SPACE);
@@ -697,13 +646,9 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
if (StringUtils.isNotEmpty(addColumn.getComment())) {
result.add(
COLUMN_COMMENT
- + PG_QUOTE
- + lazyLoadTable.name()
- + PG_QUOTE
+ + quotedIdentifier(lazyLoadTable.name())
+ "."
- + PG_QUOTE
- + col
- + PG_QUOTE
+ + quotedIdentifier(col)
+ IS
+ escapeSqlLiteral(addColumn.getComment(), '\'')
+ "';");
@@ -719,13 +664,9 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
}
String col = updateColumnComment.fieldName()[0];
return COLUMN_COMMENT
- + PG_QUOTE
- + tableName
- + PG_QUOTE
+ + quotedIdentifier(tableName)
+ "."
- + PG_QUOTE
- + col
- + PG_QUOTE
+ + quotedIdentifier(col)
+ IS
+ escapeSqlLiteral(newComment, '\'')
+ "';";
@@ -744,6 +685,29 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
return metaData.getPrimaryKeys(metaData.getConnection().getCatalog(),
schemaName, tableName);
}
+ /**
+ * {@inheritDoc}
+ *
+ * <p>PostgreSQL uses the duplicate-relation SQLSTATE for index and
constraint name conflicts as
+ * well as table conflicts. During an alter this is an invalid change, not
an existing table.
+ */
+ @Override
+ public void alterTable(String schema, String tableName, TableChange...
changes) {
+ try {
+ super.alterTable(schema, tableName, changes);
+ } catch (TableAlreadyExistsException e) {
+ throw new IllegalArgumentException(
+ "PostgreSQL relation name conflict while altering table "
+ + schema
+ + "."
+ + tableName
+ + ": an index or constraint may already use this name. Choose an
unused name. "
+ + "PostgreSQL reason: "
+ + e.getMessage(),
+ e);
+ }
+ }
+
@Override
protected Connection getConnection(String schema) throws SQLException {
Connection connection = dataSource.getConnection();
@@ -752,18 +716,37 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
return connection;
}
+ /** {@inheritDoc} */
+ @Override
+ protected ResultSet getTables(Connection connection) throws SQLException {
+ DatabaseMetaData metadata = connection.getMetaData();
+ return metadata.getTables(
+ connection.getCatalog(),
+ metadataPattern(metadata, connection.getSchema()),
+ null,
+ JdbcConnectorUtils.getTableTypes());
+ }
+
@Override
protected ResultSet getTable(Connection connection, String schema, String
tableName)
throws SQLException {
DatabaseMetaData metaData = connection.getMetaData();
- return metaData.getTables(connection.getCatalog(), schema, tableName,
null);
+ return metaData.getTables(
+ connection.getCatalog(),
+ metadataPattern(metaData, schema),
+ metadataPattern(metaData, tableName),
+ null);
}
@Override
protected ResultSet getColumns(Connection connection, String schema, String
tableName)
throws SQLException {
DatabaseMetaData metaData = connection.getMetaData();
- return metaData.getColumns(connection.getCatalog(), schema, tableName,
null);
+ return metaData.getColumns(
+ connection.getCatalog(),
+ metadataPattern(metaData, schema),
+ metadataPattern(metaData, tableName),
+ null);
}
@Override
@@ -779,4 +762,27 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
return null;
}
}
+
+ private static String quotedIdentifier(String name) {
+ return PG_QUOTE + name.replace(PG_QUOTE, PG_QUOTE + PG_QUOTE) + PG_QUOTE;
+ }
+
+ private static void validateIndexName(@Nullable String name) {
+ if (name != null && name.getBytes(StandardCharsets.UTF_8).length > 63) {
+ throw new IllegalArgumentException(
+ "PostgreSQL index name must not exceed 63 UTF-8 bytes: " + name);
+ }
+ }
+
+ @Nullable
+ private static String metadataPattern(DatabaseMetaData metadata, @Nullable
String name)
+ throws SQLException {
+ if (name == null) {
+ return null;
+ }
+ String escape = metadata.getSearchStringEscape();
+ return name.replace(escape, escape + escape)
+ .replace("%", escape + "%")
+ .replace("_", escape + "_");
+ }
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/TestPostgreSqlCatalogCapability.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/TestPostgreSqlCatalogCapability.java
index f0acb184da..6f33006344 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/TestPostgreSqlCatalogCapability.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/TestPostgreSqlCatalogCapability.java
@@ -27,6 +27,37 @@ public class TestPostgreSqlCatalogCapability {
private final PostgreSqlCatalogCapability capability = new
PostgreSqlCatalogCapability();
+ @Test
+ void testNativeDeletionNamesRemainDistinctFromCreationNames() {
+ for (Capability.Scope scope :
+ new Capability.Scope[] {Capability.Scope.TABLE,
Capability.Scope.SCHEMA}) {
+ Assertions.assertTrue(
+ capability.specificationOnExistingName(scope,
"native\"name").supported());
+ Assertions.assertFalse(capability.specificationOnName(scope,
"native\"name").supported());
+ Assertions.assertFalse(
+ capability.specificationOnExistingName(scope,
"bad\0name").supported());
+ Assertions.assertFalse(capability.specificationOnExistingName(scope,
"").supported());
+ Assertions.assertFalse(
+ capability.specificationOnExistingName(scope,
"表".repeat(22)).supported());
+ }
+ Assertions.assertFalse(
+ capability.specificationOnExistingName(Capability.Scope.SCHEMA,
"pg_catalog").supported());
+ Assertions.assertFalse(
+ capability.specificationOnExistingName(Capability.Scope.TABLE,
"*").supported());
+ }
+
+ @Test
+ void testNamesAreLimitedByUtf8Bytes() {
+ for (Capability.Scope scope : Capability.Scope.values()) {
+ Assertions.assertTrue(capability.specificationOnName(scope,
"表".repeat(21)).supported());
+ Assertions.assertFalse(
+ capability.specificationOnName(scope, "表".repeat(21) +
"a").supported());
+ Assertions.assertTrue(
+ capability.specificationOnName(scope, "é".repeat(31) +
"a").supported());
+ Assertions.assertFalse(capability.specificationOnName(scope,
"é".repeat(32)).supported());
+ }
+ }
+
@Test
void testValidNames() {
// testing valid names for all scopes
@@ -178,6 +209,14 @@ public class TestPostgreSqlCatalogCapability {
Assertions.assertTrue(result.unsupportedMessage().contains("illegal"));
}
+ @Test
+ void testByteLimitDiagnostic() {
+ CapabilityResult result =
+ capability.specificationOnName(Capability.Scope.TABLE, "表".repeat(22));
+ Assertions.assertFalse(result.supported());
+ Assertions.assertTrue(result.unsupportedMessage().contains("63 UTF-8
bytes"));
+ }
+
@Test
void testMixedValidAndInvalidCharacters() {
// names that start valid but contain invalid characters should be rejected
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperations.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperations.java
index ff557b38ec..f59d7259a2 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperations.java
@@ -39,6 +39,32 @@ import org.junit.jupiter.api.Test;
@Tag("gravitino-docker-test")
public class TestPostgreSqlSchemaOperations extends TestPostgreSql {
+ @Test
+ void testQuotedSchemaNamesLoadWithoutExecutingSql() throws SQLException {
+ for (String schema : new String[] {"quoted';CREATE SCHEMA
injected_marker;--"}) {
+ DATABASE_OPERATIONS.create(schema, "safe comment", Map.of());
+ try {
+ Assertions.assertEquals("safe comment",
DATABASE_OPERATIONS.load(schema).comment());
+ Assertions.assertEquals(schema,
DATABASE_OPERATIONS.load(schema).name());
+
Assertions.assertFalse(DATABASE_OPERATIONS.listDatabases().contains("injected_marker"));
+ } finally {
+ DATABASE_OPERATIONS.delete(schema, true);
+ }
+ }
+ }
+
+ @Test
+ void testBackslashSchemaNameLoads() {
+ String schema = "back\\slash_%";
+ DATABASE_OPERATIONS.create(schema, "comment", Map.of());
+ try {
+ Assertions.assertEquals(schema, DATABASE_OPERATIONS.load(schema).name());
+ Assertions.assertEquals("comment",
DATABASE_OPERATIONS.load(schema).comment());
+ } finally {
+ DATABASE_OPERATIONS.delete(schema, true);
+ }
+ }
+
@Test
public void testBaseOperationSchema() {
String databaseName = RandomNameUtils.genRandomName("ct_db");
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
index c961b49537..9251ecf93a 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
@@ -18,7 +18,20 @@
*/
package org.apache.gravitino.catalog.postgresql.operation;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
import java.util.Collections;
+import javax.sql.DataSource;
+import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
+import
org.apache.gravitino.catalog.postgresql.converter.PostgreSqlExceptionConverter;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -37,4 +50,37 @@ public class TestPostgreSqlSchemaOperationsSqlGeneration {
+ "IS E'owner\\\\''s comment; DROP SCHEMA marker; --'",
sql);
}
+
+ @Test
+ void testSchemaNameIsBoundAsData() throws Exception {
+ DataSource source = mock(DataSource.class);
+ Connection connection = mock(Connection.class);
+ DatabaseMetaData metadata = mock(DatabaseMetaData.class);
+ ResultSet schemas = mock(ResultSet.class);
+ PreparedStatement statement = mock(PreparedStatement.class);
+ ResultSet comments = mock(ResultSet.class);
+ String name = "a\\b_%'; SELECT 1; --";
+ when(source.getConnection()).thenReturn(connection);
+ when(connection.getMetaData()).thenReturn(metadata);
+ when(metadata.getSearchStringEscape()).thenReturn("\\");
+ when(connection.getCatalog()).thenReturn("db");
+ when(metadata.getSchemas(eq("db"), anyString())).thenReturn(schemas);
+ when(schemas.next()).thenReturn(true, false);
+ when(schemas.getString(1)).thenReturn(name);
+ when(connection.prepareStatement(anyString())).thenReturn(statement);
+ when(statement.executeQuery()).thenReturn(comments);
+ PostgreSqlSchemaOperations operations = new PostgreSqlSchemaOperations();
+ operations.initialize(
+ source,
+ new PostgreSqlExceptionConverter(),
+ Collections.singletonMap(JdbcConfig.JDBC_DATABASE.getKey(), "db"));
+ Assertions.assertEquals(name, operations.load(name).name());
+ verify(metadata).getSchemas("db", "a\\\\b\\_\\%'; SELECT 1; --");
+ verify(statement).setString(1, name);
+ verify(connection)
+ .prepareStatement(
+ "SELECT obj_description(n.oid, 'pg_namespace') AS comment\n"
+ + "FROM pg_catalog.pg_namespace n\nWHERE n.nspname = ?;\n");
+ verify(statement).close();
+ }
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
index 6dfa17fe6e..1db20ecf54 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
@@ -39,12 +39,14 @@ import
org.apache.gravitino.catalog.postgresql.converter.PostgreSqlColumnDefault
import
org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.expressions.transforms.Transforms;
import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Index.IndexType;
import org.apache.gravitino.rel.indexes.Indexes;
import org.apache.gravitino.rel.types.Type;
import org.apache.gravitino.rel.types.Types;
@@ -61,6 +63,140 @@ public class TestPostgreSqlTableOperations extends
TestPostgreSql {
private static Type VARCHAR = Types.VarCharType.of(255);
private static Type INT = Types.IntegerType.get();
+ @ParameterizedTest
+ @ValueSource(strings = {"quoted\";CREATE TABLE injected_marker (id int);--"})
+ void testQuotedTableNamesStayIdentifiersDuringAlter(String name) {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ name,
+ new JdbcColumn[] {
+
JdbcColumn.builder().withName("id").withType(INT).withNullable(true).build()
+ },
+ "comment",
+ Map.of(),
+ null,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+ try {
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME, name, TableChange.renameColumn(new String[] {"id"},
"id\"quoted"));
+ Assertions.assertEquals(
+ "id\"quoted", TABLE_OPERATIONS.load(TEST_DB_NAME,
name).columns()[0].name());
+ TABLE_OPERATIONS.alterTable(TEST_DB_NAME, name,
TableChange.updateComment("updated"));
+ Assertions.assertEquals("updated", TABLE_OPERATIONS.load(TEST_DB_NAME,
name).comment());
+
Assertions.assertFalse(TABLE_OPERATIONS.listTables(TEST_DB_NAME).contains("injected_marker"));
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, name);
+ }
+ }
+
+ @Test
+ void testIndexNameConflictDoesNotReportExistingTable() throws SQLException {
+ String table = RandomNameUtils.genRandomName("index_conflict");
+ String index = RandomNameUtils.genRandomName("index_name");
+ try (Connection connection = DATA_SOURCE.getConnection()) {
+ JdbcConnectorUtils.executeUpdate(
+ connection, "CREATE TABLE " + TEST_DB_NAME + "." + table + "(id int,
other int)");
+ try {
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ table,
+ TableChange.addIndex(IndexType.UNIQUE_KEY, index, new String[][]
{{"id"}}));
+ IllegalArgumentException error =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ table,
+ TableChange.addIndex(
+ IndexType.UNIQUE_KEY, index, new String[][]
{{"other"}})));
+ Assertions.assertTrue(error.getMessage().contains(index));
+ Assertions.assertTrue(error.getMessage().contains("index or
constraint"));
+ Assertions.assertTrue(error.getCause() instanceof
TableAlreadyExistsException);
+ Assertions.assertEquals(1, TABLE_OPERATIONS.load(TEST_DB_NAME,
table).index().length);
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, table);
+ }
+ }
+ }
+
+ @Test
+ void testUnnamedUniqueIndexesDoNotCollideAcrossTables() {
+ String first = "quoted\"" + RandomNameUtils.genRandomName("unnamed_first");
+ String second = RandomNameUtils.genRandomName("unnamed_second");
+ for (String name : new String[] {first, second}) {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ name,
+ new JdbcColumn[] {
+
JdbcColumn.builder().withName("id").withType(INT).withNullable(true).build()
+ },
+ null,
+ Map.of(),
+ null,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+ }
+ try {
+ for (String name : new String[] {first, second}) {
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ name,
+ TableChange.addIndex(IndexType.UNIQUE_KEY, null, new String[][]
{{"id"}}));
+ JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, name);
+ Assertions.assertEquals(1, table.index().length);
+ Assertions.assertNotEquals("null", table.index()[0].name());
+ }
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, first);
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, second);
+ }
+ }
+
+ @Test
+ void testListTablesEscapesSchemaPattern() throws SQLException {
+ String schema = "list\\schema_%";
+ String neighbor = "listXschema_neighbor";
+ try (Connection connection = DATA_SOURCE.getConnection()) {
+ JdbcConnectorUtils.executeUpdate(connection, "CREATE SCHEMA \"" + schema
+ "\"");
+ JdbcConnectorUtils.executeUpdate(connection, "CREATE SCHEMA " +
neighbor);
+ try {
+ JdbcConnectorUtils.executeUpdate(
+ connection, "CREATE TABLE \"" + schema + "\".actual_table(id
int)");
+ JdbcConnectorUtils.executeUpdate(
+ connection, "CREATE TABLE " + neighbor + ".wrong_table(id int)");
+ Assertions.assertEquals(List.of("actual_table"),
TABLE_OPERATIONS.listTables(schema));
+ } finally {
+ JdbcConnectorUtils.executeUpdate(connection, "DROP SCHEMA \"" + schema
+ "\" CASCADE");
+ JdbcConnectorUtils.executeUpdate(connection, "DROP SCHEMA " + neighbor
+ " CASCADE");
+ }
+ }
+ }
+
+ @Test
+ void testBackslashTableNameLoadsAndAlters() {
+ String name = "back\\slash_%";
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ name,
+ new JdbcColumn[] {
+
JdbcColumn.builder().withName("id").withType(INT).withNullable(true).build()
+ },
+ "comment",
+ Map.of(),
+ null,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+ try {
+ Assertions.assertEquals("id", TABLE_OPERATIONS.load(TEST_DB_NAME,
name).columns()[0].name());
+ TABLE_OPERATIONS.alterTable(TEST_DB_NAME, name,
TableChange.updateComment("updated"));
+ Assertions.assertEquals("updated", TABLE_OPERATIONS.load(TEST_DB_NAME,
name).comment());
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, name);
+ }
+ }
+
@Test
public void testOperationTable() {
String tableName = RandomNameUtils.genRandomName("op_table_");
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
index 00958b7e93..3f2e81eaa7 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
@@ -29,6 +29,8 @@ import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.expressions.transforms.Transforms;
+import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Index.IndexType;
import org.apache.gravitino.rel.indexes.Indexes;
import org.apache.gravitino.rel.types.Types;
import org.junit.jupiter.api.Assertions;
@@ -81,6 +83,79 @@ public class TestPostgreSqlTableOperationsSqlGeneration {
}
}
+ @Test
+ void testQuotedTableAndColumnNamesCannotEscapeIdentifiers() {
+ TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
+ String name = "t\"; DROP TABLE victim; --";
+ String quoted = "\"t\"\"; DROP TABLE victim; --\"";
+ Assertions.assertEquals("DROP TABLE " + quoted,
ops.generateDropTableSql(name));
+ Assertions.assertEquals(
+ "ALTER TABLE " + quoted + " RENAME TO " + quoted,
ops.generateRenameTableSql(name, name));
+ Assertions.assertEquals(
+ "ALTER TABLE " + quoted + " RENAME COLUMN " + quoted + " TO " + quoted
+ ";",
+ ops.alterTableSql(name, TableChange.renameColumn(new String[] {name},
name)));
+ Assertions.assertEquals(
+ "ALTER TABLE " + quoted + " DROP CONSTRAINT " + quoted + ";",
+ ops.alterTableSql(name, TableChange.deleteIndex(name, false)));
+ Assertions.assertEquals(
+ quoted, PostgreSqlTableOperations.getIndexFieldStr(new String[][]
{{name}}));
+ String sql =
+ ops.createTableSql(
+ name,
+ new JdbcColumn[] {
+ JdbcColumn.builder()
+ .withName(name)
+ .withType(Types.IntegerType.get())
+ .withNullable(true)
+ .build()
+ });
+ Assertions.assertTrue(sql.startsWith("CREATE TABLE " + quoted + " ("));
+ Assertions.assertTrue(sql.contains(" " + quoted + " int4"));
+ }
+
+ @Test
+ void testOversizedIndexNamesAreRejectedOnCreateAndAlter() {
+ String name = "表".repeat(22);
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PostgreSqlTableOperations.addIndexDefinition(
+ "t",
+ (TableChange.AddIndex)
+ TableChange.addIndex(IndexType.UNIQUE_KEY, name, new
String[][] {{"col1"}})));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PostgreSqlTableOperations.appendIndexesSql(
+ new Index[] {Indexes.unique(name, new String[][] {{"col1"}})},
+ new StringBuilder()));
+ }
+
+ @Test
+ void testUnnamedIndexesKeepNativeTableAndColumnQuotes() {
+ for (String name : new String[] {null, ""}) {
+ Assertions.assertEquals(
+ "ALTER TABLE \"native\"\"table\" ADD UNIQUE (\"quoted\"\"column\");",
+ PostgreSqlTableOperations.addIndexDefinition(
+ "native\"table",
+ (TableChange.AddIndex)
+ TableChange.addIndex(
+ IndexType.UNIQUE_KEY, name, new String[][]
{{"quoted\"column"}})));
+ }
+ }
+
+ @Test
+ void testUnnamedIndexesUsePostgreSqlGeneratedNames() {
+ for (String name : new String[] {null, ""}) {
+ Assertions.assertEquals(
+ "ALTER TABLE \"t\" ADD UNIQUE (\"col1\");",
+ PostgreSqlTableOperations.addIndexDefinition(
+ "t",
+ (TableChange.AddIndex)
+ TableChange.addIndex(IndexType.UNIQUE_KEY, name, new
String[][] {{"col1"}})));
+ }
+ }
+
@ParameterizedTest
@ValueSource(booleans = {false, true})
void testDeleteIndexesDropsOnlyConstraints(boolean ifExists) {
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/TableNormalizeDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/TableNormalizeDispatcher.java
index aeac691a37..2f766588eb 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/TableNormalizeDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/TableNormalizeDispatcher.java
@@ -23,10 +23,12 @@ import static
org.apache.gravitino.catalog.CapabilityHelpers.applyCaseSensitive;
import static org.apache.gravitino.catalog.CapabilityHelpers.getCapability;
import static
org.apache.gravitino.catalog.CapabilityHelpers.resolvePhysicalTableName;
+import com.google.common.base.Preconditions;
import java.util.Map;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.connector.capability.CapabilityResult;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
@@ -133,6 +135,17 @@ public class TableNormalizeDispatcher implements
TableDispatcher {
private NameIdentifier normalizeNameIdentifier(NameIdentifier tableIdent) {
Capability capability = getCapability(tableIdent, catalogManager);
- return applyCapabilities(tableIdent, Capability.Scope.TABLE, capability);
+ Namespace namespace = tableIdent.namespace();
+ validateExistingName(
+ Capability.Scope.SCHEMA, namespace.level(namespace.length() - 1),
capability);
+ validateExistingName(Capability.Scope.TABLE, tableIdent.name(),
capability);
+ return applyCaseSensitive(tableIdent, Capability.Scope.TABLE, capability);
+ }
+
+ private static void validateExistingName(
+ Capability.Scope scope, String name, Capability capability) {
+ CapabilityResult result = capability.specificationOnExistingName(scope,
name);
+ Preconditions.checkArgument(
+ result.supported(), result.unsupportedMessage() + " Illegal name: " +
name);
}
}
diff --git
a/core/src/main/java/org/apache/gravitino/connector/capability/Capability.java
b/core/src/main/java/org/apache/gravitino/connector/capability/Capability.java
index 239cf6a8ce..ff6cd062f1 100644
---
a/core/src/main/java/org/apache/gravitino/connector/capability/Capability.java
+++
b/core/src/main/java/org/apache/gravitino/connector/capability/Capability.java
@@ -105,6 +105,21 @@ public interface Capability {
return DEFAULT.specificationOnName(scope, name);
}
+ /**
+ * Checks a table or schema name on an existing-table deletion path.
+ *
+ * <p>The default preserves the create-time restrictions. A catalog may
accept additional native
+ * names only when its deletion operations safely quote them. This does not
relax creation or
+ * rename validation.
+ *
+ * @param scope the table or schema scope
+ * @param name the existing name to check
+ * @return whether the existing name is supported
+ */
+ default CapabilityResult specificationOnExistingName(Scope scope, String
name) {
+ return specificationOnName(scope, name);
+ }
+
/**
* Check if the metadata entity is fully managed by Gravitino for the
passed-in scope. This is
* used to determine whether Gravitino should manage the lifecycle of the
metadata and related
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestTableNormalizeDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestTableNormalizeDispatcher.java
index 08f5a6c5fe..879c448d1c 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestTableNormalizeDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestTableNormalizeDispatcher.java
@@ -31,6 +31,8 @@ import org.apache.gravitino.TestColumn;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.CatalogOperations;
import org.apache.gravitino.connector.SupportsTableNameResolution;
+import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.connector.capability.CapabilityResult;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.apache.gravitino.rel.Column;
@@ -71,6 +73,55 @@ public class TestTableNormalizeDispatcher extends
TestOperationDispatcher {
TestTableOperationDispatcher.schemaOperationDispatcher,
catalogManager);
}
+ @Test
+ void testDeletionCanUseProviderValidatedNativeNamesWithoutRelaxingCreation()
throws Exception {
+ TableDispatcher delegate = Mockito.mock(TableDispatcher.class);
+ CatalogManager manager = Mockito.mock(CatalogManager.class);
+ BaseCatalog<?> catalog = Mockito.mock(BaseCatalog.class);
+ Capability capability =
+ new Capability() {
+ @Override
+ public CapabilityResult specificationOnExistingName(Scope scope,
String name) {
+ return CapabilityResult.SUPPORTED;
+ }
+ };
+ Mockito.when(catalog.capability()).thenReturn(capability);
+ CatalogTestUtils.mockDoWithCatalog(manager, catalog);
+ TableNormalizeDispatcher dispatcher = new
TableNormalizeDispatcher(delegate, manager);
+ NameIdentifier nativeIdent =
+ NameIdentifier.of("metalake", "catalog", "native schema",
"native\"table");
+ dispatcher.dropTable(nativeIdent);
+ dispatcher.purgeTable(nativeIdent);
+ Mockito.verify(delegate).dropTable(nativeIdent);
+ Mockito.verify(delegate).purgeTable(nativeIdent);
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ dispatcher.createTable(
+ nativeIdent,
+ new Column[0],
+ null,
+ Map.of(),
+ new Transform[0],
+ Distributions.NONE,
+ new SortOrder[0],
+ new Index[0]));
+ }
+
+ @Test
+ void testDeletionKeepsDefaultNameRestrictions() throws Exception {
+ TableDispatcher delegate = Mockito.mock(TableDispatcher.class);
+ CatalogManager manager = Mockito.mock(CatalogManager.class);
+ BaseCatalog<?> catalog = Mockito.mock(BaseCatalog.class);
+ Mockito.when(catalog.capability()).thenReturn(Capability.DEFAULT);
+ CatalogTestUtils.mockDoWithCatalog(manager, catalog);
+ TableNormalizeDispatcher dispatcher = new
TableNormalizeDispatcher(delegate, manager);
+ NameIdentifier invalid = NameIdentifier.of("metalake", "catalog",
"schema", "native\"table");
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
dispatcher.dropTable(invalid));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
dispatcher.purgeTable(invalid));
+ Mockito.verifyNoInteractions(delegate);
+ }
+
@Test
public void testNameCaseInsensitive() {
Namespace tableNs = Namespace.of(metalake, catalog, "schema81");
