Copilot commented on code in PR #4281:
URL: https://github.com/apache/polaris/pull/4281#discussion_r3301173666
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -92,8 +94,19 @@ protected JdbcMetaStoreManagerFactory() {}
@Produces
@ApplicationScoped
static DatasourceOperations produceDatasourceOperations(
- Instance<DataSource> dataSource, RelationalJdbcConfiguration
relationalJdbcConfiguration) {
- return new DatasourceOperations(dataSource.get(),
relationalJdbcConfiguration);
+ Instance<DataSource> defaultDataSource,
+ @Any Instance<DataSource> dataSources,
+ RelationalJdbcConfiguration relationalJdbcConfiguration) {
+ // Use a named DataSource if one is declared for the configured
database-type;
+ // otherwise fall back to the default (shared by all PostgreSQL-driver
backends).
+ DataSource ds =
+ relationalJdbcConfiguration
+ .databaseType()
+ .map(type -> dataSources.select(new DataSourceLiteral(type)))
+ .filter(Instance::isResolvable)
+ .map(Instance::get)
+ .orElseGet(defaultDataSource::get);
Review Comment:
When `polaris.persistence.relational.jdbc.database-type` is explicitly set
to `mysql`, falling back to the default datasource if the named `mysql`
datasource isn’t resolvable can yield a confusing misconfiguration path (e.g.,
selecting the Postgres default, then failing later on type
inference/validation). Consider failing fast when a database type is explicitly
configured but the corresponding named datasource is not resolvable (at least
for `mysql`, where a distinct driver/datasource is required), and emit a
targeted error explaining how to enable the named datasource (`jdbc` build-time
+ `active=true` + URL/credentials).
--
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]
