Two profiler improvements (two separate commits)#5807
Conversation
mythical-fred
left a comment
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM. The abstract copy() on CalciteRelNode is correctly implemented in all subclasses, the WindowOperator constructor simplification (null function instead of NoExpression sentinel) is cleaner, and the position propagation in RewriteNow properly attaches source info to the synthesized window node. windowPositionTest is a good regression guard.
|
As a low-effort impovement, if you add a few |
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
mythical-fred
left a comment
mythical-fred
left a comment
There was a problem hiding this comment.
Two clean additions: the profiler tooltip now shows node id+operation in the table title (all impls correctly propagate the new list), and gets proper source position tracking with a backing test in . Still LGTM.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
4 participants
Describe Manual Test Plan
Here is a screenshot of the new display: