Copilot commented on code in PR #12930:
URL: https://github.com/apache/gravitino/pull/12930#discussion_r3933453884
##########
flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/jdbc/TestPostgresqlPropertiesConverter.java:
##########
@@ -19,13 +19,109 @@
package org.apache.gravitino.flink.connector.jdbc;
+import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
import java.util.Map;
+import org.apache.flink.table.catalog.ObjectPath;
import
org.apache.gravitino.flink.connector.jdbc.postgresql.PostgresqlPropertiesConverter;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
public class TestPostgresqlPropertiesConverter extends
AbstractJdbcPropertiesConverterTestSuite {
+ private static final String FLINK_BYPASS_DEFAULT_DATABASE =
"flink.bypass.default-database";
+
@Override
protected JdbcPropertiesConverter getConverter(Map<String, String>
catalogOptions) {
return PostgresqlPropertiesConverter.INSTANCE;
}
+
+ @Test
+ public void testToFlinkTableProperties() {
+ String jdbcDatabase = "gravitino";
+ String schema = "public";
+ String tableName = "table_meta";
+ // No 'flink.bypass.default-database' is set, so the connection database
must fall back to
+ // the catalog's jdbc-database.
+ Map<String, String> catalogPropertiesWithDatabase = new
HashMap<>(catalogProperties);
+ catalogPropertiesWithDatabase.remove(FLINK_BYPASS_DEFAULT_DATABASE);
+ catalogPropertiesWithDatabase.put(
+ JdbcPropertiesConstants.GRAVITINO_JDBC_DATABASE, jdbcDatabase);
+
+ // Mirrors the production call in BaseCatalog#toFlinkTable: the first
argument is the Flink
+ // catalog properties (as produced by toFlinkCatalogProperties), the
second is the Gravitino
+ // table's own properties, which do not carry catalog-level properties
like jdbc-database.
+ Map<String, String> flinkCatalogProperties =
+ getConverter(catalogPropertiesWithDatabase)
+ .toFlinkCatalogProperties(catalogPropertiesWithDatabase);
+ Map<String, String> tableProperties =
+ getConverter(catalogPropertiesWithDatabase)
+ .toFlinkTableProperties(
+ flinkCatalogProperties, ImmutableMap.of(), new
ObjectPath(schema, tableName));
+
+ // The connection URL must target the PostgreSQL database (jdbc-database),
not the schema.
+ Assertions.assertEquals(
+ flinkUrl + jdbcDatabase,
+
tableProperties.get(JdbcPropertiesConstants.FLINK_JDBC_TABLE_DATABASE_URL));
+ // The schema must be carried via the schema-qualified table name instead.
Review Comment:
These assertions (and the derived table scan URL) use `flinkUrl` from the
shared test suite, which is a MySQL URL (`jdbc:mysql://...`). For a
PostgreSQL-specific converter regression test, the input `jdbc-url` and
expected base URL should be PostgreSQL (e.g., `gravitinoUrlWithDomain` /
`flinkUrlWithDomain`), otherwise the test can pass even if PostgreSQL URL
handling regresses.
This issue also appears on line 81 of the same file.
--
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]
