You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "qazxcdswe123 (via GitHub)" <gi...@apache.org> on 2025/03/05 14:15:45 UTC
[PR] chore: cleanup deprecated API since `version <= 40` [datafusion]
qazxcdswe123 opened a new pull request, #15027:
URL: https://github.com/apache/datafusion/pull/15027
> Maybe it would be a good time to go remove any deprecated functions from 41 or earlier 🤔
_Originally posted by @alamb in https://github.com/apache/datafusion/pull/15016#discussion_r1981151334_
removed all API that are deprecated since `version <= 40`
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#issuecomment-2703986142
Thank you @qazxcdswe123
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#discussion_r1982240264
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -329,19 +311,10 @@ fn optimize_plan_node(
config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
if rule.supports_rewrite() {
- return rule.rewrite(plan, config);
+ rule.rewrite(plan, config)
+ } else {
Review Comment:
Actually thinking about this more I think we should simply always call rule.rewrite now (as it is required to implement now)
I am sorry about this @qazxcdswe123 but could you maybe revert the changes to OptimizerRule in this PR (and we can work on that in a follow on PR?)
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#discussion_r1981630528
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -329,19 +311,10 @@ fn optimize_plan_node(
config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
if rule.supports_rewrite() {
- return rule.rewrite(plan, config);
+ rule.rewrite(plan, config)
+ } else {
Review Comment:
Can we please make this return an internal error rather than silently error?
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#issuecomment-2703985883
🚀
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #15027:
URL: https://github.com/apache/datafusion/pull/15027
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "qazxcdswe123 (via GitHub)" <gi...@apache.org>.
qazxcdswe123 commented on code in PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#discussion_r1982975859
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -329,19 +311,10 @@ fn optimize_plan_node(
config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
if rule.supports_rewrite() {
- return rule.rewrite(plan, config);
+ rule.rewrite(plan, config)
+ } else {
Review Comment:
Done.
How should the `Optimizer` part behave? Should it just `rule.rewrite`, I think some of the comment is also out of date then
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#issuecomment-2702131558
> lgtm thanks @qazxcdswe123 @alamb I'm feeling that having removed the deprecated code and not having migration instructions in `Migration guide` it would be quite a challenge to migrate from version 39 to 46 as we do not keep any traces/hints how API evolved and what is the new API
In my opinion any additional small burden related to this API removal (of APIs that have been deprecated for 6 months) is likely tiny compared to all the other major changes.
Thus I don't think adding a migration guide adds much value here -- the fact the compiler has been telling people for months it is deprecated is the migration guide
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "qazxcdswe123 (via GitHub)" <gi...@apache.org>.
qazxcdswe123 commented on code in PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#discussion_r1981584124
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -329,19 +311,10 @@ fn optimize_plan_node(
config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
if rule.supports_rewrite() {
- return rule.rewrite(plan, config);
+ rule.rewrite(plan, config)
+ } else {
Review Comment:
potential semanic change here
--
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] chore: cleanup deprecated API since `version <= 40` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15027:
URL: https://github.com/apache/datafusion/pull/15027#issuecomment-2701092355
😍 thank you
--
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