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 UpdateByTable QST #2609

Merged
merged 3 commits into from
Jul 1, 2022
Merged

Add UpdateByTable QST #2609

merged 3 commits into from
Jul 1, 2022

Conversation

devinrsmith
Copy link
Member

Fixes #2608

Comment on lines 1778 to 1779
Table updateBy(@NotNull Collection<? extends UpdateByClause> operations,
@NotNull Collection<? extends Selectable> byColumns);
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I left this out of your original review (b/c it was easy for me to add in this PR); but from a QST point of view, I need to be able to call updateBy w/ these exact parameters. Usage in TableAdapterImpl.

@@ -517,6 +519,23 @@ TOPS raj(TABLE rightTable, Collection<? extends JoinMatch> columnsToMatch,

// -------------------------------------------------------------------------------------------

TOPS updateBy(UpdateByClause operation);
Copy link
Member

Choose a reason for hiding this comment

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

Should the JavaDoc be lifted up to here, rather than on Table?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe? At least right now, the javadoc refers to #getRowSet which isn't a concept at the TableOperations level. We've been a bit inconsistent in these regards - some of the TableOperations have javadoc, others just on Table.

@devinrsmith devinrsmith requested a review from rcaudy July 1, 2022 16:59
@devinrsmith devinrsmith merged commit 0d46f36 into deephaven:main Jul 1, 2022
@devinrsmith devinrsmith deleted the updateby-qst branch July 1, 2022 18:20
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UpdateByTable QST impl
2 participants