Skip to content
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

Fix const null clauses in runtime chunk exclusion #4880

Merged
merged 2 commits into from Nov 15, 2022

Conversation

akuzm
Copy link
Member

@akuzm akuzm commented Oct 26, 2022

The code we inherited from postgres expects that if we have a const null or false clause, it's going to be the single one, but that's not true for runtime chunk exclusion because we don't try to fold such restrictinfos after evaluating the mutable functions. Fix it to also work for multiple restrictinfos.

Related to https://github.com/timescale/Support-Dev-Collab/issues/620

Disable-check: commit-count

@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Merging #4880 (28a4310) into main (1847f64) will increase coverage by 0.11%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4880      +/-   ##
==========================================
+ Coverage   89.51%   89.62%   +0.11%     
==========================================
  Files         226      226              
  Lines       50818    51105     +287     
==========================================
+ Hits        45491    45805     +314     
+ Misses       5327     5300      -27     
Impacted Files Coverage Δ
src/chunk.h 100.00% <ø> (ø)
src/nodes/chunk_append/planner.c 93.08% <ø> (ø)
src/ts_catalog/catalog.c 85.91% <ø> (ø)
src/ts_catalog/catalog.h 100.00% <ø> (ø)
src/utils.h 80.00% <ø> (ø)
tsl/src/continuous_aggs/create.c 87.68% <ø> (ø)
tsl/src/init.c 96.00% <ø> (ø)
tsl/src/nodes/decompress_chunk/planner.c 92.23% <ø> (-0.26%) ⬇️
src/chunk.c 91.07% <16.66%> (-0.09%) ⬇️
src/bgw/job.c 93.24% <75.00%> (-0.74%) ⬇️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3a3da7...28a4310. Read the comment docs.

@fabriziomello
Copy link
Contributor

@akuzm just an advice, you can ask for support folks to convert the SDC issue into a public issue.

Copy link
Member

@svenklemm svenklemm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do the counts in the EXPLAIN output change due to this?

@akuzm
Copy link
Member Author

akuzm commented Nov 1, 2022

Why do the counts in the EXPLAIN output change due to this?

There was one null value in the device_id column on which the subquery is filtered, so now for this rows all chunks are excluded.

@akuzm akuzm added this to the TimescaleDB 2.8.2 milestone Nov 10, 2022
@akuzm akuzm force-pushed the const-null branch 5 times, most recently from 403e777 to f3a6234 Compare November 15, 2022 15:18
@akuzm akuzm enabled auto-merge (rebase) November 15, 2022 16:11
The code we inherited from postgres expects that if we have a const null
or false clause, it's going to be the single one, but that's not true
for runtime chunk exclusion because we don't try to fold such
restrictinfos after evaluating the mutable functions. Fix it to also
work for multiple restrictinfos.
@akuzm akuzm merged commit 7e4ebd1 into timescale:main Nov 15, 2022
@horzsolt horzsolt modified the milestones: TimescaleDB 2.8.2, TimescaleDB 2.9 Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants