You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "byronellis (via GitHub)" <gi...@apache.org> on 2025/08/12 08:32:30 UTC

[PR] Spanner Persistence Backend for Polaris [polaris]

byronellis opened a new pull request, #2328:
URL: https://github.com/apache/polaris/pull/2328

   This is the first commit in a series that implements a Spanner Persistence backend for Polaris. It is largely modeled on the JDBC implementation though it takes advantage of Spanner-specific features such as nested tables.
   
   In this design the parent table is Realm with all other tables nested within that parent table. 
   
   In this initial PR we commit the basic classes needed to manage the Spanner connection lifecycle as well as the Realm model as Realm bootstrapping is part of Lifecycle management. This was cleaner than making it part of the persistence implementation and has the added benefit of allowing for a cleaner initial PR.
   
   <!--
       Possible security vulnerabilities: STOP here and contact security@apache.org instead!
   
       Please update the title of the PR with a meaningful message - do not leave it "empty" or "generated"
       Please update this summary field:
   
       The summary should cover these topics, if applicable:
       * the motivation for the change
       * a description of the status quo, for example the current behavior
       * the desired behavior
       * etc
   
       PR checklist:
       - Do a self-review of your code before opening a pull request
       - Make sure that there's good test coverage for the changes included in this PR
       - Run tests locally before pushing a PR (./gradlew check)
       - Code should have comments where applicable. Particularly hard-to-understand
         areas deserve good in-line documentation.
       - Include changes and enhancements to the documentation (in site/content/in-dev/unreleased)
       - For Work In Progress Pull Requests, please use the Draft PR feature.
   
       Make sure to add the information BELOW this comment.
       Everything in this comment will NOT be added to the PR description.
   -->
   


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270810987


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);

Review Comment:
   This is `AutoCloseable`, but should we manage a `CloseableGroup` just in case?



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import io.smallrye.config.ConfigMapping;
+import java.util.Optional;
+
+@ConfigMapping(prefix = "polaris.persistence.spanner")
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   nit: Since we're calling the config `spanner`, I wonder if we need the `GoogleCloud-` prefix everywhere? It's making things quite long. For similar cloud-specific types like `S3StorageLocation` or `GcpStorageConfigInfo` we are not quite as verbose



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {

Review Comment:
   Do we also need to check if the line ends with `;`? Later we split on that



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);

