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

Remove hypertable parent from query plans #1007

Merged
merged 1 commit into from
Feb 21, 2019

Conversation

svenklemm
Copy link
Member

@svenklemm svenklemm commented Jan 22, 2019

Since the parent table of a hypertable cannot contain
any rows including it in the plan is not necessary. PostgreSQL might
also not have good statistics for the main table as autoanalyze is
triggered by inserts which never happen to the main table, leading to
worse plans.

This patch adds a new guc enable_constraint_exclusion to disable
planner constraint exclusion. Constraint exclusion will not work with
cursors and WHERE CURRENT OF so the new guc will allow you to disable
that feature.

This patch also fixes constraint exclusion for JOINs and sort
optimization for queries with chunks_in.

@cevian cevian added the cleanup label Jan 22, 2019
@mfreed mfreed added this to the 1.3.0 milestone Jan 30, 2019
-> Seq Scan on _hyper_1_1_chunk
(5 rows)
QUERY PLAN
-------------------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

This plan seems very odd. Why do we have a merge append for only one child

Copy link
Member Author

Choose a reason for hiding this comment

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

well that can happen cause postgres doesnt know before how many children it will end up with

@codecov
Copy link

codecov bot commented Feb 5, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@ba32dab). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1007   +/-   ##
=========================================
  Coverage          ?   90.88%           
=========================================
  Files             ?       89           
  Lines             ?    10258           
  Branches          ?        0           
=========================================
  Hits              ?     9323           
  Misses            ?      935           
  Partials          ?        0
Impacted Files Coverage Δ
src/plan_expand_hypertable.c 98.5% <100%> (ø)

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 ba32dab...31ba248. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 5, 2019

Codecov Report

Merging #1007 into master will decrease coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1007     +/-   ##
=========================================
- Coverage   91.87%   91.78%   -0.1%     
=========================================
  Files          91       91             
  Lines       10806    10793     -13     
=========================================
- Hits         9928     9906     -22     
- Misses        878      887      +9
Impacted Files Coverage Δ
src/constraint_aware_append.c 95.16% <ø> (-0.59%) ⬇️
src/planner.c 93.46% <100%> (+0.22%) ⬆️
src/guc.c 95% <100%> (+0.26%) ⬆️
src/plan_expand_hypertable.c 98.56% <100%> (-0.03%) ⬇️
src/loader/bgw_message_queue.c 88.11% <0%> (-3.5%) ⬇️
src/planner_import.c 91.72% <0%> (-2.76%) ⬇️

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 72f4692...3b79016. Read the comment docs.

test/expected/plan_ordered_append-11.out Outdated Show resolved Hide resolved
-> Seq Scan on t1
Filter: ((a % 2) = 0)
(3 rows)
QUERY PLAN
Copy link
Contributor

Choose a reason for hiding this comment

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

we probably do want to add some data to this table so that we can see the filter correctly applied. Right now these test no longer test anything.

-----------+---------+---------
(0 rows)

ERROR: cursor "current_check_cursor" is not a simply updatable scan of table "current_check"
Copy link
Contributor

Choose a reason for hiding this comment

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

These look bad. Why the change?

test/expected/sql_query_results_optimized.out Outdated Show resolved Hide resolved
test/expected/sql_query_results_optimized.out Outdated Show resolved Hide resolved
test/expected/sql_query_results_optimized.out Outdated Show resolved Hide resolved
test/expected/sql_query_results_optimized.out Outdated Show resolved Hide resolved
test/expected/sql_query_results_optimized.out Show resolved Hide resolved
@svenklemm svenklemm force-pushed the no_parent branch 19 times, most recently from 39e17bb to f453c63 Compare February 20, 2019 23:14
@svenklemm svenklemm force-pushed the no_parent branch 2 times, most recently from 92c073b to cc87060 Compare February 21, 2019 16:34
Since the parent table of a hypertable cannot contain
any rows including it in the plan is not necessary. PostgreSQL might
also not have good statistics for the main table as autoanalyze is
triggered by inserts which never happen to the main table, leading to
worse plans.

This patch adds a new guc enable_constraint_exclusion to disable
planner constraint exclusion. Constraint exclusion will not work with
cursors and WHERE CURRENT OF so the new guc will allow you to disable
that feature.

This patch also fixes constraint exclusion for JOINs and sort
optimization for queries with chunks_in.
@svenklemm svenklemm merged commit 207391b into timescale:master Feb 21, 2019
@svenklemm svenklemm changed the title Remove parent oid from find_children_oids result Remove hypertable parent from query plans Feb 23, 2019
@svenklemm svenklemm deleted the no_parent branch October 10, 2019 13:51
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.

3 participants