Skip to content

Refactor schema rewriter: remove lifetimes, extract column/cast helpers, add mismatch coverage#20166

Merged
adriangb merged 3 commits intoapache:mainfrom
kosiew:schema-cleanup-20161
Feb 5, 2026
Merged

Refactor schema rewriter: remove lifetimes, extract column/cast helpers, add mismatch coverage#20166
adriangb merged 3 commits intoapache:mainfrom
kosiew:schema-cleanup-20161

Conversation

@kosiew
Copy link
Contributor

@kosiew kosiew commented Feb 5, 2026

Which issue does this PR close?

Rationale for this change

This change is a focused refactor of the PhysicalExprAdapter schema rewriter to improve readability and maintainability while preserving behavior.

Key motivations:

  • Reduce complexity from explicit lifetimes by storing schema references as SchemaRef.
  • Make column/index/type handling easier to follow by extracting helper functions.
  • Strengthen the test suite to ensure refactors do not alter adapter output.

What changes are included in this PR?

  • Refactored DefaultPhysicalExprAdapterRewriter to own SchemaRef values instead of borrowing &Schema.

    • Simplifies construction and avoids lifetime plumbing.
  • Simplified column rewrite logic by:

    • Early-exiting when both the physical index and data type already match.
    • Extracting resolve_column to handle physical index/name resolution.
    • Extracting create_cast_column_expr to validate cast compatibility (including nested structs) and build CastColumnExpr.
  • Minor cleanups in struct compatibility validation and field selection to ensure the cast checks are performed against the actual physical field resolved by the final column index.

  • Test updates and additions:

    • Simplified construction of expected struct Fields in tests for clarity.
    • Added test_rewrite_column_index_and_type_mismatch to validate the combined case where the logical column index differs from the physical schema and the data type requires casting.

Are these changes tested?

Yes.

  • Existing unit tests continue to pass.

  • Added a new unit test to cover the index-and-type mismatch scenario for column rewriting, asserting:

    • The inner Column points to the correct physical index.
    • The resulting expression is a CastColumnExpr producing the expected logical type.

Are there any user-facing changes?

No.

  • This is a refactor/cleanup intended to preserve existing behavior.
  • No public API changes, no behavioral changes expected in query results.

LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed and tested.

@kosiew kosiew marked this pull request as ready for review February 5, 2026 10:25
@adriangb
Copy link
Contributor

adriangb commented Feb 5, 2026

Thanks @kosiew !

@adriangb adriangb added this pull request to the merge queue Feb 5, 2026
Merged via the queue into apache:main with commit 639971a Feb 5, 2026
32 checks passed
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.

PhysicalExprAdapter - Schema Rewriter Cleanup

2 participants