dimas-b commented on code in PR #3523:
URL: https://github.com/apache/polaris/pull/3523#discussion_r2771743474
##########
persistence/relational-jdbc/src/main/resources/postgres/schema-v4.sql:
##########
@@ -28,7 +28,7 @@ CREATE TABLE IF NOT EXISTS version (
version_value INTEGER NOT NULL
);
INSERT INTO version (version_key, version_value)
-VALUES ('version', 3)
+VALUES ('version', 4)
Review Comment:
Should we perhaps track metrics schema version separately?
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -172,6 +172,13 @@ public synchronized Map<String, PrincipalSecretsResult>
bootstrapRealms(
datasourceOperations
.getDatabaseType()
.openInitScriptResource(effectiveSchemaVersion));
+
+ // Run the metrics schema script if requested
+ if (JdbcBootstrapUtils.shouldIncludeMetrics(bootstrapOptions)) {
+ LOGGER.info("Including metrics schema for realm: {}", realm);
+ datasourceOperations.executeScript(
+
datasourceOperations.getDatabaseType().openMetricsSchemaResource(1));
Review Comment:
This is pretty close to the ideal from my POV 😅 Would it be too much trouble
to apply the metrics DDL in a metrics persistence class (separate from the JDBC
MetaStore, which deals with Entities)?
This may create a chicken-and-egg problem with respect to #3385 because we
need the schema to run persistence, and we need persistence classes to create
the schema 😅
I think we may want to merge this "as is" and later move DDL to Metrics
Persistence classes. `BaseMetaStoreCommand` will have to have a new bean
injected to deal with metrics DDL.
WDYT?
--
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]
