-
Notifications
You must be signed in to change notification settings - Fork 847
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
Avoid projection more often in DecompressChunk node #6859
Conversation
Currently its scan targetlist is always the uncompressed tuple, but in some cases we can make the scan targetlist the same as the output targetlist, thus avoiding the projection.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6859 +/- ##
==========================================
+ Coverage 80.06% 81.74% +1.67%
==========================================
Files 190 199 +9
Lines 37181 36942 -239
Branches 9450 9654 +204
==========================================
+ Hits 29770 30198 +428
+ Misses 2997 2862 -135
+ Partials 4414 3882 -532 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-approving with request to add a testcase.
tsl/src/nodes/vector_agg/plan.c
Outdated
Var *var = castNode(Var, node); | ||
if (var->varno != OUTER_VAR) | ||
Var *aggregated_var = castNode(Var, node); | ||
if (aggregated_var->varno != OUTER_VAR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test to cover this branch as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it just can't happen, I'll change it to an assertion.
Co-authored-by: Ante Kresic <antekresic@users.noreply.github.com> Signed-off-by: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com>
Currently its scan targetlist is always the uncompressed tuple, but in some cases we can make the scan targetlist the same as the required output targetlist, thus avoiding the projection.
This is required to enable vectorized aggregation in more cases, but is also a performance improvement in itself. In tsbench, it gives up to 25% speedup for some queries: https://grafana.ops.savannah-dev.timescale.com/d/fasYic_4z/compare-akuzm?orgId=1&var-branch=All&var-run1=3461&var-run2=3469&var-threshold=0&var-use_historical_thresholds=true&var-threshold_expression=2.5%20%2A%20percentile_cont%280.90%29&var-exact_suite_version=false&from=now%2Fy&to=now%2Fy
This partially implements https://github.com/timescale/eng-database/issues/564
Disable-check: force-changelog-file