Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PredicatePushdownOptimizer in tracking-only mode #1262

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

tlento
Copy link
Contributor

@tlento tlento commented Jun 12, 2024

In order to run predicate pushdown in a more robust manner we would
be better served applying it as a post-plan-building optimizer rather
than a build-time injection of additional where constraints.

This PR adds the optimizer class with full tracking of predicate
pushdown state, but it simply returns all nodes in the input
DataflowPlan without modifying anything, since the predicate pushdown
is already applied in the DataflowPlanBuilder.

Future updates will move both the where constraint and time range constraint
pushdown operations into this optimizer class. For now we simply evaluate
the output from console logs to get a basis for future implementation changes.

Copy link
Contributor Author

tlento commented Jun 12, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @tlento and the rest of your teammates on Graphite Graphite

Copy link
Contributor

@courtneyholcomb courtneyholcomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - excited to see this get used!

if len(node_semantic_models) != 1 or not current_pushdown_state.has_where_filters_to_push_down:
return self._default_handler(node)

source_semantic_model, *_ = node_semantic_models
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about *_!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that was mind-blowing. 🤯 🤣

Copy link
Contributor Author

tlento commented Jun 25, 2024

Merge activity

  • Jun 24, 8:29 PM PDT: @tlento started a stack merge that includes this pull request via Graphite.
  • Jun 24, 8:40 PM PDT: Graphite rebased this pull request as part of a merge.
  • Jun 24, 8:43 PM PDT: @tlento merged this pull request with Graphite.

@tlento tlento force-pushed the add-join-filter-itests branch from 8af1ba3 to eec446c Compare June 25, 2024 03:35
Base automatically changed from add-join-filter-itests to main June 25, 2024 03:39
In order to run predicate pushdown in a more robust manner we would
be better served applying it as a post-plan-building optimizer rather
than a build-time injection of additional where constraints.

This PR adds the optimizer class with full tracking of predicate
pushdown state, but it simply returns all nodes in the input
DataflowPlan without modifying anything, since the predicate pushdown
is already applied in the DataflowPlanBuilder.

Future updates will move both the where constraint and time range constraint
pushdown operations into this optimizer class. For now we simply evaluate
the output from console logs to get a basis for future implementation changes.
@tlento tlento force-pushed the add-tracking-only-pushdown-optimizer branch from c6f8777 to 16ae775 Compare June 25, 2024 03:39
@tlento tlento merged commit 7758362 into main Jun 25, 2024
15 checks passed
@tlento tlento deleted the add-tracking-only-pushdown-optimizer branch June 25, 2024 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants