yuqi1129 commented on code in PR #14521:
URL: https://github.com/apache/gravitino/pull/14521#discussion_r4464455441
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java:
##########
@@ -792,16 +802,123 @@ protected ResultSet getTable(Connection connection,
String schema, String tableN
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, "
Review Comment:
Thanks, added in
[7c46c6cd4e](https://github.com/apache/gravitino/commit/7c46c6cd4e839e85b522dfb1acb856b7f310bfe3).
The parameterized test now checks that an `ALWAYS` source rejects an
explicit id (SQLSTATE `428C9`), while both the recreated identity column and
the identity added through `ADD COLUMN` accept explicit values. The existing
assertions still verify subsequent generated values 1/2/3, including after
dropping the source table.
Both full 151-case module runs passed: table/schema-operation fixtures on
PostgreSQL 13 and 15 respectively, with the separate data-source fixture
remaining PostgreSQL 13. Temporarily changing CREATE to `GENERATED ALWAYS`
makes all six parameter cases fail at the new explicit-insert assertion.
This locks in the documented `BY DEFAULT` recreation contract within this
patch's basic-generation scope. Preserving `ALWAYS` mode would require separate
metadata/API work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
