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 Isolation Test Failure in isolation_multiuser_locking in PG17 #7714

Draft
wants to merge 10 commits into
base: naisila/pg17_support
Choose a base branch
from

Conversation

m3hm3t
Copy link
Contributor

@m3hm3t m3hm3t commented Oct 22, 2024

This PR enhances isolation_multiuser_locking.spec test compatibility across multiple PostgreSQL versions by handling differences in error messages and behavior. Key updates include:

  • Error Message Handling: Adjustments to manage version-specific error messages, ensuring consistent test results.

  • Modified to address variations in locking behavior across PostgreSQL versions, ensuring test stability in multiuser scenarios.

  • REINDEX Behavior Adjustment: This PR accounts for a behavioral change introduced in PostgreSQL by commit ecb0fd337, which alters how REINDEX interacts with system catalogs.

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ecb0fd337

Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.61%. Comparing base (b29c332) to head (94bbb2e).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           naisila/pg17_support    #7714   +/-   ##
=====================================================
  Coverage                 89.61%   89.61%           
=====================================================
  Files                       274      274           
  Lines                     59689    59689           
  Branches                   7446     7446           
=====================================================
+ Hits                      53490    53493    +3     
+ Misses                     4069     4066    -3     
  Partials                   2130     2130           

@m3hm3t m3hm3t marked this pull request as ready for review October 22, 2024 10:42
@m3hm3t m3hm3t self-assigned this Oct 22, 2024

step s1-insert:
UPDATE test_table SET column2 = 1;
<waiting ...>
Copy link
Member

Choose a reason for hiding this comment

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

this seems a bit suspicious as we were throwing a permission-check error in the older test output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose that because of this commit:

git.postgresql.org Git - postgresql.git/commitdiff

If you think that it is unrelated, ı Will looked into that more.

naisila and others added 9 commits November 11, 2024 11:57
(cherry picked from commit ae3ed7d)
(cherry picked from commit 76f60a7)
(cherry picked from commit df9c7b4)
In PG17, the outer loop in acquire_sample_rows() changed
from
while (BlockSampler_HasMore(&bs))
to
while (table_scan_analyze_next_block(scan, stream))

Relevant PG commit:
041b96802efa33d2bc9456f2ad946976b92b5ae1
postgres/postgres@041b968

It is expected that the scan_analyze_next_block function will
check if there are any blocks left. So we add that check in
columnar_scan_analyze_next_block

(cherry picked from commit 7eb0ad5)
PG 17 added support for DEFAULT in ALTER TABLE .. SET ACCESS METHOD

Relevant PG commit:
d61a6cad6418f643a5773352038d0dfe5d3535b8
postgres/postgres@d61a6ca

In that case, name in AlterTableCmd would be null.
Add a null check here to avoid crash.

(cherry picked from commit 71b9974)
…tests

Fix pg15 pg16 multi_mx_create_table multi_schema_support

Relevant PG commit:
postgres/postgres@f696c0c
f696c0cd5f299f1b51e214efc55a22a782cc175d

(cherry picked from commit 17a2ed0)
Relevant PG commit:
f69319f2f1fb16eda4b535bcccec90dff3a6795e
postgres/postgres@f69319f

(cherry picked from commit 6c12b10)
This PR addresses a regression test failure in the multi-mx feature of
Citus with the new PostgreSQL 17 version. The regression was identified
during the execution of multi-node tests, specifically targeting
compatibility issues introduced with PostgreSQL 17.

---------

Co-authored-by: Mehmet YILMAZ <[email protected]>
(cherry picked from commit 70cf729)
This reverts commit e4040dd.

Reverting for now as this commit is fixing more than one thing
at once at multi_extension.out file

Its a harmless revert for testing purposes
@naisila naisila force-pushed the naisila/pg17_support branch from 04a8f6a to b29c332 Compare November 11, 2024 09:03
@m3hm3t m3hm3t force-pushed the m3hm3t/pg17_isolation_multiuser_lock branch from 1fbdd9f to 94bbb2e Compare November 11, 2024 10:32
@m3hm3t m3hm3t marked this pull request as draft November 11, 2024 13:13
@m3hm3t m3hm3t marked this pull request as ready for review November 14, 2024 19:49
@naisila naisila force-pushed the naisila/pg17_support branch 3 times, most recently from e12686a to 46dc966 Compare November 19, 2024 09:27
@m3hm3t m3hm3t marked this pull request as draft November 20, 2024 11:26
@naisila naisila force-pushed the naisila/pg17_support branch from 46dc966 to 6d036b0 Compare November 20, 2024 11:54
@naisila naisila force-pushed the naisila/pg17_support branch 6 times, most recently from e108bb8 to c396ce6 Compare November 22, 2024 13:27
@naisila naisila force-pushed the naisila/pg17_support branch 7 times, most recently from 1e55f40 to 1c39f95 Compare December 8, 2024 08:33
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.

4 participants