Remove unused IdempotencyStore and idempotency_records table - #5086
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the durable-replay, key-based idempotency implementation (store interface + JDBC backend + schema table) that is described as unused and superseded by entity-property idempotency in the wider Polaris codebase.
Changes:
- Removed
IdempotencyStore/IdempotencyRecord/IdempotencyPersistenceExceptionfrompolaris-core. - Removed the relational-JDBC idempotency store implementation and its Postgres integration test.
- Removed the
idempotency_recordstable/index from the v4 SQL schemas and deleted the now-unusedQueryGeneratorextended overloads and related tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| polaris-core/src/main/java/org/apache/polaris/core/persistence/IdempotencyStore.java | Removed the core idempotency persistence abstraction. |
| polaris-core/src/main/java/org/apache/polaris/core/persistence/IdempotencyPersistenceException.java | Removed store-specific persistence exception type. |
| polaris-core/src/main/java/org/apache/polaris/core/entity/IdempotencyRecord.java | Removed the core domain record for durable-replay idempotency. |
| persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/idempotency/RelationalJdbcIdempotencyStore.java | Removed the JDBC-backed IdempotencyStore implementation. |
| persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelIdempotencyRecord.java | Removed the JDBC model mapping for idempotency_records. |
| persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/idempotency/RelationalJdbcIdempotencyStorePostgresIT.java | Removed Postgres IT coverage for the deleted store. |
| persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java | Removed extended DELETE/UPDATE overloads that were only used by the deleted store. |
| persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java | Removed tests that exercised the deleted extended overloads. |
| persistence/relational-jdbc/src/main/resources/postgres/schema-v4.sql | Removed idempotency_records DDL from Postgres v4 schema script. |
| persistence/relational-jdbc/src/main/resources/h2/schema-v4.sql | Removed idempotency_records DDL from H2 v4 schema script. |
| persistence/relational-jdbc/src/main/resources/cockroachdb/schema-v4.sql | Removed idempotency_records DDL from CockroachDB v4 schema script. |
Comment on lines
209
to
230
| /** | ||
| * Builds an UPDATE query. | ||
| * | ||
| * @param allColumns Columns to update. | ||
| * @param tableName Target table. | ||
| * @param values New values (must match columns in order). | ||
| * @param whereClause Conditions for filtering rows to update. | ||
| * @return UPDATE query with parameter values. | ||
| */ | ||
| public static PreparedQuery generateUpdateQuery( | ||
| @NonNull List<String> allColumns, | ||
| @NonNull String tableName, | ||
| @NonNull List<Object> values, | ||
| @NonNull Map<String, Object> whereClause) { | ||
| List<Object> bindingParams = new ArrayList<>(values); | ||
| QueryFragment where = generateWhereClause(new HashSet<>(allColumns), whereClause, Map.of()); | ||
| String setClause = allColumns.stream().map(c -> c + " = ?").collect(Collectors.joining(", ")); | ||
| String sql = | ||
| "UPDATE " + getFullyQualifiedTableName(tableName) + " SET " + setClause + where.sql(); | ||
| bindingParams.addAll(where.parameters()); | ||
| return new PreparedQuery(sql, bindingParams); | ||
| } |
dimas-b
reviewed
Jul 17, 2026
| PRIMARY KEY (event_id) | ||
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS idempotency_records ( |
Contributor
There was a problem hiding this comment.
Schema v4 is part of the 1.6.0 release. I think its DDL should be immutable now.
Let's switch to v5 and not have idempotency tables there.
dimas-b
approved these changes
Jul 17, 2026
flyrain
approved these changes
Jul 17, 2026
Contributor
Author
6 tasks done
cobed95
added a commit
to cobed95/polaris
that referenced
this pull request
Jul 19, 2026
Upstream introduced schema v5 as the new latest version (apache#5086), which makes v4 a legacy version. Per the review agreement to leave legacy schema versions untouched, the v4 scripts are restored to their original form and the new v5 scripts drop the CREATE SCHEMA / SET search_path header instead, so the configurable schema name applies to the current schema version.
shyundev
pushed a commit
to shyundev/polaris
that referenced
this pull request
Aug 5, 2026
…hDB schemas The CockroachDB schema resources were forked from the PostgreSQL v4 schema in apache#3352. One week later, apache#3939 added three performance indexes to fix the realm-wide `grant_records` scan reported in apache#3685, but only to the H2 and PostgreSQL v4 schemas. apache#5086 then derived the v5 schemas from their v4 ancestors, carrying the omission forward. CockroachDB has therefore never declared them: idx_grants_realm_grantee ON grant_records (realm_id, grantee_id) idx_grants_realm_securable ON grant_records (realm_id, securable_id) idx_entities_catalog_id_id ON entities (catalog_id, id) This matters most for `idx_grants_realm_grantee`. The `grant_records` primary key leads with the securable columns, so `loadAllGrantRecordsOnGrantee` - issued on every resolver cache miss for a principal, principal role, or catalog role - has `realm_id` as its only usable key prefix and scans every grant record in the realm. On CockroachDB the primary key is also the physical storage and range split key, so that scan fans out across nodes. Diffing the two dialect files shows the change restores parity with the PostgreSQL schema the CockroachDB one was forked from: apart from the header and footer comments and the deliberate `INTEGER`/`INT` to `INT4` substitution, these three indexes were the only difference. `idx_locations` is byte-identical in both. No schema version bump: the indexes affect query planning only, every statement is `CREATE INDEX IF NOT EXISTS`, and no column, constraint, or query changes. This mirrors how apache#3939 itself landed - its commit message records that a proposed new schema version was moved into the existing file during review. `SchemaIndexParityTest` guards against this class of drift recurring. It compares only `(index name, table name)` pairs per schema version, across the dialects that ship that version. Column lists are deliberately not compared, because dialects legitimately differ - `idx_locations` is a partial index on PostgreSQL but a plain index on H2. Without the schema change the test fails on v4 parity, v5 parity, and the CockroachDB grant-record assertion, naming the three missing indexes. Because Polaris has no automated schema migrations, an existing CockroachDB database only picks these up when a new realm is bootstrapped, since bootstrapping is the only production path that runs the schema script. The CHANGELOG and the Relational JDBC metastore documentation carry the one-time `CREATE INDEX` statements for databases bootstrapped by an earlier release. Related to apache#3685
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the unused durable-replay idempotency implementation:
IdempotencyStore/IdempotencyRecord/IdempotencyPersistenceException(polaris-core)RelationalJdbcIdempotencyStore/ModelIdempotencyRecordand its Postgres ITidempotency_recordstable + index from the postgres/cockroachdb/h2schema-v4.sqlQueryGeneratorextended overloads (only used by the store) and their testsThis code is superseded by the shipped entity-property idempotency (
createTable/updateTable).Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)