You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2025/12/04 22:22:08 UTC
[PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
alamb opened a new pull request, #19097:
URL: https://github.com/apache/datafusion/pull/19097
## Which issue does this PR close?
- Closes https://github.com/apache/datafusion/issues/17330 (and all subtasks I think)
## Rationale for this change
@kosiew added struct aware casting in the following PR
- https://github.com/apache/datafusion/pull/17773
However, that relies on a new physical `CastColumnExpr` that has a non trivial redundancy with `CastExpr`.
Instead of relying on the new PhysicalExpr, I think we can upgrade `CastExpr` to handle structures
## 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
(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #19097:
URL: https://github.com/apache/datafusion/pull/19097#discussion_r2591064759
##########
datafusion/common/src/nested_struct.rs:
##########
@@ -54,31 +54,50 @@ fn cast_struct_column(
target_fields: &[Arc<Field>],
cast_options: &CastOptions,
) -> Result<ArrayRef> {
+ if source_col.data_type().is_null() {
Review Comment:
what is really weird is that CastColumnExpr doesn't seem to be used anywhere other than tests 🤔
I could probably straight up delete that struct and have all the tests pass 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on PR #19097:
URL: https://github.com/apache/datafusion/pull/19097#issuecomment-4281735223
Is this still desired? Happy to help to push it through or help in any other way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #19097:
URL: https://github.com/apache/datafusion/pull/19097#issuecomment-4285681917
hi @AdamGS
We closed this issue in #20164
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19097:
URL: https://github.com/apache/datafusion/pull/19097#issuecomment-3614558703
I need to review this code a little more but I think it is looking pretty good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19097:
URL: https://github.com/apache/datafusion/pull/19097#issuecomment-3881730534
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate `CastColumnExpr` with `CastColumn` [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #19097: Consolidate `CastColumnExpr` with `CastColumn`
URL: https://github.com/apache/datafusion/pull/19097
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org