Skip to content

Minor cleanup of WindowNode constructor#12273

Merged
sopel39 merged 1 commit intotrinodb:masterfrom
pettyjamesm:cleanup-window-node-constructor
May 16, 2022
Merged

Minor cleanup of WindowNode constructor#12273
sopel39 merged 1 commit intotrinodb:masterfrom
pettyjamesm:cleanup-window-node-constructor

Conversation

@pettyjamesm
Copy link
Copy Markdown
Member

Description

Minor cleanup to avoid redundant copies during validation and field assignment in the WindowNode constructor. Also changes a usage of List#containsAll to the Set#containsAll equivalent.

Is this change a fix, improvement, new feature, refactoring, or other?

Minor refactor.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Core engine.

How would you describe this change to a non-technical end user or system administrator?

This change should not need to be described to non-technical users or administrators.

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label May 6, 2022
@pettyjamesm pettyjamesm requested a review from findepi May 6, 2022 20:25
@findepi findepi requested review from raunaqmorarka and sopel39 and removed request for findepi May 9, 2022 07:40
@sopel39 sopel39 requested a review from lukasz-stec May 9, 2022 12:54
Copy link
Copy Markdown
Member

@lukasz-stec lukasz-stec left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Member

@raunaqmorarka raunaqmorarka left a comment

Choose a reason for hiding this comment

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

Please add the PR description to the commit message.

@pettyjamesm pettyjamesm force-pushed the cleanup-window-node-constructor branch 3 times, most recently from d4477f2 to 8a31587 Compare May 10, 2022 20:40
Copy link
Copy Markdown
Member

@gaurav8297 gaurav8297 left a comment

Choose a reason for hiding this comment

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

LGTM

@pettyjamesm pettyjamesm force-pushed the cleanup-window-node-constructor branch 2 times, most recently from ee8f897 to d69ed2e Compare May 11, 2022 17:12
@pettyjamesm
Copy link
Copy Markdown
Member Author

Test failures appear unrelated and have been seemingly random throughout the day.

Minor cleanup to avoid redundant copies during validation and field
assignment in the WindowNode constructor. Also changes a usage of
List#containsAll to the Set#containsAll equivalent.
@pettyjamesm pettyjamesm force-pushed the cleanup-window-node-constructor branch from d69ed2e to 4f9eea6 Compare May 13, 2022 16:56
@sopel39
Copy link
Copy Markdown
Member

sopel39 commented May 16, 2022

Failed due to #12415

@sopel39 sopel39 merged commit b8e1ca1 into trinodb:master May 16, 2022
@github-actions github-actions bot added this to the 381 milestone May 16, 2022
@pettyjamesm pettyjamesm deleted the cleanup-window-node-constructor branch March 24, 2023 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants