Do not list tables required by extensions

[morozov]

Fixes #5668.

The implementation prior to 3.4.0 did not list the spatial_ref_sys table due to this hack:

dbal/src/Platforms/PostgreSQLPlatform.php

Line 279 in f873a82

AND table_name != 'spatial_ref_sys'

As #5668 (comment) shows, this hack is insufficient as it doesn't cover all tables required by any possible extension.

The patch fixes the query in selectTableColumns() but doesn't fix getListTablesSQL() since it's not a regression. We can fix it in a separate patch.

In order to add a new LEFT JOIN, the query needs to be reworked from using implicit joins (SELECT * FROM a, b, c) to using explicit joins.

We should consider adding a build against something like postgis/postgis on CI in order to make this change integration-tested.

[allenisalai]

Hi @morozov

I am running into an issue with the pg_depend join in this MR. It appears that the ON condition maybe be incorrect. Based on
https://www.postgresql.org/docs/15/catalog-pg-depend.html it seems like the ON condition should use the classid instead of objid.

LEFT JOIN pg_depend d
    ON d.classid = c.oid
    AND d.deptype = 'e'

In our system, the pg_depend record that matches the c.oid points to pg_proc which has the same oid as my migration_versions table but a different classid. Below are the queries I used to come to my conclusions.

Query 1 comes from src/Schema/PostgreSQLSchemaManager::selectTableColumns(), but I removed AND d.refobjid IS NULL so we could see what is returned and added some extra columns for debugging.

Query 1:

SELECT a.attnum,
       quote_ident(a.attname) AS field,
       t.typname AS type,
       format_type(a.atttypid, a.atttypmod) AS complete_type,
       (SELECT tc.collcollate FROM pg_catalog.pg_collation tc WHERE tc.oid = a.attcollation) AS collation,
       (SELECT t1.typname FROM pg_catalog.pg_type t1 WHERE t1.oid = t.typbasetype) AS domain_type,
       (SELECT format_type(t2.typbasetype, t2.typtypmod)
        FROM pg_catalog.pg_type t2
        WHERE t2.typtype = 'd'
          AND t2.oid = a.atttypid) AS domain_complete_type,
       a.attnotnull AS isnotnull,
       (SELECT 't'
        FROM pg_index
        WHERE c.oid = pg_index.indrelid
          AND pg_index.indkey[0] = a.attnum
          AND pg_index.indisprimary = 't') AS pri,
       (SELECT pg_get_expr(adbin, adrelid)
        FROM pg_attrdef
        WHERE c.oid = pg_attrdef.adrelid
          AND pg_attrdef.adnum = a.attnum) AS default,
       (SELECT pg_description.description
        FROM pg_description
        WHERE pg_description.objoid = c.oid
          AND a.attnum = pg_description.objsubid) AS comment,

          c.oid as table_oid,
          d.objid as depend_objid,
          d.classid as depend_classid

FROM pg_attribute a
         INNER JOIN pg_class c
                    ON c.oid = a.attrelid
         INNER JOIN pg_type t
                    ON t.oid = a.atttypid
         INNER JOIN pg_namespace n
                    ON n.oid = c.relnamespace
         LEFT JOIN pg_depend d
                   ON d.objid = c.oid
                       AND d.deptype = 'e'
WHERE a.attnum > 0
  AND c.relkind = 'r'
  AND c.relname = 'migration_versions'
  AND n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')
ORDER BY a.attnum;

Results of Query 1:

attnum |     field      |   type    |        complete_type        | collation | domain_type | domain_complete_type | isnotnull | pri | default | comment | table_oid | depend_objid | depend_classid
--------+----------------+-----------+-----------------------------+-----------+-------------+----------------------+-----------+-----+---------+---------+-----------+--------------+----------------
      1 | version        | varchar   | character varying(191)      |           |             |                      | t         | t   |         |         |     16500 |        16500 |           1255
      2 | executed_at    | timestamp | timestamp without time zone |           |             |                      | f         |     |         |         |     16500 |        16500 |           1255
      3 | execution_time | int4      | integer                     |           |             |                      | f         |     |         |         |     16500 |        16500 |           1255

Query 2 includes more information about the pg_class that relates to the pg_depend record.

SELECT c.oid table_class_id,
        c.relname rel_name,
        d.classid depend_class_id,
        d.objid depend_objid,
        depend_class.oid depend_table_class_id,
        depend_class.relname depend_rel_name
FROM pg_attribute a
         INNER JOIN pg_class c
                    ON c.oid = a.attrelid
         INNER JOIN pg_type t
                    ON t.oid = a.atttypid
         INNER JOIN pg_namespace n
                    ON n.oid = c.relnamespace
         LEFT JOIN pg_depend d
                   ON d.objid = c.oid
                       AND d.deptype = 'e'
         INNER JOIN pg_class depend_class
                   ON d.classid = depend_class.oid
WHERE a.attnum > 0
  AND c.relkind = 'r'
  AND c.relname = 'migration_versions'
  AND n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')
ORDER BY a.attnum;

Results of Query 2:

table_class_id |      rel_name      | depend_class_id | depend_objid | depend_table_class_id | depend_rel_name
----------------+--------------------+-----------------+--------------+-----------------------+-----------------
          16500 | migration_versions |            1255 |        16500 |                  1255 | pg_proc
          16500 | migration_versions |            1255 |        16500 |                  1255 | pg_proc
          16500 | migration_versions |            1255 |        16500 |                  1255 | pg_proc

It seems like my migration_versions table just happens to have the same oid as pg_proc which is causing records to return when they shouldn't. I'm definitely not a psql expert so there's a great chance I've just misunderstood something.

Thanks for your time.

[morozov]

@allenisalai could you file a new issue, please? I have very little experience with PostgreSQL as well, so it's quite likely that I made a mistake.

You can also file a pull request with the changes that work for you. You can omit a test for now since the issue may be inconsistently reproducible. I'll try to validate the patch using the documentation.

[allenisalai]

Done. #5781. I'll work on a PR on my end in the next couple days. Thanks.