Review Comment:
   ```suggestion
           if (!s.startsWith("--") && s.length() > 0) {
             lines.add(s);
           }
   ```



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/model/Realm.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.persistence.relational.spanner.model;
+
+import com.google.cloud.spanner.Key;
+import com.google.cloud.spanner.Mutation;
+
+public final class Realm {
+
+  public static String TABLE_NAME = "Realms";

Review Comment:
   could be final



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =

Review Comment:
   nit: could be `final`



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/util/Modifier.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.persistence.relational.spanner.util;
+
+import java.util.Optional;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+public class Modifier<T> {
+  T wrapped;

Review Comment:
   could be final



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/DatabaseAdminClientSupplier.java:
##########


Review Comment:
   Should the package really be `org.apache.polaris.persistence.relational.spanner;`? Or `org.apache.polaris.persistence.spanner`? I thought `relational` was meant for the JDBC metastore or RDBMS more generally



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/util/SpannerUtil.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.persistence.relational.spanner.util;
+
+import com.google.cloud.ServiceOptions;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.InstanceId;
+import com.google.cloud.spanner.Key;
+import com.google.cloud.spanner.KeySet;
+import com.google.cloud.spanner.ResultSet;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerOptions;
+import com.google.cloud.spanner.Struct;
+import com.google.cloud.spanner.StructReader;
+import com.google.cloud.spanner.Type;
+import com.google.cloud.spanner.Value;
+import com.google.gson.Gson;
+import com.google.gson.JsonObject;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+import org.apache.polaris.persistence.relational.spanner.GoogleCloudSpannerConfiguration;
+
+public final class SpannerUtil {
+
+  public static final String INT64_TYPE =
+      Type.int64().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+  public static final String STRING_TYPE =
+      Type.string().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  public static final String JSON_TYPE =
+      Type.json().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  private static final Gson GSON = new Gson();
+
+  public static Value jsonValue(Map<String, String> properties) {
+    JsonObject jsonObject = new JsonObject();
+    if (properties != null) {
+      properties.forEach(jsonObject::addProperty);
+    }
+    return Value.json(jsonObject.toString());
+  }
+
+  public static Map<String, String> jsonMap(String properties) {
+    HashMap<String, String> map = new HashMap<>();
+    if (properties != null && !properties.isBlank()) {
+      JsonObject obj = GSON.fromJson(properties, JsonObject.class);
+      obj.asMap().forEach((k, v) -> map.put(k, v.toString()));
+    }
+    return map;
+  }
+
+  public static KeySet asKeySet(Iterable<Key> keys) {
+    KeySet.Builder builder = KeySet.newBuilder();
+    for (Key key : keys) {
+      builder = builder.addKey(key);
+    }
+    return builder.build();
+  }
+
+  public static Struct column(
+      String name, String spannerType, boolean nullable, boolean primaryKey) {
+    return Struct.newBuilder()
+        .set("Name")
+        .to(name)
+        .set("Type")
+        .to(spannerType)
+        .set("Nullable")
+        .to(nullable)
+        .set("PrimaryKey")
+        .to(primaryKey)
+        .build();
+  }
+
+  public static Optional<String> projectFromConfiguration(GoogleCloudSpannerConfiguration config) {
+    // If the quota project is set use that.
+    if (config.quotaProjectId().isPresent()) {
+      return config.quotaProjectId();
+    }
+
+    if (config.databaseId().isPresent()) {
+      String databaseId = config.databaseId().get();
+      if (databaseId.startsWith("project")) {
+        return Optional.ofNullable(DatabaseId.of(databaseId).getInstanceId().getProject());
+      }
+    }
+    return config.projectId();
+  }
+
+  public static DatabaseId databaseFromConfiguration(GoogleCloudSpannerConfiguration config) {
+    String databaseId = config.databaseId().orElseThrow();
+    if (databaseId.startsWith("project")) {
+      return DatabaseId.of(databaseId);
+    }
+    String instanceId = config.instanceId().orElseThrow();
+    if (instanceId.startsWith("project")) {
+      return DatabaseId.of(instanceId + "/" + databaseId);
+    } else {
+      return DatabaseId.of(InstanceId.of(config.projectId().orElseThrow(), instanceId), databaseId);
+    }
+  }
+
+  public static Spanner spannerFromConfiguration(GoogleCloudSpannerConfiguration config) {
+    return Modifier.of(SpannerOptions.newBuilder())
+        .ifPresent(projectFromConfiguration(config), ServiceOptions.Builder::setProjectId)
+        .ifPresent(config.emulatorHost(), SpannerOptions.Builder::setEmulatorHost)
+        .get()
+        .build()
+        .getService();
+  }
+
+  public static Iterator<StructReader> asIterator(final ResultSet resultSet) {
+    return new Iterator<StructReader>() {
+
+      @Override
+      public boolean hasNext() {
+        return resultSet.next();
+      }
+
+      @Override
+      public StructReader next() {
+        return resultSet;
+      }
+    };
+  }
+
+  public static Stream<StructReader> asStream(final ResultSet resultSet) {
+

Review Comment:
   nit: spurious whitespace



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271497948


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;

Review Comment:
   Mostly because until a bit earlier today there wasn't a constructor... but now that is not the case so we can just make these final.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271383238


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);

Review Comment:
   Closing an `@ApplicationScoped` bean is hardly necessary, I guess... unless it holds resources on the Spanner side.
   
   If we do want to close it, I'd suggest using `@Disposes`, e.g.: https://github.com/apache/polaris/blob/ef177b51cad07411d6b2b673fe3b5cc8a01c6d70/runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java#L407



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271388329


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {

Review Comment:
   just wondering: why not do the `init` work in the constructor?



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270478041


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {
+
+  public Optional<String> projectId();
+
+  public Optional<String> instanceId();
+
+  public Optional<String> databaseId();
+
+  public Optional<String> emulatorHost();

Review Comment:
   The intent here was to mirror the `SPANNER_EMULATOR` environment variable so "host" would optionally include a port (e.g. localhost:10203). We could separate it out, but seeing as this is really only applicable to the emulator which is in-memory only might be overkill.



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {
+
+  public Optional<String> projectId();
+
+  public Optional<String> instanceId();
+
+  public Optional<String> databaseId();
+
+  public Optional<String> emulatorHost();

Review Comment:
   You're welcome!



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271797958


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();

Review Comment:
   Done



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2328: Spanner Persistence Backend for Polaris
URL: https://github.com/apache/polaris/pull/2328


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270972164


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =

Review Comment:
   Fixed



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270956956


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/model/Realm.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.persistence.relational.spanner.model;
+
+import com.google.cloud.spanner.Key;
+import com.google.cloud.spanner.Mutation;
+
+public final class Realm {
+
+  public static String TABLE_NAME = "Realms";

Review Comment:
   Fixed



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2269323475


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {
+
+  public Optional<String> projectId();
+
+  public Optional<String> instanceId();
+
+  public Optional<String> databaseId();
+
+  public Optional<String> emulatorHost();

Review Comment:
   Guess this needs a port as well?



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Inject;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  @Inject GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public Consumer<SchemaOptions> getSchemaInitializer() {

Review Comment:
   Nit: I'd personally introduce a separate interface type that extends `Consumer<X>` (ran into issues with CDI + generics in the past - might no longer be an issue though).



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {
+
+  public Optional<String> projectId();
+
+  public Optional<String> instanceId();
+
+  public Optional<String> databaseId();
+
+  public Optional<String> emulatorHost();

Review Comment:
   BTW: Thanks for adding emulator support for testing right away!



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   Doesn't hurt to add the `@ConfigMapping` annotation (from smallrye-config) right away. Might be convenient for testing as well. For testing alone, maybe just add `@PolarisImmutable` (so you don't need any mocking).



##########
persistence/google-cloud-spanner/build.gradle.kts:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.
+ */
+
+plugins {
+  id("polaris-server")
+  alias(libs.plugins.jandex)
+}
+
+dependencies {
+  implementation(project(":polaris-core"))
+  implementation(libs.slf4j.api)
+  implementation(libs.guava)
+
+  implementation(platform(libs.google.cloud.libraries.bom))
+  implementation("com.google.cloud:google-cloud-spanner")
+
+  compileOnly(libs.jakarta.annotation.api)
+  compileOnly(libs.jakarta.enterprise.cdi.api)
+  compileOnly(libs.jakarta.inject.api)
+
+  implementation(libs.smallrye.common.annotation) // @Identifier

Review Comment:
   Nit:
   
   ```suggestion
     compileOnly(libs.smallrye.common.annotation) // @Identifier
   ```



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Inject;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  @Inject GoogleCloudSpannerConfiguration spannerConfiguration;

Review Comment:
   For testing purposes it might be more convenient to `@Inject` into a constructor (without CDI).
   
   An alternative might be using `weld-junit5` to not have to have a "full Quarkus setup" just for testing.



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   Do we need options like appProfileId, quotaProjectId here?
   Wonder whether Spanner has similar retry options as Bigtable?



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270451780


##########
persistence/google-cloud-spanner/build.gradle.kts:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.
+ */
+
+plugins {
+  id("polaris-server")
+  alias(libs.plugins.jandex)
+}
+
+dependencies {
+  implementation(project(":polaris-core"))
+  implementation(libs.slf4j.api)
+  implementation(libs.guava)
+
+  implementation(platform(libs.google.cloud.libraries.bom))
+  implementation("com.google.cloud:google-cloud-spanner")
+
+  compileOnly(libs.jakarta.annotation.api)
+  compileOnly(libs.jakarta.enterprise.cdi.api)
+  compileOnly(libs.jakarta.inject.api)
+
+  implementation(libs.smallrye.common.annotation) // @Identifier

Review Comment:
   Done



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270960234


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/util/SpannerUtil.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.persistence.relational.spanner.util;
+
+import com.google.cloud.ServiceOptions;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.InstanceId;
+import com.google.cloud.spanner.Key;
+import com.google.cloud.spanner.KeySet;
+import com.google.cloud.spanner.ResultSet;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerOptions;
+import com.google.cloud.spanner.Struct;
+import com.google.cloud.spanner.StructReader;
+import com.google.cloud.spanner.Type;
+import com.google.cloud.spanner.Value;
+import com.google.gson.Gson;
+import com.google.gson.JsonObject;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+import org.apache.polaris.persistence.relational.spanner.GoogleCloudSpannerConfiguration;
+
+public final class SpannerUtil {
+
+  public static final String INT64_TYPE =
+      Type.int64().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+  public static final String STRING_TYPE =
+      Type.string().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  public static final String JSON_TYPE =
+      Type.json().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  private static final Gson GSON = new Gson();
+
+  public static Value jsonValue(Map<String, String> properties) {
+    JsonObject jsonObject = new JsonObject();
+    if (properties != null) {
+      properties.forEach(jsonObject::addProperty);
+    }
+    return Value.json(jsonObject.toString());
+  }
+
+  public static Map<String, String> jsonMap(String properties) {
+    HashMap<String, String> map = new HashMap<>();
+    if (properties != null && !properties.isBlank()) {
+      JsonObject obj = GSON.fromJson(properties, JsonObject.class);
+      obj.asMap().forEach((k, v) -> map.put(k, v.toString()));
+    }
+    return map;
+  }
+
+  public static KeySet asKeySet(Iterable<Key> keys) {
+    KeySet.Builder builder = KeySet.newBuilder();
+    for (Key key : keys) {
+      builder = builder.addKey(key);
+    }
+    return builder.build();
+  }
+
+  public static Struct column(
+      String name, String spannerType, boolean nullable, boolean primaryKey) {
+    return Struct.newBuilder()
+        .set("Name")
+        .to(name)
+        .set("Type")
+        .to(spannerType)
+        .set("Nullable")
+        .to(nullable)
+        .set("PrimaryKey")
+        .to(primaryKey)
+        .build();
+  }
+
+  public static Optional<String> projectFromConfiguration(GoogleCloudSpannerConfiguration config) {
+    // If the quota project is set use that.
+    if (config.quotaProjectId().isPresent()) {
+      return config.quotaProjectId();
+    }
+
+    if (config.databaseId().isPresent()) {
+      String databaseId = config.databaseId().get();
+      if (databaseId.startsWith("project")) {
+        return Optional.ofNullable(DatabaseId.of(databaseId).getInstanceId().getProject());
+      }
+    }
+    return config.projectId();
+  }
+
+  public static DatabaseId databaseFromConfiguration(GoogleCloudSpannerConfiguration config) {
+    String databaseId = config.databaseId().orElseThrow();
+    if (databaseId.startsWith("project")) {
+      return DatabaseId.of(databaseId);
+    }
+    String instanceId = config.instanceId().orElseThrow();
+    if (instanceId.startsWith("project")) {
+      return DatabaseId.of(instanceId + "/" + databaseId);
+    } else {
+      return DatabaseId.of(InstanceId.of(config.projectId().orElseThrow(), instanceId), databaseId);
+    }
+  }
+
+  public static Spanner spannerFromConfiguration(GoogleCloudSpannerConfiguration config) {
+    return Modifier.of(SpannerOptions.newBuilder())
+        .ifPresent(projectFromConfiguration(config), ServiceOptions.Builder::setProjectId)
+        .ifPresent(config.emulatorHost(), SpannerOptions.Builder::setEmulatorHost)
+        .get()
+        .build()
+        .getService();
+  }
+
+  public static Iterator<StructReader> asIterator(final ResultSet resultSet) {
+    return new Iterator<StructReader>() {
+
+      @Override
+      public boolean hasNext() {
+        return resultSet.next();
+      }
+
+      @Override
+      public StructReader next() {
+        return resultSet;
+      }
+    };
+  }
+
+  public static Stream<StructReader> asStream(final ResultSet resultSet) {
+

Review Comment:
   Fixed



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270521848


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Inject;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  @Inject GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public Consumer<SchemaOptions> getSchemaInitializer() {

Review Comment:
   Converted this to an explicit SchemaInitializer consumer.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270985381


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import io.smallrye.config.ConfigMapping;
+import java.util.Optional;
+
+@ConfigMapping(prefix = "polaris.persistence.spanner")
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   Sorry where is it already implemented? Autocompletion / writing long type names is not an issue, but reading them and dealing with line-size constraints can be.
   
   It is an aesthetic decision, but `GoogleCloudSpannerDatabaseClientLifecycleManager.java` would set a new record for the longest `.java` filename in the project :)



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #2328:
URL: https://github.com/apache/polaris/pull/2328#issuecomment-3319048794

   Reviving this PR as it's a very interesting one. I will start the review.


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271339306


##########
gradle/libs.versions.toml:
##########
@@ -49,7 +49,7 @@ commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.18.0
 commons-text = { module = "org.apache.commons:commons-text", version = "1.14.0" }
 eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.7" }
 errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.41.0" }
-google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.55.0" }
+google-cloud-libraries-bom = { module = "com.google.cloud:libraries-bom", version = "26.64.0" }

Review Comment:
   Good point about LICENSE updates, however using a BOM does not necessary require LICENSE changes... only real dependencies need to be mentioned... IMHO, that can be done later (we have to double check dependencies for every release anyway).



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "XN137 (via GitHub)" <gi...@apache.org>.
XN137 commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2272480146


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/util/SpannerUtil.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.persistence.relational.spanner.util;
+
+import com.google.cloud.ServiceOptions;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.InstanceId;
+import com.google.cloud.spanner.Key;
+import com.google.cloud.spanner.KeySet;
+import com.google.cloud.spanner.ResultSet;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerOptions;
+import com.google.cloud.spanner.Struct;
+import com.google.cloud.spanner.StructReader;
+import com.google.cloud.spanner.Type;
+import com.google.cloud.spanner.Value;
+import com.google.gson.Gson;
+import com.google.gson.JsonObject;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+import org.apache.polaris.persistence.relational.spanner.GoogleCloudSpannerConfiguration;
+
+public final class SpannerUtil {
+
+  public static final String INT64_TYPE =
+      Type.int64().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+  public static final String STRING_TYPE =
+      Type.string().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  public static final String JSON_TYPE =
+      Type.json().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  private static final Gson GSON = new Gson();
+
+  public static Value jsonValue(Map<String, String> properties) {
+    JsonObject jsonObject = new JsonObject();
+    if (properties != null) {
+      properties.forEach(jsonObject::addProperty);
+    }
+    return Value.json(jsonObject.toString());
+  }
+
+  public static Map<String, String> jsonMap(String properties) {
+    HashMap<String, String> map = new HashMap<>();
+    if (properties != null && !properties.isBlank()) {
+      JsonObject obj = GSON.fromJson(properties, JsonObject.class);

Review Comment:
   wondering: is GSON a strict requirement here?
   the rest of the codebase uses jackson for stuff like this



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271779686


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);
+        throw new RuntimeException(
+            "Unable to update Spanner DDL. Please disable this option for this database configuration.",
+            e);
+      }
+    };
+  }
+
+  @Produces
+  public Consumer<RealmContext> getRealmInitializer() {
+    return (realmContext) -> {
+      try {
+        spanner
+            .getDatabaseClient(databaseId)
+            .write(ImmutableList.of(Realm.upsert(realmContext.getRealmIdentifier())));

Review Comment:
   Fair enough, moving this to the bootstrap code.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270453298


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   Done



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270484040


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   Added the SpannerOptions ProjectId as quotaProjectId below. Not sure what the "App Profile Id" is in this case? 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270507925


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import java.util.Optional;
+
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   As for retries, Spanner does offer the ability to customize retries though it's non-trivial as there's a lot of options and three separate gRPC connections (and setting a retry configuration globally is disallowed). If we can I would prefer to defer that to a later PR and get things up and running since that's probably sufficient for a lot of folks.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271852003


##########
gradle/libs.versions.toml:
##########
@@ -49,7 +49,7 @@ commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.18.0
 commons-text = { module = "org.apache.commons:commons-text", version = "1.14.0" }
 eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.7" }
 errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.41.0" }
-google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.55.0" }
+google-cloud-libraries-bom = { module = "com.google.cloud:libraries-bom", version = "26.64.0" }

Review Comment:
   I'm OK with a followup PR.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2278042476


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {

Review Comment:
   Added a check for lines only containing ';'



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2328:
URL: https://github.com/apache/polaris/pull/2328#issuecomment-3290231776

   This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270967285


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/DatabaseAdminClientSupplier.java:
##########


Review Comment:
   I don't have any preference (personally I think relational vs not is just implementation detail), though if you have the pick Spanner in 2025 really fits better in the relational model than the nosql model.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271361566


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;

Review Comment:
   having mutable `protected` fields in an `@ApplicationScoped` class looks a bit odd to me. What is the intent here?



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);
+        throw new RuntimeException(
+            "Unable to update Spanner DDL. Please disable this option for this database configuration.",
+            e);
+      }
+    };
+  }
+
+  @Produces
+  public Consumer<RealmContext> getRealmInitializer() {
+    return (realmContext) -> {
+      try {
+        spanner
+            .getDatabaseClient(databaseId)
+            .write(ImmutableList.of(Realm.upsert(realmContext.getRealmIdentifier())));

Review Comment:
   Also `RealmContext` CDI beans may come and go very frequently in runtime (once per request at least).



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);
+        throw new RuntimeException(
+            "Unable to update Spanner DDL. Please disable this option for this database configuration.",
+            e);
+      }
+    };
+  }
+
+  @Produces
+  public Consumer<RealmContext> getRealmInitializer() {
+    return (realmContext) -> {
+      try {
+        spanner
+            .getDatabaseClient(databaseId)
+            .write(ImmutableList.of(Realm.upsert(realmContext.getRealmIdentifier())));

Review Comment:
   I wonder if this should be done under the "bootstrap" call path as opposed to on observing new realm IDs in runtime. The difference would be delegating realm initialization to the "admin" user / admin tool. Cf. #2196



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271347778


##########
gradle/libs.versions.toml:
##########
@@ -49,7 +49,7 @@ commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.18.0
 commons-text = { module = "org.apache.commons:commons-text", version = "1.14.0" }
 eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.7" }
 errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.41.0" }
-google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.55.0" }
+google-cloud-libraries-bom = { module = "com.google.cloud:libraries-bom", version = "26.64.0" }

Review Comment:
   Just wondering: does `libraries-bom` get updated as frequently as any of its upstream artifacts are published?



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271467602


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);
+        throw new RuntimeException(
+            "Unable to update Spanner DDL. Please disable this option for this database configuration.",
+            e);
+      }
+    };
+  }
+
+  @Produces
+  public Consumer<RealmContext> getRealmInitializer() {
+    return (realmContext) -> {
+      try {
+        spanner
+            .getDatabaseClient(databaseId)
+            .write(ImmutableList.of(Realm.upsert(realmContext.getRealmIdentifier())));

Review Comment:
   Cf. https://lists.apache.org/thread/9nl0dt6fhqx4t4q55kyzf1v9r2vhl2gg



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271799125


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);

Review Comment:
   Removed



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2278042135


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/DatabaseAdminClientSupplier.java:
##########


Review Comment:
   eh, changed my mind and just started moving things now. A little more work building the follow on PRs but maybe clearer for folks



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2278043544


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);

Review Comment:
   I think I'm OK? I wanted to remove lines that are comments or just blank... Added a check for only containing ';' in case some monster does that... that said it's not like we're sending arbitrary SQL through this thing so we don't need to be super careful.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270902715


##########
gradle/libs.versions.toml:
##########
@@ -49,7 +49,7 @@ commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.18.0
 commons-text = { module = "org.apache.commons:commons-text", version = "1.14.0" }
 eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.7" }
 errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.41.0" }
-google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.55.0" }
+google-cloud-libraries-bom = { module = "com.google.cloud:libraries-bom", version = "26.64.0" }

Review Comment:
   We will need to change the License file for this change, somewhere like here https://github.com/apache/polaris/blob/d7f15a29fa1b6463087b5d275b8f9c82e1ef4b6b/runtime/server/distribution/LICENSE#L556-L556. cc @jbonofre 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271042104


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import io.smallrye.config.ConfigMapping;
+import java.util.Optional;
+
+@ConfigMapping(prefix = "polaris.persistence.spanner")
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   The full implementation is here: https://github.com/byronellis/polaris/tree/spanner-persistence just didn't want to drop all of that on y'all in one go.
   
   I'm just going to leave the name as-is for right now, if someone feels really strongly about it they can file a PR to change all the callsites later if that works for you. 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270517105


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Inject;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  @Inject GoogleCloudSpannerConfiguration spannerConfiguration;

Review Comment:
   Converted to a constructor since it seems Quarkus will just do the right thing there.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271383238


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);

Review Comment:
   Closing an `ApplicationScoped` bean is hardly necessary, I guess... unless it holds resources on the Spanner side.



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);

Review Comment:
   Closing an `@ApplicationScoped` bean is hardly necessary, I guess... unless it holds resources on the Spanner side.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271369235


##########
gradle/libs.versions.toml:
##########
@@ -49,7 +49,7 @@ commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.18.0
 commons-text = { module = "org.apache.commons:commons-text", version = "1.14.0" }
 eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.7" }
 errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.41.0" }
-google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.55.0" }
+google-cloud-libraries-bom = { module = "com.google.cloud:libraries-bom", version = "26.64.0" }

Review Comment:
   I believe most of the Cloud SDKs, libraries included, are on a two week cadence more or less. The advantage (coming from the Beam experience with the Cloud Java SDKs) of using the BOM is that it keeps the various support libraries synchronized across specific SDKS. What happens otherwise is you get version drift in shared components like Protobuf or gRPC core libraries which can be really hard to spot. 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270565412


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Inject;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  @Inject GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public Consumer<SchemaOptions> getSchemaInitializer() {

Review Comment:
   For completeness converted the rest of the consumers and suppliers as well.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271055635


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/DatabaseAdminClientSupplier.java:
##########


Review Comment:
   I think what mostly happened is that when I started this y'all had a relational package for eclipselink and jdbc but somewhere along the way you moved things up a level in the project structure... Probably the best way to resolve that would be to merge the PRs and then do a package name refactor since the refactoring tool could do it all in one go. Also makes it clear what's happening in the commit chain. 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2278041539


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/util/SpannerUtil.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.persistence.relational.spanner.util;
+
+import com.google.cloud.ServiceOptions;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.InstanceId;
+import com.google.cloud.spanner.Key;
+import com.google.cloud.spanner.KeySet;
+import com.google.cloud.spanner.ResultSet;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerOptions;
+import com.google.cloud.spanner.Struct;
+import com.google.cloud.spanner.StructReader;
+import com.google.cloud.spanner.Type;
+import com.google.cloud.spanner.Value;
+import com.google.gson.Gson;
+import com.google.gson.JsonObject;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+import org.apache.polaris.persistence.relational.spanner.GoogleCloudSpannerConfiguration;
+
+public final class SpannerUtil {
+
+  public static final String INT64_TYPE =
+      Type.int64().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+  public static final String STRING_TYPE =
+      Type.string().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  public static final String JSON_TYPE =
+      Type.json().getSpannerTypeName(Dialect.GOOGLE_STANDARD_SQL);
+
+  private static final Gson GSON = new Gson();
+
+  public static Value jsonValue(Map<String, String> properties) {
+    JsonObject jsonObject = new JsonObject();
+    if (properties != null) {
+      properties.forEach(jsonObject::addProperty);
+    }
+    return Value.json(jsonObject.toString());
+  }
+
+  public static Map<String, String> jsonMap(String properties) {
+    HashMap<String, String> map = new HashMap<>();
+    if (properties != null && !properties.isBlank()) {
+      JsonObject obj = GSON.fromJson(properties, JsonObject.class);

Review Comment:
   I don't think it's a strict requirement. It comes along for the ride with Spanner so I used it more or less out of habit. Jackson would be fine too I think? Let me try it



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2271522016


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);

Review Comment:
   I think the error message isn't necessary as we throw the runtime exception next line.



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();

Review Comment:
   Minor: reuse the client?
   ```suggestion
           client.updateDatabaseDdl(dbInfo, ddlStatements, null).get();
   ```



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);
+        throw new RuntimeException(
+            "Unable to update Spanner DDL. Please disable this option for this database configuration.",
+            e);
+      }
+    };
+  }
+
+  @Produces
+  public Consumer<RealmContext> getRealmInitializer() {
+    return (realmContext) -> {
+      try {
+        spanner
+            .getDatabaseClient(databaseId)
+            .write(ImmutableList.of(Realm.upsert(realmContext.getRealmIdentifier())));

Review Comment:
   Agreed with @dimas-b. The realm initialization doesn't happen very often. It only happens when we bootstrap a new realm, https://polaris.apache.org/in-dev/unreleased/admin-tool/#bootstrapping-realms-and-principal-credentials. Producing a bean here isn't necessary to me, as Polaris server will never use it for realm initialization. Here is the reference code path in JDBC impl.: https://github.com/polaris-catalog/polaris/blob/main/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java#L142-L142



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {

Review Comment:
   I think we could make this method static.



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {

Review Comment:
   Same here. The scheme options are only used while bootstrapping. Bootstrappping was done by the Admin tool. It doesn't seem necessary to me that we need a SchemaInitializer bean for any dynamic options inside Polaris server. A normal function with schema options should be good enough.



##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerDatabaseClientLifecycleManager.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import com.google.cloud.spanner.Database;
+import com.google.cloud.spanner.DatabaseAdminClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerException;
+import com.google.common.collect.ImmutableList;
+import jakarta.annotation.PostConstruct;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Produces;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.persistence.bootstrap.SchemaOptions;
+import org.apache.polaris.persistence.relational.spanner.model.Realm;
+import org.apache.polaris.persistence.relational.spanner.util.SpannerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+public class GoogleCloudSpannerDatabaseClientLifecycleManager {
+
+  private static Logger LOGGER =
+      LoggerFactory.getLogger(GoogleCloudSpannerDatabaseClientLifecycleManager.class);
+
+  protected final GoogleCloudSpannerConfiguration spannerConfiguration;
+
+  public GoogleCloudSpannerDatabaseClientLifecycleManager(
+      GoogleCloudSpannerConfiguration spannerConfiguration) {
+    this.spannerConfiguration = spannerConfiguration;
+  }
+
+  protected Spanner spanner;
+  protected DatabaseId databaseId;
+
+  @PostConstruct
+  protected void init() {
+    spanner = SpannerUtil.spannerFromConfiguration(spannerConfiguration);
+    databaseId = SpannerUtil.databaseFromConfiguration(spannerConfiguration);
+  }
+
+  protected List<String> getSpannerDatabaseDdl(SchemaOptions options) {
+    final InputStream schemaStream;
+    if (options.schemaFile() != null) {
+      try {
+        schemaStream = new FileInputStream(options.schemaFile());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Unable to load file " + options.schemaFile(), e);
+      }
+    } else {
+      if (options.schemaVersion() == null || options.schemaVersion() == 1) {
+        schemaStream =
+            getClass().getResourceAsStream("/org/apache/polaris/persistence/spanner/schema-v1.sql");
+      } else {
+        throw new IllegalArgumentException("Unknown schema version " + options.schemaVersion());
+      }
+    }
+    try (schemaStream) {
+      String schema = new String(schemaStream.readAllBytes(), Charset.forName("UTF-8"));
+      List<String> lines = new ArrayList<>();
+      for (String s : schema.split("\n")) {
+        s = s.trim();
+        if (s.startsWith("--") || s.length() == 0) {
+          continue;
+        }
+        lines.add(s);
+      }
+      return List.of(String.join(" ", lines).split(";"));
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to retrieve DDL statements", e);
+    }
+  }
+
+  @Produces
+  public SchemaInitializer getSchemaInitializer() {
+    return (options) -> {
+      List<String> ddlStatements = getSpannerDatabaseDdl(options);
+      LOGGER.info(
+          "Attempting to initialize Spanner database DDL with {} statements,",
+          ddlStatements.size());
+      DatabaseAdminClient client = spanner.getDatabaseAdminClient();
+      Database dbInfo =
+          client.newDatabaseBuilder(databaseId).setDialect(Dialect.GOOGLE_STANDARD_SQL).build();
+      try {
+        spanner.getDatabaseAdminClient().updateDatabaseDdl(dbInfo, ddlStatements, null).get();
+        LOGGER.info("Successfully applied DDL update.");
+      } catch (InterruptedException | ExecutionException e) {
+        LOGGER.error("Unable to update Spanner DDL.", e);
+        throw new RuntimeException(
+            "Unable to update Spanner DDL. Please disable this option for this database configuration.",
+            e);
+      }
+    };
+  }
+
+  @Produces
+  public Consumer<RealmContext> getRealmInitializer() {
+    return (realmContext) -> {
+      try {
+        spanner
+            .getDatabaseClient(databaseId)
+            .write(ImmutableList.of(Realm.upsert(realmContext.getRealmIdentifier())));
+      } catch (SpannerException e) {
+        LOGGER.error("Unable to initialize realm " + realmContext.getRealmIdentifier(), e);

Review Comment:
   Throw a runtime exception instead of logging an error? So that the stack trace will show the complete call chain.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270971313


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/GoogleCloudSpannerConfiguration.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.persistence.relational.spanner;
+
+import io.smallrye.config.ConfigMapping;
+import java.util.Optional;
+
+@ConfigMapping(prefix = "polaris.persistence.spanner")
+public interface GoogleCloudSpannerConfiguration {

Review Comment:
   No real preference since my editor autocompletes everything for me... It's already implemented as GoogleCloudSpanner* so it would be a fair amount of toil to rename for what amounts to an aesthetic decision though.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270958132


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/util/Modifier.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.persistence.relational.spanner.util;
+
+import java.util.Optional;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+public class Modifier<T> {
+  T wrapped;

Review Comment:
   Fixed



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #2328:
URL: https://github.com/apache/polaris/pull/2328#discussion_r2270977976


##########
persistence/google-cloud-spanner/src/main/java/org/apache/polaris/persistence/relational/spanner/DatabaseAdminClientSupplier.java:
##########


Review Comment:
   Yeah so this is what I'm saying, I think the package name is a bit misleading. This is really for `relational-jdbc`, it's not saying everything in this package uses a relational model (and implicitly everything outside of it does not)



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Spanner Persistence Backend for Polaris [polaris]

Posted by "byronellis (via GitHub)" <gi...@apache.org>.
byronellis commented on PR #2328:
URL: https://github.com/apache/polaris/pull/2328#issuecomment-3319598386

   Thanks JB,
   
   I think there might be one thing that people wanted me to change but Life
   has been pretty busy this month (probably also needs to be rebased since
   I'm sure some breaking change has happened). There's more to the
   implementation, but wanted to break things up so nobody had to review a 5k
   line PR :-)
   
   On Mon, Sep 22, 2025 at 6:29 AM JB Onofré ***@***.***> wrote:
   
   > *jbonofre* left a comment (apache/polaris#2328)
   > <https://github.com/apache/polaris/pull/2328#issuecomment-3319048794>
   >
   > Reviving this PR as it's a very interesting one. I will start the review.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/polaris/pull/2328#issuecomment-3319048794>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AAE7OMA3ND6HSXCTUQV7TFT3T72TZAVCNFSM6AAAAACDVXF7FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMJZGA2DQNZZGQ>
   > .
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   
   
   -- 
   Byron Ellis ***@***.***)
   "Oook" -- The Librarian
   


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org