Skip to content

Consolidate CastColumnExpr with CastColumn - #19097

Closed
alamb wants to merge 1 commit into
apache:mainfrom
alamb:alamb/remove_cast_column
Closed

Consolidate CastColumnExpr with CastColumn#19097
alamb wants to merge 1 commit into
apache:mainfrom
alamb:alamb/remove_cast_column

Conversation

@alamb

@alambalamb commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

@kosiew added struct aware casting in the following PR

However, that relies on a new physical CastColumnExpr that has a non trivial redundancy with CastExpr.

Instead of relying on the new PhysicalExpr, I we should upgrade CastExpr to handle both normal and struct casting

What changes are included in this PR?

  1. Consolidate CastColumnExpr with CastColumn
  2. Remove CastColumnExpr

I had codex do the first version for me, and it did quite a nice job

Are these changes tested?

Yes by CI

Are there any user-facing changes?

No

@github-actionsgithub-actionsBot added physical-expr Changes to the physical-expr crates common Related to common crate execution Related to the execution crate labels Dec 4, 2025
@alamb

alamb commented Dec 4, 2025

Copy link
Copy Markdown
ContributorAuthor

I need to review this code a little more but I think it is looking pretty good

Comment threaddatafusion/common/src/nested_struct.rs Outdated
@alamb
alambforce-pushed the alamb/remove_cast_column branch from 9fd6490 to 5ef668cCompareDecember 6, 2025 13:14
@github-actionsgithub-actionsBot removed the execution Related to the execution crate label Dec 6, 2025
@github-actions

Copy link
Copy Markdown

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actionsgithub-actionsBot added the Stale PR has not had any activity for some time label Feb 11, 2026
@AdamGS

Copy link
Copy Markdown
Contributor

Is this still desired? Happy to help to push it through or help in any other way

@kosiew

Copy link
Copy Markdown
Contributor

hi @AdamGS
We closed this issue in #20164

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commonRelated to common cratecoreCore DataFusion cratephysical-exprChanges to the physical-expr cratesprotoRelated to proto crateStalePR has not had any activity for some time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate cast_column into PhysicalExprAdapter

3 participants

@alamb@AdamGS@kosiew