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

feat(plan): generalize attributes in planner #4982

Merged
merged 2 commits into from
Jul 13, 2022
Merged

Conversation

wolffcm
Copy link

@wolffcm wolffcm commented Jul 8, 2022

This set of changes creates a framework whereby plan nodes can model characteristics of the data they consume and produce. These characteristics are called attributes in the code and currently there are three different kinds:

  • collation to indicate data that is collated (sorted) by particular columns
  • parallel-run to indicate that operations are run in parallel
  • parallel-merge to indicate a plan node that represents a merging of parallel data streams

The parallel attributes were previously existing, but with this set of changes it is the procedure specs themselves that describe whether run in parallel or if they merge parallel streams.

Different attributes being produced or required by a particular operation is expressed by implementing interfaces. The comment at the top of attributes.go describes this in more detail.

This change set is mostly the same as the POC that was in #4902. There is a more comprehensive description over there, which is also valid for this PR. The main differences from this PR are changes that were required to allow the tests in parallel_test.go to pass.

This work is part of (but does not complete) #4628.

@wolffcm wolffcm requested a review from a team as a code owner July 8, 2022 21:54
@wolffcm wolffcm requested review from OfTheDelmer and adrian-thurston and removed request for a team July 8, 2022 21:54
return true
}
return false
}
Copy link
Contributor

@adrian-thurston adrian-thurston Jul 11, 2022

Choose a reason for hiding this comment

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

This is slick! ⬆️

@adrian-thurston adrian-thurston self-requested a review July 11, 2022 22:59
Copy link
Contributor

@adrian-thurston adrian-thurston left a comment

Choose a reason for hiding this comment

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

Looks good. Tried this out with a new parallel pivot experiment I have and it works great.

ts []execute.Transformation
a execute.Administration
id execute.DatasetID
factor int
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

plan/attributes.go Outdated Show resolved Hide resolved
@wolffcm wolffcm merged commit d01f82a into master Jul 13, 2022
@wolffcm wolffcm deleted the feat/planner-attributes branch July 13, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants