-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Generalize non-streaming result stats aggregation #16480
Conversation
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Instead of handling RowsAffected directly Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Instead of updating RowsAffected specifically Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Signed-off-by: Rafer Hazen <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16480 +/- ##
==========================================
+ Coverage 68.64% 68.68% +0.04%
==========================================
Files 1551 1551
Lines 199471 199719 +248
==========================================
+ Hits 136925 137178 +253
+ Misses 62546 62541 -5 ☔ View full report in Codecov by Sentry. |
This reverts commit 251d690. Signed-off-by: Rafer Hazen <[email protected]>
6c9a3d0
to
fcda667
Compare
slapping the |
Closing in favor of #16638 |
Description
Introduces
Result#MergeStats
and ensures that the non-streaming versions (#TryExecute
) of allPrimitives
return aggregated stats for any underlying wrapped primitives.MergeStats
only handlesRowsAffected
at the moment, but extracting this function and making sure it is called on wrapped primitives makes it easier for downstream forks to add additional result-level stats (by updatingMergeStats
), and ensures that they correctly are propagated (and not dropped).This is an incremental improvement, and does not attempt to implement stats aggregation in
#TryStreamExecute
.Related Issue(s)
#16481
Checklist
Deployment Notes
N/A