Fix multiple consecutive Exchange returning empty response #11885
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11885 +/- ##
============================================
- Coverage 61.45% 61.40% -0.05%
Complexity 1147 1147
============================================
Files 2375 2375
Lines 128500 128501 +1
Branches 19846 19846
============================================
- Hits 78974 78912 -62
- Misses 43815 43892 +77
+ Partials 5711 5697 -14
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 13 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Jackie-Jiang
force-pushed
the
fix_multiple_exchanges
branch
from
7c6d0fb
to
1178bc7
Compare
| // Set previous PlanFragment ID in the context to be the next PlanFragment ID to be used by the child node. | ||
| context._previousPlanFragmentId = nextPlanFragmentId; |
There was a problem hiding this comment.
the logic of previous/current plan fragment id looks very convoluted
in normal node, _previousPlanFragmentId is set to the "node itself before it visit its children, thus guaranteed at the time child is being visit, current is always what the child is suppose to be on, and the previous is what the parent suppose to be on.
in exchange _previousPlanFragmentId setting to the next plan fragment id seems weird.
thus "previousPlanFragmentId" really should be named "plan fragment id for the parent PlanNode": the actual problem is
- once exchange visits the child, the plan break into fragments
- b/c mailbox send doesn't have a parent, the "plan fragment id for the parent" definition is moot, and thus will just use the current plan fragment ID :-/
^which still seems weird, i wonder keeping this as a global context might not be the best way
There was a problem hiding this comment.
this might be more clear:
| // Set previous PlanFragment ID in the context to be the next PlanFragment ID to be used by the child node. | |
| context._previousPlanFragmentId = nextPlanFragmentId; | |
| // when visiting the child of the exchange, the parent is no longer the exchange node, but the mailboxSendNode | |
| // created by this exchange, thus the "previousPlanFragmentId" when visiting the child should be nextPlanFragmentId | |
| context._previousPlanFragmentId = nextPlanFragmentId; |
please let me know if my understanding is correct
Fix #11880