This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-05e59885-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit a444d7d752f1832dcae5fc82990ebe51ada54bcf Author: Qi Yu <[email protected]> AuthorDate: Wed Nov 4 17:01:07 2026 +0800 [#13159] fix(postgresql): preserve scalar and array type metadata (#14521) ### What changes were proposed in this pull request? Preserve array element length, numeric precision/scale and supported temporal precision; map smallserial/serial/bigserial to their integer types. Includes the focused serial mapping from #10970 with original-author credit. Read PostgreSQL's actual character declarations instead of treating pgjdbc's configurable unknownLength as a declared bound. Unbounded varchar maps to String; bounded character types retain their lengths even when a bound equals unknownLength. Unbounded bpchar remains ExternalType("bpchar"), including array elements, so native DDL round trips retain its trailing-space-insensitive semantics. Consumers that read external types as strings still do not preserve native bpchar comparison semantics. Normalize pgjdbc's display-width metadata for unconstrained numeric arrays without changing explicitly constrained numeric declarations. Unbounded varchar defaults are represented as string literals consistently with their column type. Column metadata lookups preserve literal schema/table names, including backslashes and wildcard characters, when applying declaration normalization. ### Why are the changes needed? CHAR(n)[] loading loses element metadata, unbounded varchar can expose an invalid downstream length, and serial columns are exposed as unsupported external types. Copying unknown display lengths into array element constraints can additionally narrow valid source data; treating unbounded bpchar as text changes its type semantics. Fixes #13159 Fixes #14486 Fixes #10957 Consolidates #14487 into this PR. Existing array proposals #13162 and #13167 remain related; external contributors' PRs are not closed by this consolidation. ### Does this PR introduce _any_ user-facing change? Bounded character/decimal/time array element metadata is retained. Unbounded varchar maps to String, while unconstrained or unsupported numeric declarations and unbounded bpchar retain external representations. Native serial columns retain integer types and auto-increment metadata. Standard non-null serial columns that use their own sequence can be loaded and recreated as independent GENERATED BY DEFAULT AS IDENTITY columns; PostgreSQL create/add-column validation also accepts Short auto-increment columns. Sequence current values/options and GENERATED ALWAYS mode are not copied. Shared sequence defaults, custom expressions and serial columns altered to allow NULL remain unparsed and unsupported for recreation. This follow-up is intentionally limited to basic serial value generation, without expanding the public type system or implementing general PostgreSQL expression round trips. Loading a table containing character columns or sequence-backed defaults performs one additional read-only catalog query per table to distinguish real bounds from driver display lengths. Sequence ownership is resolved using catalog OIDs, preserving metadata loading without an additional schema USAGE requirement. No new configuration is required. Multi-dimensional arrays and deployed REST/Spark/Trino end-to-end behavior remain outside scope. ### How was this patch tested? - Full PostgreSQL module: 151 cases per run with pgjdbc 42.7.11, no failures or skips. Table/schema-operation containers ran on PostgreSQL 13 and then PostgreSQL 15; the separate data-source fixture remained PostgreSQL 13. Container-tagged cases were explicitly enabled and verified from test XML. - Restricted-role loading remains covered: metadata is readable without schema USAGE, table data remains forbidden, and serial/character metadata is preserved. The permission regression was verified to fail against the earlier name-based ownership query and pass with OID ownership lookup. - Character tests cover unknownLength values 0, 12, 255 and Integer.MAX_VALUE, matching declared bounds, defaults, scalar/array types, and copying 300-character values through a loaded schema. - A native bpchar schema round trip checks that scalar and array DISTINCT results still ignore trailing spaces. Existing numeric/temporal arrays and serial/auto-increment regressions remain covered. - Earlier type-fix mutation checks verified that reverting declaration normalization, bpchar preservation and unbounded-default conversion makes all six targeted regression cases fail. - A combined regression loads bounded/unbounded character scalars and arrays under schema/table names containing quotes, backslashes and wildcard characters. Removing JDBC pattern escaping makes this regression fail; restoring it passes the full suites. - Serial/identity regressions cover all three integer widths, CREATE and ADD COLUMN, quoted sequence/table names, ordinary defaults, and generation after dropping the source table. Shared, custom-owned and nullable-owned defaults remain unchanged and fail recreation before DDL. Reverting the serial fix makes five of seven targeted regression cases fail. - Explicit identity values are tested: an ALWAYS source rejects an explicit id with SQLSTATE 428C9, while recreated and added identity columns accept explicit values. Subsequent default inserts still generate 1/2/3, including after dropping the source table. - Temporarily changing recreated columns to GENERATED ALWAYS makes all six serial/identity parameter cases fail at the new explicit-insert assertion; restoring BY DEFAULT passes the full suite. - Affected-module Spotless, RAT and diff checks passed. Independent fresh-context cumulative review completed on the final code head. - Deployed REST/Spark/Trino and additional server versions were not tested. --------- Signed-off-by: 张万义 <[email protected]> Co-authored-by: 张万义 <[email protected]> Co-authored-by: Sachin <[email protected]> # Conflicts: # catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java --- .../PostgreSqlColumnDefaultValueConverter.java | 5 +- .../converter/PostgreSqlTypeConverter.java | 38 +- .../operation/PostgreSqlTableOperations.java | 133 ++++- .../TestPostgreSqlColumnDefaultValueConverter.java | 16 + .../converter/TestPostgreSqlTypeConverter.java | 109 +++++ .../operation/TestPostgreSqlTableOperations.java | 542 ++++++++++++++++++++- docs/jdbc-postgresql-catalog.md | 17 + 7 files changed, 846 insertions(+), 14 deletions(-) diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java index b115aa6fad..7dc3dc5398 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java @@ -130,7 +130,10 @@ public class PostgreSqlColumnDefaultValueConverter extends JdbcColumnDefaultValu return Literals.timestampLiteral( LocalDateTime.parse(columnDefaultValue, DATE_TIME_FORMATTER)); case VARCHAR: - return Literals.varcharLiteral(type.getColumnSize(), columnDefaultValue); + Integer length = type.getColumnSize(); + return length == null || length <= 0 || length == Integer.MAX_VALUE + ? Literals.stringLiteral(columnDefaultValue) + : Literals.varcharLiteral(length, columnDefaultValue); case PostgreSqlTypeConverter.BPCHAR: case JdbcTypeConverter.TEXT: return Literals.stringLiteral(columnDefaultValue); diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java index c87a63f86c..ceb66c3498 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java @@ -40,22 +40,30 @@ public class PostgreSqlTypeConverter extends JdbcTypeConverter { static final String BPCHAR = "bpchar"; static final String BYTEA = "bytea"; static final String UUID = "uuid"; + static final String SERIAL = "serial"; + static final String SMALLSERIAL = "smallserial"; + static final String BIGSERIAL = "bigserial"; @VisibleForTesting static final String JDBC_ARRAY_PREFIX = "_"; @VisibleForTesting static final String ARRAY_TOKEN = "[]"; + private static final int MAX_NUMERIC_PRECISION = 1000; + @Override public Type toGravitino(JdbcTypeBean typeBean) { String typeName = typeBean.getTypeName().toLowerCase(); if (typeName.startsWith(JDBC_ARRAY_PREFIX)) { - return toGravitinoArrayType(typeName); + return toGravitinoArrayType(typeName, typeBean); } switch (typeName) { case BOOL: return Types.BooleanType.get(); + case SMALLSERIAL: case INT_2: return Types.ShortType.get(); + case SERIAL: case INT_4: return Types.IntegerType.get(); + case BIGSERIAL: case INT_8: return Types.LongType.get(); case FLOAT_4: @@ -88,11 +96,19 @@ public class PostgreSqlTypeConverter extends JdbcTypeConverter { } return Types.DecimalType.of(columnSize, scale == null ? 0 : scale); case VARCHAR: + // Table metadata normalizes unbounded declarations to zero. Also accept pgjdbc's default + // sentinel for callers supplying driver metadata directly. return typeBean.getColumnSize() == null + || typeBean.getColumnSize() <= 0 + || typeBean.getColumnSize() == Integer.MAX_VALUE ? Types.StringType.get() : Types.VarCharType.of(typeBean.getColumnSize()); case BPCHAR: - return Types.FixedCharType.of(typeBean.getColumnSize()); + // An unbounded bpchar ignores trailing spaces and is not equivalent to String. + Integer length = typeBean.getColumnSize(); + return length == null || length <= 0 || length == Integer.MAX_VALUE + ? Types.ExternalType.of(BPCHAR) + : Types.FixedCharType.of(length); case TEXT: return Types.StringType.get(); case BYTEA: @@ -173,9 +189,25 @@ public class PostgreSqlTypeConverter extends JdbcTypeConverter { return elementTypeString + ARRAY_TOKEN; } - private ListType toGravitinoArrayType(String typeName) { + private ListType toGravitinoArrayType(String typeName, JdbcTypeBean columnBean) { String elementTypeName = typeName.substring(JDBC_ARRAY_PREFIX.length(), typeName.length()); + // PostgreSQL has no dedicated element metadata for array columns: the driver reports the + // element's type information (length, scale, precision) on the array column itself, encoded + // through atttypmod. Propagate it to the element bean so constrained character, numeric and + // temporal element types keep the same metadata as their scalar counterparts. JdbcTypeBean bean = new JdbcTypeBean(elementTypeName); + Integer columnSize = columnBean.getColumnSize(); + // pgjdbc getColumns substitutes the display width (131089) when an unconstrained NUMERIC + // array's precision is zero, because its JDBC type is ARRAY rather than NUMERIC. That width + // is not a declared precision: PostgreSQL only allows explicit precision up to 1000. + if (NUMERIC.equals(elementTypeName) + && columnSize != null + && columnSize > MAX_NUMERIC_PRECISION) { + columnSize = 0; + } + bean.setColumnSize(columnSize); + bean.setScale(columnBean.getScale()); + bean.setDatetimePrecision(columnBean.getDatetimePrecision()); return ListType.nullable(toGravitino(bean)); } } 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 194c7dd66b..e5ceaff71d 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 @@ -31,12 +31,17 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; import javax.sql.DataSource; +import javax.sql.rowset.CachedRowSet; +import javax.sql.rowset.RowSetProvider; import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.BooleanUtils; @@ -65,6 +70,7 @@ import org.apache.gravitino.rel.expressions.literals.Literals; import org.apache.gravitino.rel.expressions.sorts.SortOrder; import org.apache.gravitino.rel.expressions.transforms.Transform; import org.apache.gravitino.rel.indexes.Index; +import org.apache.gravitino.rel.types.Type; import org.apache.gravitino.rel.types.Types; /** Table operations for PostgreSQL. */ @@ -317,12 +323,16 @@ public class PostgreSqlTableOperations extends JdbcTableOperations .collect(Collectors.joining(", ")); } + private static boolean allowsAutoIncrement(Type type) { + return type instanceof Types.ShortType || Types.allowAutoIncrement(type); + } + private void appendColumnDefinition(JdbcColumn column, StringBuilder sqlBuilder) { // Add data type sqlBuilder.append(SPACE).append(typeConverter.fromGravitino(column.dataType())).append(SPACE); if (column.autoIncrement()) { - if (!Types.allowAutoIncrement(column.dataType())) { + if (!allowsAutoIncrement(column.dataType())) { throw new IllegalArgumentException( "Unsupported auto-increment , column: " + column.name() @@ -661,7 +671,7 @@ public class PostgreSqlTableOperations extends JdbcTableOperations .append(SPACE); if (addColumn.isAutoIncrement()) { - if (!Types.allowAutoIncrement(addColumn.getDataType())) { + if (!allowsAutoIncrement(addColumn.getDataType())) { throw new IllegalArgumentException( "Unsupported auto-increment , column: " + Arrays.toString(addColumn.getFieldName()) @@ -792,16 +802,123 @@ public class PostgreSqlTableOperations extends JdbcTableOperations protected ResultSet getColumns(Connection connection, String schema, String tableName) throws SQLException { DatabaseMetaData metaData = connection.getMetaData(); - return metaData.getColumns( - connection.getCatalog(), - metadataPattern(metaData, schema), - metadataPattern(metaData, tableName), - null); + CachedRowSet columns = RowSetProvider.newFactory().createCachedRowSet(); + try (ResultSet metadata = + metaData.getColumns( + connection.getCatalog(), + metadataPattern(metaData, schema), + metadataPattern(metaData, tableName), + null)) { + columns.populate(metadata); + } + + boolean needsDeclarations = false; + while (columns.next()) { + String type = columns.getString("TYPE_NAME"); + if ("varchar".equals(type) + || "bpchar".equals(type) + || "_varchar".equals(type) + || "_bpchar".equals(type) + || (getAutoIncrementInfo(columns) && columns.getString("COLUMN_DEF") != null)) { + needsDeclarations = true; + break; + } + } + columns.beforeFirst(); + if (!needsDeclarations) { + return columns; + } + + // COLUMN_SIZE substitutes the configurable pgjdbc unknownLength for unbounded character + // types. Serial defaults also need their declaration: JDBC marks shared sequence defaults as + // auto-incrementing too. One catalog query resolves both without querying per column. + Set<String> unboundedColumns = new HashSet<>(); + Set<String> serialColumns = new HashSet<>(); + // Match only a bare nextval call and verify its dependency on the owned sequence. Ownership + // alone would also match custom defaults such as nextval(owned_sequence) + 1. + // Resolve ownership by OID: pg_get_serial_sequence would resolve the table name and require + // schema USAGE even when JDBC and catalog metadata are readable without that privilege. + String sql = + "SELECT a.attname, a.atttypmod = -1 AND a.atttypid IN " + + "('pg_catalog.varchar'::pg_catalog.regtype, 'pg_catalog.bpchar'::pg_catalog.regtype, " + + "'pg_catalog.varchar[]'::pg_catalog.regtype, " + + "'pg_catalog.bpchar[]'::pg_catalog.regtype) AS unbounded_character, " + + "a.attnotnull AND a.atttypid IN ('pg_catalog.int2'::pg_catalog.regtype, " + + "'pg_catalog.int4'::pg_catalog.regtype, 'pg_catalog.int8'::pg_catalog.regtype) " + + "AND pg_catalog.pg_get_expr(d.adbin, d.adrelid) ~ " + + "'^nextval[(]''([^'']|'''')*''::regclass[)]$' " + + "AND EXISTS (SELECT 1 FROM pg_catalog.pg_depend dep " + + "WHERE dep.classid = 'pg_catalog.pg_attrdef'::pg_catalog.regclass " + + "AND dep.objid = d.oid AND dep.refclassid = 'pg_catalog.pg_class'::pg_catalog.regclass " + + "AND dep.deptype = 'n' " + + "AND EXISTS (SELECT 1 FROM pg_catalog.pg_depend owned " + + "JOIN pg_catalog.pg_class seq ON seq.oid = owned.objid AND seq.relkind = 'S' " + + "WHERE owned.classid = 'pg_catalog.pg_class'::pg_catalog.regclass " + + "AND owned.objid = dep.refobjid AND owned.objsubid = 0 " + + "AND owned.refclassid = 'pg_catalog.pg_class'::pg_catalog.regclass " + + "AND owned.refobjid = a.attrelid AND owned.refobjsubid = a.attnum " + + "AND owned.deptype IN ('a', 'i'))) AS owned_serial " + + "FROM pg_catalog.pg_attribute a " + + "JOIN pg_catalog.pg_class c ON a.attrelid = c.oid " + + "JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid " + + "LEFT JOIN pg_catalog.pg_attrdef d ON d.adrelid = a.attrelid AND d.adnum = a.attnum " + + "WHERE n.nspname = ? AND c.relname = ? AND a.attnum > 0 AND NOT a.attisdropped"; + try (PreparedStatement statement = connection.prepareStatement(sql)) { + statement.setString(1, schema); + statement.setString(2, tableName); + try (ResultSet declarations = statement.executeQuery()) { + while (declarations.next()) { + if (declarations.getBoolean("unbounded_character")) { + unboundedColumns.add(declarations.getString("attname")); + } + if (declarations.getBoolean("owned_serial")) { + serialColumns.add(declarations.getString("attname")); + } + } + } + } + while (columns.next()) { + boolean updated = false; + String name = columns.getString("COLUMN_NAME"); + if (unboundedColumns.contains(name)) { + // Zero is the converter's canonical unknown length, independent of driver configuration. + columns.updateInt("COLUMN_SIZE", 0); + updated = true; + } + if (getAutoIncrementInfo(columns) && serialColumns.contains(name)) { + // Preserve basic auto-increment semantics using a new IDENTITY on recreation, rather + // than copying a default that still depends on the source table's owned sequence. + // Shared sequences and custom expressions deliberately retain their original defaults. + columns.updateNull("COLUMN_DEF"); + updated = true; + } + if (updated) { + columns.updateRow(); + } + } + columns.beforeFirst(); + return columns; + } + + /** + * Reads the JDBC YES/NO marker without relying on pgjdbc's boolean coercion, which a cached + * metadata row set does not provide. + * + * @param resultSet the column metadata positioned on the current column + * @return whether the column is auto-incrementing + * @throws SQLException if the metadata cannot be read + */ + @Override + protected boolean getAutoIncrementInfo(ResultSet resultSet) throws SQLException { + return "YES".equals(resultSet.getString("IS_AUTOINCREMENT")); } @Override public Integer calculateDatetimePrecision(String typeName, int columnSize, int scale) { - String upperTypeName = typeName.toUpperCase(); + String upperTypeName = typeName.toUpperCase(Locale.ROOT); + if (upperTypeName.startsWith("_")) { + upperTypeName = upperTypeName.substring(1); + } switch (upperTypeName) { case "TIME": case "TIMETZ": diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java index ec1825323a..dcf29eab48 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java @@ -76,4 +76,20 @@ public class TestPostgreSqlColumnDefaultValueConverter { Assertions.assertEquals( Literals.stringLiteral(""), converter.toGravitino(type, "''::bpchar", false, true)); } + + @Test + void testUnboundedVarcharDefaultIsString() { + for (Integer length : new Integer[] {null, 0, Integer.MAX_VALUE}) { + JdbcTypeConverter.JdbcTypeBean type = new JdbcTypeConverter.JdbcTypeBean("varchar"); + type.setColumnSize(length); + Assertions.assertEquals( + Literals.stringLiteral("value"), + converter.toGravitino(type, "'value'::character varying", false, true)); + } + JdbcTypeConverter.JdbcTypeBean bounded = new JdbcTypeConverter.JdbcTypeBean("varchar"); + bounded.setColumnSize(12); + Assertions.assertEquals( + Literals.varcharLiteral(12, "value"), + converter.toGravitino(bounded, "'value'::character varying", false, true)); + } } diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java index 23a65e19a0..f44a5038f4 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java @@ -24,6 +24,7 @@ import static org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter.TIME import static org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter.TIMESTAMP; import static org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter.VARCHAR; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.ARRAY_TOKEN; +import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BIGSERIAL; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BOOL; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BPCHAR; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BYTEA; @@ -34,6 +35,8 @@ import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeCo import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.INT_8; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.JDBC_ARRAY_PREFIX; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.NUMERIC; +import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.SERIAL; +import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.SMALLSERIAL; import static org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.UUID; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; @@ -51,8 +54,19 @@ public class TestPostgreSqlTypeConverter { new PostgreSqlTypeConverter(); private static final String USER_DEFINED_TYPE = "user-defined"; + @Test + void testUnboundedVarcharDriverSentinelMapsToString() { + checkJdbcTypeToGravitinoType(Types.StringType.get(), VARCHAR, Integer.MAX_VALUE, null, 0); + checkJdbcTypeToGravitinoType(Types.StringType.get(), VARCHAR, null, null, 0); + checkJdbcTypeToGravitinoType(Types.StringType.get(), VARCHAR, 0, null, 0); + checkJdbcTypeToGravitinoType(Types.VarCharType.of(255), VARCHAR, 255, null, 0); + } + @Test public void testToGravitinoType() { + checkJdbcTypeToGravitinoType(Types.ShortType.get(), SMALLSERIAL, null, null, 0); + checkJdbcTypeToGravitinoType(Types.IntegerType.get(), SERIAL, null, null, 0); + checkJdbcTypeToGravitinoType(Types.LongType.get(), BIGSERIAL, null, null, 0); checkJdbcTypeToGravitinoType(Types.BooleanType.get(), BOOL, null, null, 0); checkJdbcTypeToGravitinoType(Types.ShortType.get(), INT_2, null, null, 0); checkJdbcTypeToGravitinoType(Types.IntegerType.get(), INT_4, null, null, 0); @@ -105,6 +119,101 @@ public class TestPostgreSqlTypeConverter { } @Test +<<<<<<< HEAD +======= + public void testCharArrayArrayType() { + // CHAR(n)[] (type name "_bpchar") carries its element length through the column size; the + // element bean must keep it or toGravitino unboxes a null column size and fails with an NPE. + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.FixedCharType.of(5)), + JDBC_ARRAY_PREFIX + BPCHAR, + 5, + null, + null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.VarCharType.of(20)), + JDBC_ARRAY_PREFIX + VARCHAR, + 20, + null, + null); + } + + @Test + void testCharArrayWithoutLengthMetadataStillLoads() { + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("bpchar")), "_bpchar", null, null, null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("bpchar")), "_bpchar", 0, null, null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("bpchar")), + "_bpchar", + Integer.MAX_VALUE, + null, + null); + checkJdbcTypeToGravitinoType( + Types.ExternalType.of("bpchar"), BPCHAR, Integer.MAX_VALUE, null, null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.StringType.get()), "_varchar", Integer.MAX_VALUE, null, null); + } + + @Test + void testNumericArrayElementMetadata() { + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.DecimalType.of(10, 2)), "_numeric", 10, 2, null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("numeric")), "_numeric", 0, 0, null); + // pgjdbc getColumns reports display width rather than precision for unconstrained arrays. + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("numeric")), "_numeric", 131089, 0, null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("numeric(1000,0)")), + "_numeric", + 1000, + 0, + null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("numeric(39,0)")), "_numeric", 39, 0, null); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.ExternalType.of("numeric(2,-3)")), "_numeric", 2, 2045, null); + } + + @Test + void testTemporalArrayElementMetadata() { + for (int precision : new int[] {0, 3, 6}) { + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.TimeType.of(precision)), "_time", null, null, precision); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.TimestampType.withoutTimeZone(precision)), + "_timestamp", + null, + null, + precision); + checkJdbcTypeToGravitinoType( + Types.ListType.nullable(Types.TimestampType.withTimeZone(precision)), + "_timestamptz", + null, + null, + precision); + } + } + + @Test + public void testConstrainedNumericOutsideDecimalLimits() { + checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(39,0)"), NUMERIC, 39, 0, 0); + checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(1000,0)"), NUMERIC, 1000, 0, 0); + checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(2,-3)"), NUMERIC, 2, -3, 0); + // PostgreSQL JDBC metadata can expose a negative scale as an unsigned 11-bit value. + checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(2,-3)"), NUMERIC, 2, 2045, 0); + checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(3,5)"), NUMERIC, 3, 5, 0); + checkJdbcTypeToGravitinoType(Types.DecimalType.of(38, 38), NUMERIC, 38, 38, 0); + checkJdbcTypeToGravitinoType(Types.DecimalType.of(38, 0), NUMERIC, 38, null, 0); + for (String declaration : new String[] {"numeric(39,0)", "numeric(2,-3)", "numeric(3,5)"}) { + checkGravitinoTypeToJdbcType(declaration, Types.ExternalType.of(declaration)); + } + } + + @Test +>>>>>>> 05e59885a ([#13159] fix(postgresql): preserve scalar and array type metadata (#14521)) public void testFromGravitinoType() { checkGravitinoTypeToJdbcType(BOOL, Types.BooleanType.get()); checkGravitinoTypeToJdbcType(INT_2, Types.ShortType.get()); 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 54ecea04c4..74374f9193 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 @@ -20,7 +20,9 @@ package org.apache.gravitino.catalog.postgresql.operation; import com.google.common.collect.Maps; import java.sql.Connection; +import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.Statement; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -45,6 +47,7 @@ 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.UnparsedExpression; import org.apache.gravitino.rel.expressions.distributions.Distributions; import org.apache.gravitino.rel.expressions.literals.Literals; import org.apache.gravitino.rel.expressions.transforms.Transforms; @@ -200,6 +203,83 @@ public class TestPostgreSqlTableOperations extends TestPostgreSql { } } + @Test + void testLoadMetadataWithoutSchemaUsage() throws SQLException { + String role = RandomNameUtils.genRandomName("metadata_reader"); + String schema = RandomNameUtils.genRandomName("metadata_schema"); + String table = RandomNameUtils.genRandomName("metadata_table"); + try (Connection admin = DATA_SOURCE.getConnection()) { + JdbcConnectorUtils.executeUpdate( + admin, "CREATE ROLE " + role + " LOGIN PASSWORD 'test-only-password'"); + try { + JdbcConnectorUtils.executeUpdate(admin, "CREATE SCHEMA " + schema); + JdbcConnectorUtils.executeUpdate(admin, "REVOKE ALL ON SCHEMA " + schema + " FROM PUBLIC"); + JdbcConnectorUtils.executeUpdate( + admin, + "CREATE TABLE " + + schema + + "." + + table + + " (id serial, label varchar, tags varchar(12)[])"); + JdbcConnectorUtils.executeUpdate( + admin, "GRANT SELECT ON " + schema + "." + table + " TO " + role); + Map<String, String> properties = new HashMap<>(); + properties.put("jdbc-driver", CONTAINER.getDriverClassName()); + properties.put("jdbc-url", CONTAINER.getJdbcUrl()); + properties.put("jdbc-database", admin.getCatalog()); + properties.put("jdbc-user", role); + properties.put("jdbc-password", "test-only-password"); + properties.put("jdbc.pool.max-size", "1"); + properties.put("jdbc.pool.min-size", "1"); + try (BasicDataSource limitedSource = + (BasicDataSource) DataSourceUtils.createDataSource(properties)) { + PostgreSqlTableOperations limited = new PostgreSqlTableOperations(); + limited.initialize( + limitedSource, + JDBC_EXCEPTION_CONVERTER, + new PostgreSqlTypeConverter(), + new PostgreSqlColumnDefaultValueConverter(), + properties); + try (Connection connection = limitedSource.getConnection(); + Statement statement = connection.createStatement()) { + try (ResultSet privileges = + statement.executeQuery( + "SELECT pg_catalog.has_schema_privilege(current_user, '" + + schema + + "', 'USAGE')")) { + Assertions.assertTrue(privileges.next()); + Assertions.assertFalse(privileges.getBoolean(1)); + } + try (ResultSet columns = + connection.getMetaData().getColumns(connection.getCatalog(), schema, table, null)) { + int count = 0; + while (columns.next()) { + count++; + } + Assertions.assertEquals(3, count); + } + SQLException denied = + Assertions.assertThrows( + SQLException.class, + () -> statement.executeQuery("SELECT * FROM " + schema + "." + table)); + Assertions.assertEquals("42501", denied.getSQLState()); + } + JdbcTable loaded = limited.load(schema, table); + Assertions.assertEquals(3, loaded.columns().length); + Assertions.assertEquals(Types.IntegerType.get(), loaded.columns()[0].dataType()); + Assertions.assertTrue(loaded.columns()[0].autoIncrement()); + Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET, loaded.columns()[0].defaultValue()); + Assertions.assertEquals(Types.StringType.get(), loaded.columns()[1].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.VarCharType.of(12)), loaded.columns()[2].dataType()); + } + } finally { + JdbcConnectorUtils.executeUpdate(admin, "DROP SCHEMA IF EXISTS " + schema + " CASCADE"); + JdbcConnectorUtils.executeUpdate(admin, "DROP ROLE " + role); + } + } + } + @Test void testLeastPrivilegeCreateDistinguishesMissingSchemaFromForbidden() throws SQLException { String role = RandomNameUtils.genRandomName("limited_role"); @@ -348,6 +428,449 @@ public class TestPostgreSqlTableOperations extends TestPostgreSql { } } + @Test + void testNativeSerialColumnsRetainIntegerTypesAndAutoIncrement() throws SQLException { + String name = RandomNameUtils.genRandomName("serial_types"); + try (Connection connection = DATA_SOURCE.getConnection()) { + JdbcConnectorUtils.executeUpdate( + connection, + "CREATE TABLE " + + TEST_DB_NAME + + "." + + name + + " (small_id smallserial, id serial, big_id bigserial)"); + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, name); + Assertions.assertEquals(Types.ShortType.get(), table.columns()[0].dataType()); + Assertions.assertEquals(Types.IntegerType.get(), table.columns()[1].dataType()); + Assertions.assertEquals(Types.LongType.get(), table.columns()[2].dataType()); + for (Column column : table.columns()) { + Assertions.assertTrue(column.autoIncrement()); + Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET, column.defaultValue()); + } + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, name); + } + } + } + + @ParameterizedTest + @ValueSource( + strings = { + "smallserial", + "serial", + "bigserial", + "smallint GENERATED BY DEFAULT AS IDENTITY", + "integer GENERATED ALWAYS AS IDENTITY", + "bigint GENERATED BY DEFAULT AS IDENTITY" + }) + void testNativeAutoIncrementColumnsCanBeRecreated(String declaration) throws SQLException { + String source = RandomNameUtils.genRandomName("serial_source") + "_'\"\\%_"; + String target = RandomNameUtils.genRandomName("serial_copy"); + String qualifiedSource = TEST_DB_NAME + ".\"" + source.replace("\"", "\"\"") + "\""; + try (Connection connection = DATA_SOURCE.getConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate( + "CREATE TABLE " + + qualifiedSource + + " (id " + + declaration + + ", ordinary int DEFAULT 42, label varchar DEFAULT 'value')"); + try { + statement.executeUpdate("INSERT INTO " + qualifiedSource + " DEFAULT VALUES"); + if (declaration.contains("GENERATED ALWAYS")) { + SQLException error = + Assertions.assertThrows( + SQLException.class, + () -> + statement.executeUpdate( + "INSERT INTO " + qualifiedSource + " (id) VALUES (1000)")); + Assertions.assertEquals("428C9", error.getSQLState()); + } + JdbcTable loaded = TABLE_OPERATIONS.load(TEST_DB_NAME, source); + Column id = loaded.columns()[0]; + Assertions.assertTrue(id.autoIncrement()); + Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET, id.defaultValue()); + Assertions.assertFalse(id.nullable()); + TABLE_OPERATIONS.create( + TEST_DB_NAME, + target, + Arrays.stream(loaded.columns()).map(JdbcColumn.class::cast).toArray(JdbcColumn[]::new), + null, + Map.of(), + null, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME, + target, + TableChange.addColumn( + new String[] {"added_id"}, + id.dataType(), + null, + TableChange.ColumnPosition.defaultPos(), + false, + true, + id.defaultValue())); + // Recreation preserves basic generation through BY DEFAULT, including for an ALWAYS + // source. Explicit values remain accepted by both the copied and added identity columns. + try (ResultSet rows = + statement.executeQuery( + "INSERT INTO " + + TEST_DB_NAME + + "." + + target + + " (id, added_id) VALUES (1000, 2000) RETURNING id, added_id")) { + Assertions.assertTrue(rows.next()); + Assertions.assertEquals(1000, rows.getLong("id")); + Assertions.assertEquals(2000, rows.getLong("added_id")); + Assertions.assertFalse(rows.next()); + } + for (int expected = 1; expected <= 3; expected++) { + if (expected == 3) { + // Dropping the original table must not remove either generator on the new table. + TABLE_OPERATIONS.drop(TEST_DB_NAME, source); + } + try (ResultSet rows = + statement.executeQuery( + "INSERT INTO " + + TEST_DB_NAME + + "." + + target + + " DEFAULT VALUES RETURNING id, added_id, ordinary, label")) { + Assertions.assertTrue(rows.next()); + Assertions.assertEquals(expected, rows.getLong("id")); + Assertions.assertEquals(expected, rows.getLong("added_id")); + Assertions.assertEquals(42, rows.getInt("ordinary")); + Assertions.assertEquals("value", rows.getString("label")); + } + } + JdbcTable copy = TABLE_OPERATIONS.load(TEST_DB_NAME, target); + for (int index : new int[] {0, 3}) { + Assertions.assertEquals(id.dataType(), copy.columns()[index].dataType()); + Assertions.assertTrue(copy.columns()[index].autoIncrement()); + Assertions.assertEquals( + Column.DEFAULT_VALUE_NOT_SET, copy.columns()[index].defaultValue()); + } + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, target); + TABLE_OPERATIONS.drop(TEST_DB_NAME, source); + } + } + } + + @ParameterizedTest + @ValueSource(strings = {"shared", "owned_custom", "nullable_owned"}) + void testNonSerialSequenceDefaultsRemainUnparsed(String scenario) throws SQLException { + boolean ownedCustomExpression = scenario.equals("owned_custom"); + boolean ownedSequence = !scenario.equals("shared"); + String source = RandomNameUtils.genRandomName("sequence_default"); + String target = source + "_copy"; + String sequence = source + "_seq"; + try (Connection connection = DATA_SOURCE.getConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate("CREATE SEQUENCE " + TEST_DB_NAME + "." + sequence); + try { + statement.executeUpdate( + "CREATE TABLE " + + TEST_DB_NAME + + "." + + source + + " (id integer " + + (scenario.equals("nullable_owned") ? "" : "NOT NULL ") + + "DEFAULT nextval('" + + TEST_DB_NAME + + "." + + sequence + + "'::regclass)" + + (ownedCustomExpression ? " + 1" : "") + + ")"); + if (ownedSequence) { + statement.executeUpdate( + "ALTER SEQUENCE " + + TEST_DB_NAME + + "." + + sequence + + " OWNED BY " + + TEST_DB_NAME + + "." + + source + + ".id"); + } + JdbcColumn id = (JdbcColumn) TABLE_OPERATIONS.load(TEST_DB_NAME, source).columns()[0]; + Assertions.assertTrue(id.autoIncrement()); + Assertions.assertInstanceOf(UnparsedExpression.class, id.defaultValue()); + IllegalArgumentException error = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + TABLE_OPERATIONS.create( + TEST_DB_NAME, + target, + new JdbcColumn[] {id}, + null, + Map.of(), + null, + Distributions.NONE, + Indexes.EMPTY_INDEXES)); + Assertions.assertTrue(error.getMessage().contains("Not a supported column default value")); + Assertions.assertFalse(TABLE_OPERATIONS.listTables(TEST_DB_NAME).contains(target)); + TABLE_OPERATIONS.drop(TEST_DB_NAME, source); + if (!ownedSequence) { + try (ResultSet rows = + statement.executeQuery( + "SELECT nextval('" + TEST_DB_NAME + "." + sequence + "'::regclass)")) { + Assertions.assertTrue(rows.next()); + Assertions.assertEquals(1, rows.getLong(1)); + } + } + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, target); + TABLE_OPERATIONS.drop(TEST_DB_NAME, source); + statement.executeUpdate("DROP SEQUENCE IF EXISTS " + TEST_DB_NAME + "." + sequence); + } + } + } + + @Test + void testArrayElementMetadataFromSource() throws SQLException { + String name = RandomNameUtils.genRandomName("array_metadata"); + try (Connection connection = DATA_SOURCE.getConnection()) { + JdbcConnectorUtils.executeUpdate( + connection, + "CREATE TABLE " + + TEST_DB_NAME + + "." + + name + + "(c char(3)[], v varchar(12)[], n numeric(10,2)[], ts timestamp(3)[], wide numeric(39,0)[], " + + "unbounded varchar[], unconstrained numeric[], t time(0)[], tz timestamptz(6)[], " + + "max_precision numeric(1000,0)[])"); + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, name); + Assertions.assertEquals( + Types.ListType.nullable(Types.FixedCharType.of(3)), table.columns()[0].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.VarCharType.of(12)), table.columns()[1].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.DecimalType.of(10, 2)), table.columns()[2].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.TimestampType.withoutTimeZone(3)), + table.columns()[3].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.ExternalType.of("numeric(39,0)")), + table.columns()[4].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.StringType.get()), table.columns()[5].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.ExternalType.of("numeric")), + table.columns()[6].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.TimeType.of(0)), table.columns()[7].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.TimestampType.withTimeZone(6)), + table.columns()[8].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.ExternalType.of("numeric(1000,0)")), + table.columns()[9].dataType()); + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, name); + } + } + } + + @Test + void testNativeUnboundedBpcharPreservesSemanticsOnRoundTrip() throws SQLException { + String name = RandomNameUtils.genRandomName("unbounded_bpchar"); + String copy = RandomNameUtils.genRandomName("bpchar_copy"); + try (Connection connection = DATA_SOURCE.getConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate( + "CREATE TABLE " + + TEST_DB_NAME + + "." + + name + + " (label bpchar, labels bpchar[], bounded char(3))"); + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, name); + Assertions.assertEquals(Types.ExternalType.of("bpchar"), table.columns()[0].dataType()); + Assertions.assertEquals( + Types.ListType.nullable(Types.ExternalType.of("bpchar")), + table.columns()[1].dataType()); + Assertions.assertEquals(Types.FixedCharType.of(3), table.columns()[2].dataType()); + TABLE_OPERATIONS.create( + TEST_DB_NAME, + copy, + Arrays.stream(table.columns()).map(JdbcColumn.class::cast).toArray(JdbcColumn[]::new), + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + statement.executeUpdate( + "INSERT INTO " + + TEST_DB_NAME + + "." + + copy + + " (label, labels) VALUES ('a', '{a}'), ('a ', '{\"a \"}')"); + try (ResultSet result = + statement.executeQuery( + "SELECT count(DISTINCT label), count(DISTINCT labels) FROM " + + TEST_DB_NAME + + "." + + copy)) { + Assertions.assertTrue(result.next()); + Assertions.assertEquals(1, result.getInt(1)); + Assertions.assertEquals(1, result.getInt(2)); + } + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, copy); + TABLE_OPERATIONS.drop(TEST_DB_NAME, name); + } + } + } + + @ParameterizedTest + @ValueSource(ints = {0, 12, 255, Integer.MAX_VALUE}) + void testCharacterDeclarationsDoNotDependOnDriverUnknownLength(int unknownLength) + throws SQLException { + Map<String, String> properties = new HashMap<>(); + properties.put(JdbcConfig.JDBC_DRIVER.getKey(), CONTAINER.getDriverClassName()); + String jdbcUrl = CONTAINER.getJdbcUrl(); + properties.put( + JdbcConfig.JDBC_URL.getKey(), + jdbcUrl + (jdbcUrl.contains("?") ? "&" : "?") + "unknownLength=" + unknownLength); + properties.put(JdbcConfig.USERNAME.getKey(), CONTAINER.getUsername()); + properties.put(JdbcConfig.PASSWORD.getKey(), CONTAINER.getPassword()); + DataSource dataSource = DataSourceUtils.createDataSource(properties); + PostgreSqlTableOperations operations = new PostgreSqlTableOperations(); + operations.initialize( + dataSource, + JDBC_EXCEPTION_CONVERTER, + new PostgreSqlTypeConverter(), + new PostgreSqlColumnDefaultValueConverter(), + Map.of(JdbcConfig.JDBC_DATABASE.getKey(), CONTAINER.getDatabaseName())); + operations.setDatabaseOperation(DATABASE_OPERATIONS); + String name = RandomNameUtils.genRandomName("character_lengths"); + String copy = RandomNameUtils.genRandomName("character_copy"); + try (Connection connection = dataSource.getConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate( + "CREATE TABLE " + + TEST_DB_NAME + + "." + + name + + " (label varchar DEFAULT 'value', labels varchar[], " + + "bounded varchar(12) DEFAULT 'bound', bounded_array varchar(12)[], " + + "fixed char(12), fixed_array char(12)[], " + + "bounded_255 varchar(255), bounded_255_array varchar(255)[])"); + try { + JdbcTable table = operations.load(TEST_DB_NAME, name); + Type[] types = { + Types.StringType.get(), Types.ListType.nullable(Types.StringType.get()), + Types.VarCharType.of(12), Types.ListType.nullable(Types.VarCharType.of(12)), + Types.FixedCharType.of(12), Types.ListType.nullable(Types.FixedCharType.of(12)), + Types.VarCharType.of(255), Types.ListType.nullable(Types.VarCharType.of(255)) + }; + for (int i = 0; i < types.length; i++) { + Assertions.assertEquals(types[i], table.columns()[i].dataType()); + } + Assertions.assertEquals(Literals.stringLiteral("value"), table.columns()[0].defaultValue()); + Assertions.assertEquals( + Literals.varcharLiteral(12, "bound"), table.columns()[2].defaultValue()); + operations.create( + TEST_DB_NAME, + copy, + Arrays.stream(table.columns()).map(JdbcColumn.class::cast).toArray(JdbcColumn[]::new), + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + statement.executeUpdate( + "INSERT INTO " + + TEST_DB_NAME + + "." + + name + + " (label, labels) VALUES (repeat('x', 300), ARRAY[repeat('x', 300)])"); + statement.executeUpdate( + "INSERT INTO " + + TEST_DB_NAME + + "." + + copy + + " SELECT * FROM " + + TEST_DB_NAME + + "." + + name); + try (ResultSet result = + statement.executeQuery( + "SELECT char_length(label), char_length(labels[1]) FROM " + + TEST_DB_NAME + + "." + + copy)) { + Assertions.assertTrue(result.next()); + Assertions.assertEquals(300, result.getInt(1)); + Assertions.assertEquals(300, result.getInt(2)); + } + } finally { + operations.drop(TEST_DB_NAME, copy); + operations.drop(TEST_DB_NAME, name); + } + } finally { + DataSourceUtils.closeDataSource(dataSource); + } + } + + @Test + void testCharacterMetadataEscapesSchemaAndTablePatterns() throws SQLException { + String schema = RandomNameUtils.genRandomName("character_schema") + "\\_%"; + String table = "character\"\\_%"; + String qualified = + "\"" + schema.replace("\"", "\"\"") + "\".\"" + table.replace("\"", "\"\"") + "\""; + DATABASE_OPERATIONS.create(schema, null, Map.of()); + try (Connection connection = DATA_SOURCE.getConnection()) { + JdbcConnectorUtils.executeUpdate( + connection, + "CREATE TABLE " + + qualified + + " (v varchar, vs varchar[], b bpchar, bs bpchar[], bounded varchar(12))"); + try { + JdbcTable loaded = TABLE_OPERATIONS.load(schema, table); + Type[] expected = { + Types.StringType.get(), + Types.ListType.nullable(Types.StringType.get()), + Types.ExternalType.of("bpchar"), + Types.ListType.nullable(Types.ExternalType.of("bpchar")), + Types.VarCharType.of(12) + }; + Assertions.assertEquals(expected.length, loaded.columns().length); + for (int i = 0; i < expected.length; i++) { + Assertions.assertEquals(expected[i], loaded.columns()[i].dataType()); + } + } finally { + TABLE_OPERATIONS.drop(schema, table); + } + } finally { + DATABASE_OPERATIONS.delete(schema, false); + } + } + + @Test + void testNativeUnboundedVarcharLoadsAsString() throws SQLException { + String name = RandomNameUtils.genRandomName("unbounded_varchar"); + try (Connection connection = DATA_SOURCE.getConnection()) { + JdbcConnectorUtils.executeUpdate( + connection, + "CREATE TABLE " + TEST_DB_NAME + "." + name + " (label varchar, bounded varchar(255))"); + } + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, name); + Assertions.assertEquals(Types.StringType.get(), table.columns()[0].dataType()); + Assertions.assertEquals(Types.VarCharType.of(255), table.columns()[1].dataType()); + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, name); + } + } + @Test public void testOperationTable() { String tableName = RandomNameUtils.genRandomName("op_table_"); @@ -915,7 +1438,7 @@ public class TestPostgreSqlTableOperations extends TestPostgreSql { columns.add( JdbcColumn.builder() .withName("col_1") - .withType(Types.ShortType.get()) + .withType(Types.BooleanType.get()) .withComment("increment key") .withNullable(false) .withAutoIncrement(true) @@ -928,7 +1451,7 @@ public class TestPostgreSqlTableOperations extends TestPostgreSql { .withComment("set test key") .build()); - // Testing does not support auto increment column types + // Boolean columns remain unsupported; smallint is covered by the native round-trip tests. String randomName = RandomNameUtils.genRandomName("increment_table_"); JdbcColumn[] jdbcColumns = columns.toArray(new JdbcColumn[0]); IllegalArgumentException illegalArgumentException = @@ -948,6 +1471,21 @@ public class TestPostgreSqlTableOperations extends TestPostgreSql { Assertions.assertTrue( StringUtils.contains(illegalArgumentException.getMessage(), "Unsupported auto-increment")); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME, + tableName, + TableChange.addColumn( + new String[] {"invalid_identity"}, + Types.BooleanType.get(), + null, + TableChange.ColumnPosition.defaultPos(), + false, + true, + Column.DEFAULT_VALUE_NOT_SET))); + Assertions.assertEquals(2, TABLE_OPERATIONS.load(TEST_DB_NAME, tableName).columns().length); } @Test diff --git a/docs/jdbc-postgresql-catalog.md b/docs/jdbc-postgresql-catalog.md index f545d6740a..05c64e3edd 100644 --- a/docs/jdbc-postgresql-catalog.md +++ b/docs/jdbc-postgresql-catalog.md @@ -128,6 +128,23 @@ Type `numeric` instead. A `Numeric(p, s)` column is mapped to `Decimal(p, s)` an PostgreSQL array elements always accept NULL and cannot be declared otherwise, so an `Array` column is always mapped to a `List` whose elements are nullable. A `List` created with non-nullable elements is accepted and produces an ordinary array whose elements accept NULL. +Array element types retain their character length, numeric precision and scale, and supported temporal +precision, following the same conversion rules as scalar columns. Unbounded `varchar` maps to +`String`, including array elements. Unbounded `bpchar` maps to `ExternalType("bpchar")` because its +trailing-space-insensitive comparison semantics cannot be represented by `String` or a bounded +`FixedChar`. Consumers that read external types as strings must not assume native `bpchar` +comparison semantics. Bounded character types retain their declared lengths, independently of pgjdbc's +`unknownLength` setting. Loading a table with character columns or sequence-backed defaults performs +one additional read of PostgreSQL's column declarations to distinguish unbounded types from driver +display lengths and identify column-owned serial sequences. + +Native `smallserial`, `serial`, and `bigserial` columns map to `Short`, `Integer`, and `Long`, respectively, +and retain their auto-increment metadata. Standard non-null serial defaults that reference the column's own +sequence are represented by `autoIncrement`, without a separate default expression. Recreating such +columns creates an independent `GENERATED BY DEFAULT AS IDENTITY` sequence, as for loaded identity +columns. This preserves basic value generation, not the source sequence's current value, options, or +`GENERATED ALWAYS` mode. Shared sequence defaults, custom expressions, and serial columns altered +to allow NULL remain unparsed and are not supported for recreation. ::: ### Table Column Auto-Increment
