Skip to content

fix: DH-22381: Correct guard logic in adaptEmOptions for EM aggregations#7938

Merged
cpwright merged 3 commits intodeephaven:mainfrom
lbooker42:nightly/DH-22381-updateby-grpc-options
Apr 28, 2026
Merged

fix: DH-22381: Correct guard logic in adaptEmOptions for EM aggregations#7938
cpwright merged 3 commits intodeephaven:mainfrom
lbooker42:nightly/DH-22381-updateby-grpc-options

Conversation

@lbooker42
Copy link
Copy Markdown
Contributor

No description provided.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

No docs changes detected for 34d0561

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes the server-side guard logic when adapting UpdateByEmOptions for exponential moving (EM*) update-by operations, ensuring only explicitly specified on_null_value / on_nan_value behaviors are applied and avoiding passing NOT_SPECIFIED into adaptBadDataBehavior.

Changes:

  • Correct adaptEmOptions() to guard on on_null_value / on_nan_value being specified (!= NOT_SPECIFIED) and to check the correct field for the null guard.
  • Add a new gRPC test class intended to cover combinations of EM options for EMA and EmStd operations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
server/src/main/java/io/deephaven/server/table/ops/UpdateByGrpcImpl.java Fixes the guard logic in adaptEmOptions() so OperationControl is only populated when behaviors are actually specified.
server/src/test/java/io/deephaven/server/table/ops/UpdateByEmOptionsGrpcTest.java Adds tests around EM option combinations for EMA / EmStd, aiming to prevent regressions in option adaptation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server/src/test/java/io/deephaven/server/table/ops/UpdateByEmOptionsGrpcTest.java Outdated
Comment thread server/src/test/java/io/deephaven/server/table/ops/UpdateByEmOptionsGrpcTest.java Outdated
Comment thread server/src/test/java/io/deephaven/server/table/ops/UpdateByEmOptionsGrpcTest.java Outdated
lbooker42 and others added 2 commits April 24, 2026 12:17
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lbooker42 lbooker42 requested a review from cpwright April 28, 2026 17:31
@cpwright cpwright merged commit f6ea293 into deephaven:main Apr 28, 2026
24 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants