Push resolved linkable elements through to WhereFilterSpec #1179
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
| @@ -672,6 +677,7 @@ class WhereFilterSpec(Mergeable, SerializableDataclass): | |||
| where_sql: str | |||
| bind_parameters: SqlBindParameters | |||
| linkable_specs: Tuple[LinkableInstanceSpec, ...] | |||
| linkable_elements: Tuple[LinkableElement, ...] | |||
There was a problem hiding this comment.
Not necessary for this PR, but I wonder if it would be cleaner to add some simple converters between linkable specs and linkable elements so that you don't have to pass both through everywhere.
There was a problem hiding this comment.
We've talked about consolidating these and I think we'll get there eventually. One of the issues we have right now is LinkableSpecs aren't convertible to LinkableElements because the latter contain a bunch of additional metadata that LinkableSpecs can't carry along with them, so there's no way to simply add that information without causing all kinds of havoc with existing spec comparisons (e.g., some_entity_spec == some_other_entity_spec).
For now I could pass the LinkableElementSet along but that gets slightly more complicated so I decided to wire the elements through. I might still switch over to passing the set to the end here. I'm honestly not sure which approach is less annoying to read.
There was a problem hiding this comment.
I could replace the linkable_specs with a property accessor that converts from the element to a spec, I suppose, but it's a bit gnarly without the LinkableElementSet.
tlento
force-pushed
the
linkable-elements-to-filter-spec-lookup
branch
from
4a62cec to
c2e773b
Compare
tlento
force-pushed
the
linkable-elements-to-where-filter-spec
branch
from
402f25f to
db53b83
Compare
tlento
force-pushed
the
linkable-elements-to-filter-spec-lookup
branch
from
c2e773b to
3f454a4
Compare
tlento
force-pushed
the
linkable-elements-to-where-filter-spec
branch
from
db53b83 to
c494b04
Compare
tlento
force-pushed
the
linkable-elements-to-filter-spec-lookup
branch
from
3f454a4 to
73f7b85
Compare
The resolved LinkableElements gathered during the filter spec resolution phase of query processing need to be available to each WhereFilterSpec so that we can determine whether or not a given WhereFilterSpec is eligible for predicate pushdown. This change simply adds the necessary property and wires these elements through to where we will need to access them in the DataflowPlanBuilder. This change proved a bit troublesome due to the SerializableDataclass construct, which has some particular requirements which required some slightly odd inheritance tagging.
tlento
force-pushed
the
linkable-elements-to-where-filter-spec
branch
from
c494b04 to
88469f0
Compare
Push resolved linkable elements through to WhereFilterSpec
The resolved LinkableElements gathered during the filter spec
resolution phase of query processing need to be available to
each WhereFilterSpec so that we can determine whether or not
a given WhereFilterSpec is eligible for predicate pushdown.
This change simply adds the necessary property and wires these
elements through to where we will need to access them in the
DataflowPlanBuilder. This change proved a bit troublesome due
to the SerializableDataclass construct, which has some particular
requirements which required some slightly odd inheritance tagging.