Skip to content

Fix relational JDBC location-overlap index to match the optimized sibling-check query - #5301

Merged
adutra merged 3 commits into
apache:mainfrom
iprithv:fix/jdbc-location-overlap-index-catalog-id
Aug 24, 2026
Merged

Fix relational JDBC location-overlap index to match the optimized sibling-check query#5301
adutra merged 3 commits into
apache:mainfrom
iprithv:fix/jdbc-location-overlap-index-catalog-id

Conversation

@iprithv

@iprithv iprithv commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

The optimized sibling check (OPTIMIZED_SIBLING_CHECK) reads location_without_scheme through QueryGenerator.generateOverlapQuery, whose predicate is WHERE realm_id = ? AND catalog_id = ? AND (...).

On Postgres and CockroachDB, idx_locations currently leads with parent_id instead of catalog_id:

-- before (postgres/cockroachdb)
ON entities USING btree (realm_id, parent_id, location_without_scheme)

Because of that, the query can only use the realm_id prefix and ends up doing a realm/catalog-wide scan instead of the intended indexed lookup. H2 already uses (realm_id, catalog_id, location_without_scheme), which is why the default H2-based test matrix didn't catch this.

This changes the latest (v5) Postgres and CockroachDB scripts to use (realm_id, catalog_id, location_without_scheme) and adds SchemaLocationIndexParityTest to make sure the idx_locations column list stays consistent across backends.

Older scripts (v2-v4) are intentionally left unchanged. Fresh installs always start at the latest schema version, so those scripts are frozen. Existing realms on older versions are covered by the manual DDL in CHANGELOG.md.

The optimized sibling check is meant to replace an O(siblings) listing with an indexed lookup. With the current index on Postgres and CockroachDB, table/namespace creation can instead scale with the total catalog size.

Polaris doesn't have automated schema migrations, so existing Postgres/CockroachDB deployments will need to recreate the index manually:

DROP INDEX polaris_schema.idx_locations;
CREATE INDEX idx_locations ON polaris_schema.entities USING btree (realm_id, catalog_id, location_without_scheme)
  WHERE location_without_scheme IS NOT NULL;

H2 is unaffected.

@github-project-automation github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Aug 16, 2026
@iprithv
iprithv force-pushed the fix/jdbc-location-overlap-index-catalog-id branch from ee5fa47 to 135ed26 Compare August 16, 2026 18:12

@vigneshio vigneshio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM..

Comment thread persistence/relational-jdbc/src/main/resources/cockroachdb/schema-v5.sql Outdated
…ling-check query

The optimized sibling check reads location_without_scheme via
QueryGenerator.generateOverlapQuery, which filters `realm_id` and
`catalog_id`. On Postgres and CockroachDB the idx_locations index led
with `parent_id` instead of `catalog_id`, so the predicate could only
probe the realm_id prefix and every CREATE TABLE / CREATE NAMESPACE with
OPTIMIZED_SIBLING_CHECK enabled fell back to a scan instead of the
intended indexed lookup. H2 already used the correct columns.

Schema v5 has already shipped in 1.7.0, so rather than editing a released
script this adds schema v6 for all backends with the corrected index and
bumps the latest schema version. Fresh installs bootstrap at v6 and get
the right index automatically. Existing deployments need a one-time
manual index recreation, documented in the changelog upgrade notes,
because Polaris has no automated schema migrations.
@iprithv
iprithv force-pushed the fix/jdbc-location-overlap-index-catalog-id branch from 135ed26 to 3a082e4 Compare August 18, 2026 19:19
@zhang-arvin

Copy link
Copy Markdown

I would like to work on this issue.

@vigneshio

Copy link
Copy Markdown
Contributor

I would like to work on this issue.

Thanks @zhang-arvin - This is already being handled here. Please close #5331 so we keep the discussion in one place.

@iprithv
iprithv requested review from adutra and vigneshio August 19, 2026 13:17
adutra
adutra previously approved these changes Aug 19, 2026
--

-- Changes from v5:
-- * Schema version bumped to keep H2 in lockstep with Postgres/CockroachDB v6, which correct the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting issue. I'm not sure we need to keep the scripts in lockstep.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's actually keep them in sync. This will make the migration to single DDL scripts easier imho. See #5349.

@github-project-automation github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Aug 19, 2026
@adutra

adutra commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

@iprithv could you please fix the merge conflicts? 🙏

@iprithv

iprithv commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

@adutra sure, done. thanks!

@adutra
adutra enabled auto-merge (squash) August 24, 2026 10:00
@adutra
adutra merged commit 75f97a7 into apache:main Aug 24, 2026
43 of 45 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to merge to Done in Basic Kanban Board Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants