flyrain commented on code in PR #247:
URL: https://github.com/apache/polaris/pull/247#discussion_r1755743486
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java:
##########
@@ -280,8 +283,8 @@ public void runActionInTransaction(
LOGGER.debug("transaction committed");
}
} catch (Exception e) {
+ LOGGER.debug("Rolled back transaction due to an error", e);
Review Comment:
Nit: Rolled -> Rolling
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisSequenceManager.class);
+
+ private static AtomicBoolean sequenceCleaned = new AtomicBoolean(false);
+
+ /* Get the database platform associated with the `EntityManager` */
+ private static DatabasePlatform getDatabasePlatform(EntityManager session) {
+ EntityManagerImpl entityManagerImpl =
session.unwrap(EntityManagerImpl.class);
+ return entityManagerImpl.getDatabaseSession().getPlatform();
+ }
+
+ private static void removeSequence(EntityManager session) {
+ LOGGER.info("Renaming legacy sequence `POLARIS_SEQ` to
`POLARIS_SEQ_UNUSED`");
+ String renameSequenceQuery = "ALTER SEQUENCE POLARIS_SEQ RENAME TO
POLARIS_SEQ_UNUSED";
+ session.createNativeQuery(renameSequenceQuery).executeUpdate();
+ }
+
+ private static synchronized Optional<Long> getSequenceId(EntityManager
session) {
+ DatabasePlatform databasePlatform = getDatabasePlatform(session);
+ if (databasePlatform instanceof PostgreSQLPlatform) {
+ Optional<Long> result = Optional.empty();
+ if (!sequenceCleaned.get()) {
+ try {
+ LOGGER.info("Checking if the sequence POLARIS_SEQ exists");
+ String checkSequenceQuery =
+ "SELECT COUNT(*) FROM information_schema.sequences WHERE
sequence_name IN ('polaris_seq', 'POLARIS_SEQ')";
+ int sequenceExists =
+ ((Number)
session.createNativeQuery(checkSequenceQuery).getSingleResult()).intValue();
+
+ if (sequenceExists > 0) {
+ LOGGER.info("POLARIS_SEQ exists, calling NEXTVAL");
+ long queryResult =
+ (long) session.createNativeQuery("SELECT
NEXTVAL('POLARIS_SEQ')").getSingleResult();
+ result = Optional.of(queryResult);
+ } else {
+ LOGGER.info("POLARIS_SEQ does not exist, skipping NEXTVAL");
+ }
+ } catch (Exception e) {
+ LOGGER.info(
+ "Encountered an exception when checking sequence or calling
`NEXTVAL('POLARIS_SEQ')`",
+ e);
+ }
+ if (result.isPresent()) {
+ removeSequence(session);
+ }
+ sequenceCleaned.set(true);
+ }
+ return result;
+ } else {
+ LOGGER.info("Skipping POLARIS_SEQ / NEXTVAL check for platform " +
databasePlatform);
Review Comment:
Minor: I feel this log message isn't necessary, as the user using a database
other than Postgres doesn't have to know this check, or any Postgres related
checks.
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisSequenceManager.class);
+
+ private static AtomicBoolean sequenceCleaned = new AtomicBoolean(false);
+
+ /* Get the database platform associated with the `EntityManager` */
+ private static DatabasePlatform getDatabasePlatform(EntityManager session) {
+ EntityManagerImpl entityManagerImpl =
session.unwrap(EntityManagerImpl.class);
+ return entityManagerImpl.getDatabaseSession().getPlatform();
+ }
+
+ private static void removeSequence(EntityManager session) {
+ LOGGER.info("Renaming legacy sequence `POLARIS_SEQ` to
`POLARIS_SEQ_UNUSED`");
+ String renameSequenceQuery = "ALTER SEQUENCE POLARIS_SEQ RENAME TO
POLARIS_SEQ_UNUSED";
+ session.createNativeQuery(renameSequenceQuery).executeUpdate();
+ }
+
+ private static synchronized Optional<Long> getSequenceId(EntityManager
session) {
+ DatabasePlatform databasePlatform = getDatabasePlatform(session);
+ if (databasePlatform instanceof PostgreSQLPlatform) {
+ Optional<Long> result = Optional.empty();
+ if (!sequenceCleaned.get()) {
+ try {
+ LOGGER.info("Checking if the sequence POLARIS_SEQ exists");
+ String checkSequenceQuery =
+ "SELECT COUNT(*) FROM information_schema.sequences WHERE
sequence_name IN ('polaris_seq', 'POLARIS_SEQ')";
+ int sequenceExists =
+ ((Number)
session.createNativeQuery(checkSequenceQuery).getSingleResult()).intValue();
+
+ if (sequenceExists > 0) {
+ LOGGER.info("POLARIS_SEQ exists, calling NEXTVAL");
+ long queryResult =
+ (long) session.createNativeQuery("SELECT
NEXTVAL('POLARIS_SEQ')").getSingleResult();
+ result = Optional.of(queryResult);
+ } else {
+ LOGGER.info("POLARIS_SEQ does not exist, skipping NEXTVAL");
+ }
+ } catch (Exception e) {
+ LOGGER.info(
+ "Encountered an exception when checking sequence or calling
`NEXTVAL('POLARIS_SEQ')`",
+ e);
+ }
+ if (result.isPresent()) {
+ removeSequence(session);
+ }
+ sequenceCleaned.set(true);
+ }
+ return result;
+ } else {
+ LOGGER.info("Skipping POLARIS_SEQ / NEXTVAL check for platform " +
databasePlatform);
+ return Optional.empty();
+ }
+ }
+
+ /**
+ * Prepare the `PolarisSequenceManager` to generate IDs. This may run a
failing query, so it
+ * should be called for the first time outside the context of a transaction.
+ */
+ public static void initialize(EntityManager session) {
+ // Trigger cleanup of the POLARIS_SEQ if it is present
+ getSequenceId(session);
+ }
+
+ /**
+ * Generates a new ID from `POLARIS_SEQUENCE` or `POLARIS_SEQ` depending on
availability. If
+ * `POLARIS_SEQ` exists, it will be renamed to `POLARIS_SEQ_UNUSED`.
+ */
+ public static Long getNewId(EntityManager session) {
Review Comment:
renaming suggestion: `getNewId()` -> `nextSequence()`? `Id` is OK to use,
but here we may use `sequence` for consistency.
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java:
##########
@@ -280,8 +283,8 @@ public void runActionInTransaction(
LOGGER.debug("transaction committed");
}
} catch (Exception e) {
+ LOGGER.debug("Rolled back transaction due to an error", e);
tr.rollback();
Review Comment:
Should we retry in certain case(e.g., database isn't available temporily) of
rollback failure? It's not a blocker for this PR though. We can fix it in a
follow-up PR if needed.
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
Review Comment:
Minor: it should be fine to be an package level class instead of `public`.
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java:
##########
@@ -62,14 +66,22 @@ public PolarisEclipseLinkStore(@NotNull PolarisDiagnostics
diagnostics) {
this.diagnosticServices = diagnostics;
}
+ /** Initialize the store. This should be called before other methods. */
+ public void initialize(EntityManager session) {
+ PolarisSequenceManager.initialize(session);
+ initialized.set(true);
+ }
Review Comment:
Is this needed as one-time cleanup logic has been handled by
`PolarisSequenceManager` already?
```
if (!sequenceCleaned.get()) {
sequenceCleaned.set(true);
}
```
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
Review Comment:
Can we have a private constructor to prevent any object of this class?
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisSequenceManager.class);
+
+ private static AtomicBoolean sequenceCleaned = new AtomicBoolean(false);
+
+ /* Get the database platform associated with the `EntityManager` */
+ private static DatabasePlatform getDatabasePlatform(EntityManager session) {
+ EntityManagerImpl entityManagerImpl =
session.unwrap(EntityManagerImpl.class);
+ return entityManagerImpl.getDatabaseSession().getPlatform();
+ }
+
+ private static void removeSequence(EntityManager session) {
+ LOGGER.info("Renaming legacy sequence `POLARIS_SEQ` to
`POLARIS_SEQ_UNUSED`");
+ String renameSequenceQuery = "ALTER SEQUENCE POLARIS_SEQ RENAME TO
POLARIS_SEQ_UNUSED";
+ session.createNativeQuery(renameSequenceQuery).executeUpdate();
+ }
+
+ private static synchronized Optional<Long> getSequenceId(EntityManager
session) {
+ DatabasePlatform databasePlatform = getDatabasePlatform(session);
+ if (databasePlatform instanceof PostgreSQLPlatform) {
+ Optional<Long> result = Optional.empty();
+ if (!sequenceCleaned.get()) {
+ try {
+ LOGGER.info("Checking if the sequence POLARIS_SEQ exists");
+ String checkSequenceQuery =
+ "SELECT COUNT(*) FROM information_schema.sequences WHERE
sequence_name IN ('polaris_seq', 'POLARIS_SEQ')";
+ int sequenceExists =
+ ((Number)
session.createNativeQuery(checkSequenceQuery).getSingleResult()).intValue();
+
+ if (sequenceExists > 0) {
+ LOGGER.info("POLARIS_SEQ exists, calling NEXTVAL");
+ long queryResult =
+ (long) session.createNativeQuery("SELECT
NEXTVAL('POLARIS_SEQ')").getSingleResult();
+ result = Optional.of(queryResult);
+ } else {
+ LOGGER.info("POLARIS_SEQ does not exist, skipping NEXTVAL");
+ }
+ } catch (Exception e) {
+ LOGGER.info(
+ "Encountered an exception when checking sequence or calling
`NEXTVAL('POLARIS_SEQ')`",
+ e);
+ }
+ if (result.isPresent()) {
+ removeSequence(session);
+ }
+ sequenceCleaned.set(true);
+ }
+ return result;
+ } else {
+ LOGGER.info("Skipping POLARIS_SEQ / NEXTVAL check for platform " +
databasePlatform);
+ return Optional.empty();
+ }
+ }
+
+ /**
+ * Prepare the `PolarisSequenceManager` to generate IDs. This may run a
failing query, so it
+ * should be called for the first time outside the context of a transaction.
+ */
+ public static void initialize(EntityManager session) {
+ // Trigger cleanup of the POLARIS_SEQ if it is present
+ getSequenceId(session);
+ }
+
+ /**
+ * Generates a new ID from `POLARIS_SEQUENCE` or `POLARIS_SEQ` depending on
availability. If
+ * `POLARIS_SEQ` exists, it will be renamed to `POLARIS_SEQ_UNUSED`.
+ */
+ public static Long getNewId(EntityManager session) {
+ ModelSequenceId modelSequenceId = new ModelSequenceId();
+
+ // If a legacy sequence ID is present, use that as an override:
+ getSequenceId(session).ifPresent(modelSequenceId::setId);
+
Review Comment:
I feel it is easier to understand if we do this
```
ModelSequenceId modelSequenceId = new ModelSequenceId();
DatabasePlatform databasePlatform = getDatabasePlatform(session);
if (databasePlatform instanceof PostgreSQLPlatform) {
getSequenceFromPostgres(session).ifPresent(modelSequenceId::setId);
}
// Persist the new ID:
session.persist(modelSequenceId);
session.flush();
```
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisSequenceManager.class);
+
+ private static AtomicBoolean sequenceCleaned = new AtomicBoolean(false);
+
+ /* Get the database platform associated with the `EntityManager` */
+ private static DatabasePlatform getDatabasePlatform(EntityManager session) {
+ EntityManagerImpl entityManagerImpl =
session.unwrap(EntityManagerImpl.class);
+ return entityManagerImpl.getDatabaseSession().getPlatform();
+ }
+
+ private static void removeSequence(EntityManager session) {
+ LOGGER.info("Renaming legacy sequence `POLARIS_SEQ` to
`POLARIS_SEQ_UNUSED`");
+ String renameSequenceQuery = "ALTER SEQUENCE POLARIS_SEQ RENAME TO
POLARIS_SEQ_UNUSED";
+ session.createNativeQuery(renameSequenceQuery).executeUpdate();
+ }
+
+ private static synchronized Optional<Long> getSequenceId(EntityManager
session) {
+ DatabasePlatform databasePlatform = getDatabasePlatform(session);
+ if (databasePlatform instanceof PostgreSQLPlatform) {
+ Optional<Long> result = Optional.empty();
+ if (!sequenceCleaned.get()) {
+ try {
+ LOGGER.info("Checking if the sequence POLARIS_SEQ exists");
+ String checkSequenceQuery =
+ "SELECT COUNT(*) FROM information_schema.sequences WHERE
sequence_name IN ('polaris_seq', 'POLARIS_SEQ')";
+ int sequenceExists =
+ ((Number)
session.createNativeQuery(checkSequenceQuery).getSingleResult()).intValue();
+
+ if (sequenceExists > 0) {
+ LOGGER.info("POLARIS_SEQ exists, calling NEXTVAL");
+ long queryResult =
+ (long) session.createNativeQuery("SELECT
NEXTVAL('POLARIS_SEQ')").getSingleResult();
+ result = Optional.of(queryResult);
+ } else {
+ LOGGER.info("POLARIS_SEQ does not exist, skipping NEXTVAL");
+ }
+ } catch (Exception e) {
+ LOGGER.info(
+ "Encountered an exception when checking sequence or calling
`NEXTVAL('POLARIS_SEQ')`",
+ e);
+ }
+ if (result.isPresent()) {
+ removeSequence(session);
+ }
+ sequenceCleaned.set(true);
+ }
+ return result;
+ } else {
+ LOGGER.info("Skipping POLARIS_SEQ / NEXTVAL check for platform " +
databasePlatform);
+ return Optional.empty();
+ }
+ }
Review Comment:
We may extract a new method for Postgres like this, so that we confine the
database specific code into to a separated method.
```
private static synchronized Optional<Long> getSequenceId(EntityManager
session) {
DatabasePlatform databasePlatform = getDatabasePlatform(session);
if (databasePlatform instanceof PostgreSQLPlatform) {
return getSequenceFromPostgres(session);
} else {
return Optional.empty();
}
}
```
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisSequenceManager.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.extension.persistence.impl.eclipselink;
+
+import jakarta.persistence.*;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.polaris.core.persistence.models.ModelSequenceId;
+import org.eclipse.persistence.internal.jpa.EntityManagerImpl;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
+import org.eclipse.persistence.platform.database.PostgreSQLPlatform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Used to generate sequence IDs for Polaris entities. If the legacy
`POLARIS_SEQ` generator is
+ * available it will be used then cleaned up. In all other cases the
`POLARIS_SEQUENCE` table is
+ * used directly.
+ */
+public class PolarisSequenceManager {
Review Comment:
For a class with all static methods, would the name `PolarisSequenceUtil`
more suitable? Usually a manager class is considered to be allowed to have
multiple instances.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
