dimas-b commented on code in PR #2196:
URL: https://github.com/apache/polaris/pull/2196#discussion_r2238004882


##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -116,6 +116,28 @@ public void executeScript(InputStream scriptInputStream) 
throws SQLException {
     }
   }
 
+  public boolean checkSchemaExists(@Nonnull String schemaName) {
+    final String sql = "SELECT 1 FROM information_schema.schemata WHERE 
schema_name = ?";

Review Comment:
   Is `information_schema.schemata` standard? :thinking: Could it be preferable 
to use JDBC APIs (`Connection.getMetaData()`) for  this and let the driver 
figure out the right source for this data?



##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/SchemaInitializer.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.persistence;
+
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+
+public interface SchemaInitializer {
+
+  boolean setupSchema(SchemaOptions schemaOptions);
+
+  boolean isSchemaPresent(String schemaName);

Review Comment:
   `schemaName` looks like a concept leak in this interface. `SchemaOptions` 
does not define a "name"... How can the caller know what schema name to ask for?



##########
runtime/admin/src/main/java/org/apache/polaris/admintool/BaseCommand.java:
##########
@@ -29,8 +30,11 @@ public abstract class BaseCommand implements 
Callable<Integer> {
   public static final int EXIT_CODE_USAGE = 2;
   public static final int EXIT_CODE_BOOTSTRAP_ERROR = 3;
   public static final int EXIT_CODE_PURGE_ERROR = 4;
+  public static final int EXIT_CODE_SCHEMA_ERROR = 5;
 
   @Inject MetaStoreManagerFactory metaStoreManagerFactory;
 
+  @Inject SchemaInitializer schemaInitializer;

Review Comment:
   Is this common to all commands? I tend to think it's only for `bootstrap` 
and `setup` :thinking: 



##########
runtime/admin/src/main/java/org/apache/polaris/admintool/config/QuarkusProducers.java:
##########
@@ -43,6 +44,12 @@ public MetaStoreManagerFactory metaStoreManagerFactory(
     return 
metaStoreManagerFactories.select(Identifier.Literal.of(persistenceType)).get();
   }
 
+  @Produces
+  @ApplicationScoped
+  public SchemaInitializer schemaInitializer(MetaStoreManagerFactory 
metaStoreManagerFactory) {
+    return (SchemaInitializer) metaStoreManagerFactory;

Review Comment:
   Please use `.select()` as in `metaStoreManagerFactory()` for proper type 
handling.



-- 
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

Reply via email to