dimas-b commented on code in PR #1942:
URL: https://github.com/apache/polaris/pull/1942#discussion_r2167738233
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -108,18 +115,19 @@ private void initializeForRealm(
metaStoreManagerMap.put(realmContext.getRealmIdentifier(),
metaStoreManager);
}
- private DatasourceOperations getDatasourceOperations(boolean isBootstrap) {
+ private DatasourceOperations getDatasourceOperations(Optional<SchemaOptions>
schemaOptions) {
DatasourceOperations databaseOperations;
try {
databaseOperations = new DatasourceOperations(dataSource.get(),
relationalJdbcConfiguration);
} catch (SQLException sqlException) {
throw new RuntimeException(sqlException);
}
- if (isBootstrap) {
+ // If this is set, we are bootstrapping
+ if (schemaOptions.isPresent()) {
Review Comment:
Since we're touching this code, I'd like to avoid having bootstrap logic on
the main runtime call path (in server). Do you think we could refactor that to
execute DDL only in the admin tool?
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java:
##########
@@ -71,6 +78,23 @@ static class FileInputOptions {
description = "A file containing root principal credentials to
bootstrap.")
Path file;
}
+
+ static class SchemaInputOptions {
+ @CommandLine.Option(
+ names = {"-v", "--schema-version"},
+ paramLabel = "<schema version>",
+ description = "The version of the schema to load in [1, 2, LATEST].",
+ defaultValue = SchemaOptions.LATEST)
+ String schemaVersion;
+
+ @CommandLine.Option(
+ names = {"--schema-file"},
+ paramLabel = "<schema file>",
+ description =
+ "A schema file to bootstrap from. If unset, the bundled files
will be used.",
Review Comment:
"file" may be misleading here. The script is loaded from resources built
into the admin tool jar, IIRC.
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatabaseType.java:
##########
@@ -43,7 +45,21 @@ public static DatabaseType fromDisplayName(String
displayName) {
};
}
- public String getInitScriptResource() {
- return String.format("%s/schema-v1.sql", this.getDisplayName());
+ public String getInitScriptResource(@Nonnull SchemaOptions schemaOptions) {
+ final String schemaFile;
+ if (schemaOptions.schemaFile() != null) {
+ schemaFile = schemaOptions.schemaFile();
+ } else {
+ String schemaSuffix;
+ switch (schemaOptions.schemaVersion()) {
+ case SchemaOptions.LATEST -> schemaSuffix = "schema-v1.sql";
Review Comment:
suggestion: make `schemaVersion()` optional and default to latest.
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
