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-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
