Skip to content

compute: reduce boilerplate in linear join rendering #22563

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.

Sign up for GitHub

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

Jump to bottom
Merged
teskje merged 1 commit into from
Oct 23, 2023
Merged

compute: reduce boilerplate in linear join rendering #22563

teskje merged 1 commit into from
Oct 23, 2023

Conversation

@teskje
Copy link
Contributor

@teskje teskje commented Oct 22, 2023 •
edited
Loading

This PR reduces the amount of boilerplate code in linear join rendering in two ways:

  • It replaces the four dispatch_ functions with a single macro.
  • It makes differential_join and differential_join_inner methods of Context, to reduce the number of arguments that need to be passed around.

Motivation

  • This PR refactors existing code.

Tips for reviewer

The diff looks much better with whitespace ignored!

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:
    • N/A

@teskje teskje marked this pull request as ready for review October 22, 2023 09:44
@teskje teskje requested review from a team, antiguru and vmarcos October 22, 2023 09:44
@teskje teskje mentioned this pull request Oct 22, 2023
Merged
5 tasks
@teskje teskje force-pushed the reduce-join-boilerplate branch from 957b326 to 3e8e9c7 Compare October 22, 2023 09:48
@teskje
compute: reduce boilerplate in linear join rendering
ed684d0 
This commit reduces the amount of boilerplate code in linear join
rendering in two ways:

 * It replaces the four `dispatch_` functions with a single macro.
 * It makes `differential_join` and `differential_join_inner` methods of
   `Context`, to reduce the number of arguments that need to be passed
   around.
@teskje teskje force-pushed the reduce-join-boilerplate branch from 3e8e9c7 to ed684d0 Compare October 23, 2023 10:19
vmarcos
vmarcos approved these changes Oct 23, 2023
View reviewed changes
Copy link
Contributor

@vmarcos vmarcos left a comment

Choose a reason for hiding this comment

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

Thanks!

@teskje
Copy link
Contributor Author

teskje commented Oct 23, 2023

TFTRs!

@teskje teskje merged commit 072ff08 into MaterializeInc:main Oct 23, 2023
@teskje teskje deleted the reduce-join-boilerplate branch October 23, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antiguru antiguru antiguru approved these changes

+1 more reviewer

@vmarcos vmarcos vmarcos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@teskje @antiguru @vmarcos