Port DH-12148: Formula Array Access #3346
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nbauernfeind
added
query engine
NoDocumentationNeeded
ReleaseNotesNeeded
lbooker42
reviewed
Mar 7, 2023
engine/updategraph/src/main/java/io/deephaven/engine/updategraph/UpdateGraphProcessor.java
Outdated
Show resolved
Hide resolved
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java
Show resolved
Hide resolved
lbooker42
previously approved these changes
Mar 7, 2023
lbooker42
previously approved these changes
Mar 8, 2023
lbooker42
approved these changes
Mar 10, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I had to pull together several PR's to completely port this feature. There was a lot more work than I anticipated lurking beneath the façade of DH-12148.
I felt that ShiftColumnOperation could be improved on from a performance point of view. I've included a test that arbitrarily takes ~30s on the original implementation, and ~8s on the replacement implementation. This is +70% improvement for that arbitrary, but hand picked, case using random updates. There are degenerate scenarios of the original implementation that the new implementation avoids. I dropped the replacement in a separate commit should one want to review the port vs the rewrite.
An interesting bonus of supporting formula array access is that we can drop most usages of
EvalNugget#hasUnstableColumnsas array offset access is now guaranteed to propagate downstream. This helped me identify a bug or two.There were a couple of snippets/scenarios in the DHE port that just did not work in DHC. I reached out to @cpwright offline to highlight concerns.
DH-12148: Formula Array Access changes commit
DH-12030 Implement ShiftColumnOperation commit
DH-12199: ShiftedColumnOperation's ModifiedColumnSet should include all Source Column + Shifted Columns commit
DH-12273: FormulaArrayAccess changes for where clause commit
Nightlies are running here.