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

PostgreSQLSchemaManager::selectTableColumns is excluding columns that are not related to an extension #5781

Closed
allenisalai opened this issue Oct 20, 2022 · 1 comment

Comments

@allenisalai
Copy link

Bug Report

Q A
Dbal Version 3.4.5
Psql Version 14.2
Doctrine Migration Version 3.5.2

Summary

I am running into an issue with the pg_depend join here. 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.

Expected behaviour

Since my migration_versions table isn't managed by and extension, the columns should be returned by selectTableColumns.

I'll create a PR to test if changing the condition to use classid will solve the issue on my end.

allenisalai pushed a commit to allenisalai/dbal that referenced this issue Oct 24, 2022
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Nov 2, 2022
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Nov 2, 2022
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Nov 23, 2022
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Dec 21, 2022
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Dec 27, 2022
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Jan 16, 2023
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Feb 1, 2023
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Feb 1, 2023
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Feb 16, 2023
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
allenisalai pushed a commit to allenisalai/dbal that referenced this issue Feb 17, 2023
This fixes a bug where there was a pg_proc which was a dependency of
an extention and also had an identical oid to the table class being
described.  Oids are not guaranteed to be unique because they will
wrap around once they hit the unsigned integer max.  The added
conditional will ensure that the target object of the dependency is a
table.

Fixes: doctrine#5781
@github-actions
Copy link

github-actions bot commented Apr 2, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant