Skip to content

Conversation

@Fottas
Copy link

@Fottas Fottas commented Dec 2, 2025

Overview

This is the second PR for the IN predicate optimization feature, integrating parameter filtering logic into RouteSQLRewriteEngine.

Changes

Modified RouteSQLRewriteEngine.java

  • Enhanced getParameters() method to call parameter filtering
  • Added filterParametersIfNeeded() method to detect and apply parameter filtering when ParameterFilterable tokens exist
  • Added findParameterFilterableTokens() method to identify tokens implementing ParameterFilterable interface
  • Added applyParameterFiltering() method to merge removed indices from multiple tokens and filter parameters

Added unit tests in RouteSQLRewriteEngineTest.java

  • Test behavior when no filterable tokens exist
  • Test single token parameter filtering
  • Test multiple tokens with index merging
  • Test parameter order preservation after filtering

Design Rationale

  • Zero performance impact when no filterable tokens exist (only a simple list traversal)
  • Supports multiple ParameterFilterable tokens with automatic index merging
  • Preserves parameter order for remaining parameters
  • Fully backward compatible - existing tokens continue to work without changes

Related Issue

Part of #36454

Next Steps

The next PR will introduce data structures (ShardingInPredicateValue and ShardingInPredicateToken) that utilize this parameter filtering infrastructure.

@Fottas Fottas changed the title [Part 2/9] Integrate parameter filtering into RouteSQLRewriteEngine [Part 2] Integrate parameter filtering into RouteSQLRewriteEngine Dec 2, 2025
Copy link
Member

@terrymanu terrymanu left a comment

Choose a reason for hiding this comment

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

Right now the changes are spread across multiple PRs, which makes it hard to assess the overall design and impact. Please consolidate the main code into a single, complete PR so we can review the design, code, and tests together. Otherwise, each fragmented PR lacks context and cannot be merged.

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.

2 participants