Skip to content

Fix optimized sibling check for JDBC persistence#4580

Open
venkateshwaracholan wants to merge 4 commits into
apache:mainfrom
venkateshwaracholan:jdbc-optimized-sibling-check-fix
Open

Fix optimized sibling check for JDBC persistence#4580
venkateshwaracholan wants to merge 4 commits into
apache:mainfrom
venkateshwaracholan:jdbc-optimized-sibling-check-fix

Conversation

@venkateshwaracholan
Copy link
Copy Markdown
Contributor

@venkateshwaracholan venkateshwaracholan commented May 29, 2026

Fixes #3378

This fixes a ClassCastException in the JDBC optimized sibling location check when OPTIMIZED_SIBLING_CHECK=true.

JDBC persistence materializes queried entities as PolarisBaseEntity, but the optimized sibling check assumed results could be cast to LocationBasedEntity. This caused table creation to fail when optimized sibling checking was enabled.

The fix uses the persisted base location property stored on the entity instead of relying on a runtime type cast. Existing overlap detection behavior is preserved.

Added regression coverage for JDBC schema versions that support location_without_scheme, including both overlapping and non-overlapping sibling location scenarios.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

Copy link
Copy Markdown
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution, @venkateshwaracholan !

// JDBC materializes persisted rows as PolarisBaseEntity, so use the stored location
// property instead of casting to LocationBasedEntity.
String siblingBaseLocation =
result.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION);
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.

WDYT about using PolarisEntityUtils.asLocationBasedEntity()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dimas-b Switched to PolarisEntityUtils.asLocationBasedEntity() as suggested. Thanks!

dimas-b
dimas-b previously approved these changes Jun 2, 2026
Comment on lines +814 to +816
// JDBC materializes persisted rows as PolarisBaseEntity; resolve location via entity
// utils
// instead of casting to LocationBasedEntity.
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.

nit: better line wraps

void testHasOverlappingSiblingsUsesStoredBaseLocation() {
// The optimized check relies on the location_without_scheme column added in schema v2.
if (schemaVersion() < 2) {
return;
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.

nit: assumeThat(schemaVersion()).isLessThan(2)?

@github-project-automation github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jun 2, 2026
@dimas-b dimas-b requested a review from singhpk234 June 2, 2026 01:46
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.

Table Creation Fails with 500 when OPTIMIZED_SIBLING_CHECK=true with JDBC Persistence

2 participants