xudong963 closed pull request #14207: fix: fetch is missed during
EnforceDistribution
URL: https://github.com/apache/datafusion/pull/14207
--
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 specifi
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971330872
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
berkaysynnada commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971311766
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext
berkaysynnada commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971311766
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971299668
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
berkaysynnada commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971303497
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext
berkaysynnada commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971291950
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971299668
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971122511
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971122511
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1968751449
##
datafusion/core/tests/physical_optimizer/enforce_distribution.rs:
##
@@ -3154,3 +3164,104 @@ fn optimize_away_unnecessary_repartition2() ->
Result<()> {
alamb commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2678487816
> > Besides this test, a data test in .slt's would still be helpful IMO
>
> Yes, but I don't find such a sqllogictest to reproduce this bug without
changing the default optimizer
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1967465982
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -1361,22 +1390,67 @@ pub fn ensure_distribution(
} else {
plan.with_new_childre
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1967335967
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -1361,22 +1390,67 @@ pub fn ensure_distribution(
} else {
plan.with_new_childre
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2677884058
> Besides this test, a data test in .slt's would still be helpful IMO
Yes, but I don't find such a sqllogictest to reproduce this bug without
changing the default optimizer r
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1967294411
##
datafusion/core/tests/physical_optimizer/enforce_distribution.rs:
##
@@ -3154,3 +3164,104 @@ fn optimize_away_unnecessary_repartition2() ->
Result<()> {
berkaysynnada commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1966888998
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -1021,21 +1043,25 @@ fn remove_dist_changing_operators(
fn replace_order_preserving_vari
alamb commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2676862136
I plan to review this one carefully tomorrow
--
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 g
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2665954212
cc @wiedld , maybe you are interested in the 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 ab
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1959896775
##
datafusion/core/tests/physical_optimizer/enforce_distribution.rs:
##
@@ -3172,3 +3181,78 @@ fn optimize_away_unnecessary_repartition2() ->
Result<()> {
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1959895206
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -1362,6 +1383,21 @@ pub fn ensure_distribution(
plan.with_new_children(children_plan
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2665089862
I'm still resolving the comments from @alamb , I'll ping you @alamb
@berkaysynnada to review after I'm ready!
--
This is an automated message from the Apache Git Service.
To resp
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1959179376
##
datafusion/core/tests/physical_optimizer/enforce_distribution.rs:
##
@@ -3172,3 +3181,78 @@ fn optimize_away_unnecessary_repartition2() ->
Result<()> {
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1959138660
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -1020,23 +1033,26 @@ fn remove_dist_changing_operators(
/// ```
fn replace_order_preserving
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1959171412
##
datafusion/core/tests/physical_optimizer/enforce_distribution.rs:
##
@@ -3172,3 +3181,78 @@ fn optimize_away_unnecessary_repartition2() ->
Result<()> {
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2664517439
Yes, the bug still lives.
--
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 speci
berkaysynnada commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2663989003
@xudong963 the bug still lives?
--
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
alamb commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2648266814
I wonder if this PR is related to
- https://github.com/apache/datafusion/pull/14569
--
This is an automated message from the Apache Git Service.
To respond to the message, please
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2631275476
> What I don't understand is how this fix related to running
`EnforceDistribution` twice. I understand the bug surfaces when
`EnforceDistribution` is run twice, but it seems like t
xudong963 commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1939527050
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -986,18 +993,24 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
/
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2627501242
Thanks, @alamb, I'm on vacation, will reply asap
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL ab
alamb commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1934089417
##
datafusion/physical-optimizer/src/enforce_distribution.rs:
##
@@ -932,12 +932,16 @@ fn add_hash_on_top(
/// # Arguments
///
/// * `input`: Current node.
+/// *
alamb commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2621967843
I merged up from main to resolve a conflict on this PR as part of my review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to Gi
xudong963 commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2620477661
thanks @berkaysynnada @alamb
--
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 sp
alamb commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2620061423
I plan to review this PR later today or tomorrow as it is on my "45
blockers" list
Thank you for your patience @xudong963
--
This is an automated message from the Apache Git
berkaysynnada commented on PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#issuecomment-2613992380
This week I couldn't spare time to review this fix, sorry @xudong963. That
will be one of my priorities in the next week.
--
This is an automated message from the Apache Git
36 matches
Mail list logo
