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

PG17 compatibility (#7653): Fix test diffs in columnar schedule #7768

Open
wants to merge 1 commit into
base: naisila/pg17_support
Choose a base branch
from

Conversation

colm-mchugh
Copy link
Contributor

@colm-mchugh colm-mchugh commented Nov 25, 2024

This PR fixes diffs in columnnar_chunk_filtering and columnar_paths tests.

In columnnar_chunk_filtering an expression (NOT (SubPlan 1)) changed to (NOT (ANY (a = (SubPlan 1).col1))). This is due to aPG17 commit that improved how scalar subqueries (InitPlans) and ANY subqueries (SubPlans) are EXPLAINed in expressions. The fix uses a helper function which converts the PG17 format to the pre-PG17 format. It is done this way because pre-PG17 EXPLAIN does not provide enough context to convert to the PG17 format. The helper function can (and should) be retired when 17 becomes the minimum supported PG.

In columnar_paths, a merge join changed to a hash join. This is due to this PG17 commit, which improved the PG optimizer's ability to estimate the size of a CTE scan. The impacted query involves a CTE scan with a point predicate (a=123) and before the change the CTE size was estimated to be 5000, but with the change it is correctly (given the data in the table) estimated to be 1, making hash join a more attractive join method. The fix is to have an alternative goldfile for pre-PG17. I tried, but was unable, to force a specific kind of join method using the GUCs (enable_nestloop, enable_hashjoin, enable_mergejoin), but it was not possible to obtain a consistent plan across all supported PG versions (in some cases the join inputs switched sides).

Add an alternative file for columnar_paths. The diff is because of PG17 commit:
postgres/postgres@f7816ae
which enabled the planner to use statistics to estimate the cardinality
of CTE scans.

Add a helper function for columnnar_chunk_filtering. The diff is due to:
postgres/postgres@fd0398fc
which changed how subquery outputs appear in expressions.
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.61%. Comparing base (c396ce6) to head (15b3bf0).

Additional details and impacted files
@@                   Coverage Diff                    @@
##           naisila/pg17_support    #7768      +/-   ##
========================================================
- Coverage                 89.62%   89.61%   -0.01%     
========================================================
  Files                       274      274              
  Lines                     59689    59689              
  Branches                   7446     7446              
========================================================
- Hits                      53495    53490       -5     
- Misses                     4065     4069       +4     
- Partials                   2129     2130       +1     
---- 🚨 Try these New Features:

@colm-mchugh colm-mchugh changed the title PG17 regress sanity (#7653): Fix test diffs in columnar schedule PG17 compatibility (#7653): Fix test diffs in columnar schedule Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant