flyrain commented on code in PR #3385:
URL: https://github.com/apache/polaris/pull/3385#discussion_r2855663427
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -167,18 +169,11 @@ public synchronized Map<String, PrincipalSecretsResult>
bootstrapRealms(
effectiveSchemaVersion,
realm);
try {
- // Run the set-up script to create the tables.
+ // Run the set-up script to create the tables (includes metrics
tables).
Review Comment:
Nit: do we need this comments?
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -105,6 +106,7 @@ private void initializeForRealm(
JdbcBasePersistenceImpl.loadSchemaVersion(
datasourceOperations,
realmConfig.getConfig(BehaviorChangeConfiguration.SCHEMA_VERSION_FALL_BACK_ON_DNE));
+
Review Comment:
nit: not needed
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -75,6 +75,7 @@ public class JdbcMetaStoreManagerFactory implements
MetaStoreManagerFactory {
@Inject PolarisDiagnostics diagnostics;
@Inject PolarisStorageIntegrationProvider storageIntegrationProvider;
@Inject Instance<DataSource> dataSource;
+
Review Comment:
nit: not needed
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -169,6 +169,63 @@ public static PreparedQuery generateInsertQuery(
return new PreparedQuery(sql, finalValues);
}
+ /**
+ * Generates an idempotent INSERT query that ignores conflicts on the
primary key.
+ *
+ * <p>This is useful for metrics and other write-once data where duplicate
inserts should be
+ * silently ignored. The conflict handling is database-specific:
+ *
+ * <ul>
+ * <li>PostgreSQL: Uses INSERT ... ON CONFLICT (columns) DO NOTHING
+ * <li>H2: Uses MERGE INTO ... KEY (columns) VALUES
+ * </ul>
+ *
+ * @param columns Columns to insert values into (should already include
realm_id if needed).
+ * @param tableName Target table name.
+ * @param values Values for each column (must match order of columns).
+ * @param databaseType The database type for dialect-specific syntax.
+ * @param conflictColumns The columns that form the conflict/key constraint.
+ * @return INSERT query with value bindings that ignores conflicts.
+ */
+ public static PreparedQuery generateIdempotentInsertQuery(
Review Comment:
why do we change the idempotent query in this PR?
##########
CHANGELOG.md:
##########
@@ -58,6 +58,7 @@ request adding CHANGELOG notes for breaking (!) changes and
possibly other secti
- Relaxed `client_id`, `client_secret` regex/pattern validation on reset
endpoint call
- Added support for S3-compatible storage that does not have KMS (use
`kmsUavailable: true` in catalog storage configuration)
- Added support for storage-scoped AWS credentials, allowing different AWS
credentials to be configured per named storage. Enable with the
`RESOLVE_CREDENTIALS_BY_STORAGE_NAME` feature flag (default: false). Storage
names can be set explicitly via the `storageName` field on storage
configuration, or inferred from the first allowed location's host.
+- Added support for persisting Iceberg metrics (ScanReport, CommitReport) to
the database. Metrics tables are included in the main schema and metrics are
automatically persisted when using JDBC persistence.
Review Comment:
I think this is not accurate, when the default metrics reporter is used,
there is no metrics persistence. We may clarify that users need to configure to
enable it.
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -58,6 +58,7 @@ polaris.persistence.type=relational-jdbc
## MongoDB version store specific configuration
#quarkus.mongodb.database=polaris
#quarkus.mongodb.metrics.enabled=true
+
Review Comment:
not needed
--
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]
