Make columnarAM dependent on rel objects in columnar schema

[onurctirtir]

DESCRIPTION: Fixes a bug that could break pg upgrades due to missing pg_depend records for columnar table access method

Fixes #5437.

• First commit enables storing pg_upgrade logs for new data dir in case of failure.
  (Reverting third commit would reveal how it looks like)

• Second commit adds a test that reproduces the bug on our regression test suite.
  The only scenario that I could come up with was renaming the schema (public schema in our case) to something else, then creating the columnar table there.
  (Reverting third commit would make this branch reproduce the bug on CI)

• Third commit is the commit that fixes the bug:

During pg upgrades, we have seen that it is not guaranteed that a
columnar table will be created after metadata objects got created.
Prior to changes done in this commit, we had such a dependency
relationship in pg_depend:

columnar_table ----> columnarAM ----> citus extension
                                           ^  ^
                                           |  |
columnar.storage_id_seq --------------------  |
                                              |
columnar.stripe -------------------------------

Since pg_upgrade just knows to follow topological sort of the objects
when creating database dump, above dependency graph doesn't imply that
columnar_table should be created before metadata objects such as
columnar.storage_id_seq and columnar.stripe are created.

For this reason, with this commit we add new records to pg_depend to
make columnarAM depending on all rel objects living in columnar
schema. That way, pg_upgrade will know it needs to create those before
creating columnarAM, and similarly, before creating any tables using
columnarAM.

Note that in addition to inserting those records via installation script, we also
do the same in citus_finish_pg_upgrade(). This is because, pg_upgrade
rebuilds catalog tables in the new cluster and that means, we must insert
them in the new cluster too.

---

Given that we decided not to backport this fix to older versions, here is the workaround for 10.0 & 10.1:
#5456 (comment).

[codecov]

Codecov Report

Merging #5456 (debca26) into master (8c0bc94) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head debca26 differs from pull request most recent head 2026166. Consider uploading reports for the commit 2026166 to get more accurate results

@@            Coverage Diff             @@
##           master    #5456      +/-   ##
==========================================
- Coverage   92.74%   92.74%   -0.01%     
==========================================
  Files         215      215              
  Lines       45321    45321              
==========================================
- Hits        42035    42031       -4     
- Misses       3286     3290       +4     

[onurctirtir]

I wonder what @SaitTalhaNisanci would say for the first and second commits.
For the third one, it would be very nice if @thanodnl can review.

[thanodnl]

Code looks good to me.

As commented on a thread, I would like to see one change.

Even though today having duplicate entries in pg_depend seems to not be an issue, I don't think postgres has codepaths where a duplicate entry can exists.

Lets treat entries in pg_depend as unique, and prevent them to be added twice.
This could either be done with an INSERT ON CONFLICT DO NOTHING assuming this works with the INSERT INTO SELECT clause.

Otherwise, lets filter the selected rows by what is already present in pg_depend and insert only the missing ones.

It seems that pg15 might get support for SQL Merge: https://www.postgresql.org/message-id/flat/20210108192200.GA25633%40alvherre.pgsql#a8636c22f696aa519c1cdbb824b70e51 which is a bit far out for this patch.

[onurctirtir]

Code looks good to me.

As commented on a thread, I would like to see one change.

Even though today having duplicate entries in pg_depend seems to not be an issue, I don't think postgres has codepaths where a duplicate entry can exists.

Lets treat entries in pg_depend as unique, and prevent them to be added twice. This could either be done with an INSERT ON CONFLICT DO NOTHING assuming this works with the INSERT INTO SELECT clause.

Otherwise, lets filter the selected rows by what is already present in pg_depend and insert only the missing ones.

It seems that pg15 might get support for SQL Merge: https://www.postgresql.org/message-id/flat/20210108192200.GA25633%40alvherre.pgsql#a8636c22f696aa519c1cdbb824b70e51 which is a bit far out for this patch.

I guess ON CONFLICT requires having a unique/exclusion constraint that covers the columns that we are interested in :
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification

Even more, catalog tables don't seem to support ON CONFLICT anyway:
ERROR: ON CONFLICT is not supported with system catalog tables

So I will check if the entry already exists using a CTE etc.

[onurctirtir]

Given that we will only backport this patch to 10.2, below is the command that needs to be run on older versions (10.0 & 10.1) to manually fix this issue there. Note that the only difference is the commented line for stripe_first_row_number_idx.
Also note that below command should be run on all nodes, before calling citus_prepare_pg_upgrade():

INSERT INTO pg_depend
SELECT -- Define a dependency edge from "columnar table access method" ..
        'pg_am'::regclass::oid as classid,
        (select oid from pg_am where amname = 'columnar') as objid,
        0 as objsubid,
        -- ... to each object that is registered to pg_class and that lives
        -- in "columnar" schema. That contains catalog tables, indexes
        -- created on them and the sequences created in "columnar" schema.
        --
        -- Given the possibility of user might have created their own objects
        -- in columnar schema, we explicitly specify list of objects that we
        -- are interested in.
        'pg_class'::regclass::oid as refclassid,
        columnar_schema_members.relname::regclass::oid as refobjid,
        0 as refobjsubid,
        'n' as deptype
FROM (VALUES ('columnar.chunk'),
            ('columnar.chunk_group'),
            ('columnar.chunk_group_pkey'),
            ('columnar.chunk_pkey'),
            ('columnar.options'),
            ('columnar.options_pkey'),
            ('columnar.storageid_seq'),
            ('columnar.stripe'),
            -- ('columnar.stripe_first_row_number_idx'), -- ignore for 10.0 & 10.1, otherwise, would throw an error
            ('columnar.stripe_pkey')
     ) columnar_schema_members(relname)
-- Avoid inserting duplicate entries into pg_depend.
EXCEPT TABLE pg_depend;