alamb commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3049011361
🎉 thank you @adriangb and @kosiew
--
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
kosiew merged PR #16642:
URL: https://github.com/apache/datafusion/pull/16642
--
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-unsubscr...@datafus
kosiew commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3047833471
yep
--
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 unsub
alamb commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3045669349
@kosiew are you happy with this PR too?
--
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
adriangb commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3044828528
Yes by me!
--
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
alamb commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3044597593
Is this PR ready to merge?
--
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
kosiew commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2184184042
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -291,18 +136,8 @@ impl FilterPushdownPropagation {
}
}
-/// Create a new [`FilterPushd
kosiew commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2184184902
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -520,10 +520,19 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
parent_filters: Vec>
adriangb commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3033712468
> Looks like has one more failure (and maybe we can merge up too)
done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on
alamb commented on PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#issuecomment-3033664854
Looks like has one more failure (and maybe we can merge up too)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182979808
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -83,163 +85,6 @@ impl PredicateSupport {
}
}
Review Comment:
Good suggestion, I like your su
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182744547
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -520,10 +520,19 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
parent_filters: Ve
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182677781
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -291,18 +136,8 @@ impl FilterPushdownPropagation {
}
}
-/// Create a new [`FilterPus
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182672509
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -520,10 +520,19 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
parent_filters: Ve
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182671663
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -291,18 +136,8 @@ impl FilterPushdownPropagation {
}
}
-/// Create a new [`FilterPus
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182669398
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -346,14 +233,48 @@ pub struct FilterDescription {
child_filter_descriptions: Vec,
}
+impl Defaul
adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182655294
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -317,24 +152,76 @@ impl FilterPushdownPropagation {
}
#[derive(Debug, Clone)]
-struct ChildFilterDes
kosiew commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182297728
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -520,10 +520,19 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
parent_filters: Vec>
alamb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2180625134
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -317,24 +152,74 @@ impl FilterPushdownPropagation {
}
#[derive(Debug, Clone)]
-struct ChildFilterDescri
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2853564869
@berkaysynnada I added some: https://github.com/apache/datafusion/pull/15955
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on t
berkaysynnada commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2852390838
@adriangb I forgot to add some note: we have converted the design here, but
there wasn't any test change or addition. I think we should add some tests to
protect these behavior
alamb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2852018421
Woohoo -- it is moving!
--
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 com
berkaysynnada merged PR #15801:
URL: https://github.com/apache/datafusion/pull/15801
--
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-unsubscr...@
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2851023517
@berkaysynnada let's merge this first; I have some work in
https://github.com/apache/datafusion/pull/15769 that will be easier once we do
/ we can wait for Andy and others to take a
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2850633452
I want to fix the benchmark I added. One sec...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL abov
berkaysynnada commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2850485713
waiting to actions 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 spec
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2850471234
Thanks @berkaysynnada! Could you take a look at 3b4c4fa? I think it should
resolve the concern in
https://github.com/apache/datafusion/pull/15801#discussion_r2073043171
--
This i
adriangb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2073150977
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -191,6 +191,7 @@ impl FilterPushdownPropagation {
#[derive(Debug, Clone)]
pub struct FilterDescripti
berkaysynnada commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2073043171
##
datafusion/physical-plan/src/filter_pushdown.rs:
##
@@ -191,6 +191,7 @@ impl FilterPushdownPropagation {
#[derive(Debug, Clone)]
pub struct FilterDesc
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2848894671
@alamb there's several in-flight PRs now. They all interact and they'll be
things we want to tweak from the resultant merged change, but could we start
merging them, starting with t
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2848840198
> However, the existing ExecutionPlan APIs are general-purpose,
self-explanatory, and simple, whereas these new ones feel harder to understand
and are quite specific to the current
berkaysynnada commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2846962860
Hi again @adriangb. I've sent the commit, but unfortunately I can't say I
fully achieved what I had in mind. Most of the work are idiomatic changes,
comments and naming -- I'd
alamb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2070810103
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -471,39 +471,53 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
Ok(None)
}
-
alamb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2845779215
> Hi @alamb. I've a WIP commit for this PR. I'm trying to both address
@adriangb concerns and needs, and at the same time trying to keep complexity at
minimum and trying to make things
adriangb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2070340727
##
datafusion/core/tests/physical_optimizer/push_down_filter.rs:
##
@@ -154,29 +153,25 @@ impl FileSource for TestSource {
fn try_pushdown_filters(
berkaysynnada commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2844916340
Hi @alamb. I've a WIP commit for this PR. I'm trying to both address
@adriangb concerns and needs, and at the same time trying to keep complexity at
minimum and trying to make
adriangb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2070247089
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -471,39 +471,53 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
Ok(None)
}
adriangb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2070247089
##
datafusion/physical-plan/src/execution_plan.rs:
##
@@ -471,39 +471,53 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
Ok(None)
}
adriangb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2070234428
##
datafusion/core/tests/physical_optimizer/push_down_filter.rs:
##
@@ -154,29 +153,25 @@ impl FileSource for TestSource {
fn try_pushdown_filters(
alamb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2070220686
##
datafusion/core/tests/physical_optimizer/push_down_filter.rs:
##
@@ -154,29 +153,25 @@ impl FileSource for TestSource {
fn try_pushdown_filters(
&
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2828261533
Sounds good happy to wait for your suggestions. And we can always iterate
again!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log
berkaysynnada commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2828219078
I understand your concerns and motivations. Since you're leading this work,
I'm happy to follow your direction. I'm sure we both aim to do what's right,
even if our priorities
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2827824299
I don't understand where this `fetch()` method would go or how it will help.
I feel that what you are proposing is not necessarily any simpler. It
requires understanding sever
berkaysynnada commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2827570314
> 1. Not requiring downcast matching of specific `ExecutionPlan`s (as
discussed previously this could be resolved by adding a new method, but in the
end both approaches requir
adriangb commented on PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#issuecomment-2822461601
cc @berkaysynnada
--
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 comme
adriangb commented on code in PR #15801:
URL: https://github.com/apache/datafusion/pull/15801#discussion_r2054603061
##
datafusion/physical-plan/src/filter.rs:
##
@@ -438,55 +440,112 @@ impl ExecutionPlan for FilterExec {
try_embed_projection(projection, self)
}
adriangb opened a new pull request, #15801:
URL: https://github.com/apache/datafusion/pull/15801
(no comment)
--
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
47 matches
Mail list logo
