PostgreSQL listTableColumns() not returning view columns since version 3.4.0

[amcsi]

Bug Report

Q	A
Version	3.4.0

Summary

In DBAL version 3.3.* when you did \Doctrine\DBAL\Schema\PostgreSQLSchemaManager::listTableColumns() on a view table, it would list the columns fine.

Current behaviour

But since version 3.4.0 that would return zero columns (for view tables).

How to reproduce

1. Create a normal table in PostgreSQL.
2. Create a view table that reads from that normal table.
3. Perform \Doctrine\DBAL\Schema\PostgreSQLSchemaManager::listTableColumns() on that view table.

Expected behaviour

All the columns of the view table should be listed.

Additional details

I believe I know what SQL bit is causing the breakage.

Here is what I believe the offending commit is that caused the breakage:
194c6eb

Below are the SQL scripts compared using dw.vw_time_tracking as the example view table. If you compare them and rearrange them a bit, you'll see that the bad SQL query in 3.4.0 includes c.relkind = 'r' as a new filter which filters out view table columns; removing that filter makes the query return all the columns as expected like in 3.3.*.

This breakage was not mentioned in the merged PR with the breaking change commit (#5268), and nothing was mentioned about this breaking SQL change nor anything about views.

I also see no separate method to list columns of views, so I believe I am calling what should be the right method to list the view table's columns.

Also note that although version 3.5.0 rearranged \Doctrine\DBAL\Schema\PostgreSQLSchemaManager a bit, the offending c.relkind = 'r' filter is still there and is still causing the same breakage.

So here are the good and bad SQL columns:

-- 3.3.* good
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
FROM pg_attribute a,
     pg_class c,
     pg_type t,
     pg_namespace n
WHERE n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')
  AND c.relname = 'vw_time_tracking'
  AND n.nspname = 'dw'
  AND a.attnum > 0
  AND a.attrelid = c.oid
  AND a.atttypid = t.oid
  AND n.oid = c.relnamespace
ORDER BY a.attnum;

-- 3.4.0 bad
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
FROM pg_attribute a,
     pg_class c,
     pg_type t,
     pg_namespace n
WHERE a.attnum > 0
  AND a.attrelid = c.oid
  AND a.atttypid = t.oid
  AND n.oid = c.relnamespace
  AND c.relkind = 'r'
  AND c.relname = 'vw_time_tracking'
  AND n.nspname = 'dw'
ORDER BY a.attnum;

SQL queries rearranged and compared with a diff:

[amcsi]

@morozov @mondrake

[zKoz210]

@greg0ire @morozov @mondrake @SenseException @derrabus same problem Soyhuce/next-ide-helper#82

can anyone tell me when this will be fixed? I can't work correctly with materialized views

on ~3.3.8 everything works correctly

[derrabus]

@greg0ire @morozov @mondrake @SenseException

Pinging random people how have contributed to this repository because you demand your issue to be fixed is not nice btw. Also, I'm a little offended because you forgot to include me.

/edit: Oh nice, you've edited me in later. 😎

can anyone tell me when this will be fixed?

The short answer is no. And tbh, this question is a little rude as well. This is an open source project. Anyone can contribute and a change happens if someone makes it happen.

I can't work correctly with materialized views

And DBAL was never meant to work on those. DBAL currently has a very limited and incomplete abstraction for views. Listing columns of a view might have worked to some extend by accident in the past, but it has never been an advertised feature.

If you have an idea how view introspection might be improved, please don't hesitate to work on a proposal.

[zKoz210]

@derrabus sorry for my tactlessness, thank you for such a voluminous work.

if it's not difficult for you, then please tell me why doListTableColumns() is used here. if you do parent::listTableColumns(), then displaying the columns starts working correctly.

https://github.com/doctrine/dbal/blob/3.6.x/src/Schema/PostgreSQLSchemaManager.php#L81

this SQL returns the correct columns in the view

https://github.com/doctrine/dbal/blob/3.6.x/src/Platforms/PostgreSQLPlatform.php#L448

[derrabus]

I don't know from the top of my head why this change has been made. But again, listTableColumns() was never meant to work on views and I don't think that it works on every other platform that supports views.

And even if we "fixed" this now, our functional tests don't cover this scenario, so we'll likely break this again.

My suggestion is to not rely on listTableColumns() for your scenario and either build this feature outside of DBAL the way you need it or contribute a portable column intropection for views to this repository, including functional tests.

[juanparati]

@derrabus : I also faced this issue after upgrade to doctrine 3.4. I am understand that "views" is not very standard across different platforms, some platforms doesn't supported at all and others have limitations, however the list the table columns it may be a good feature for platforms that supports views.

I am willing to implement the "getListView" methods if you think that may be a good idea.

[derrabus]

If you find a good and portable way to introspect view columns, sure, go ahead.

[juanparati]

I was checking the code for 3.6 and I found many deprecated methods that probably are not going to be used in the future 4.x versions. I also in order to differentiate the tables from the views I will have to create a bunch of specific methods only for the views, and I am sure that so massive changes are not going to be accepted, specially when Symphony Doctrine team is developing major version (4.0). In case of MySQL/MariaDB the views works in very similar way so introspect the columns is easy, however the ^3.4 versions force to only to retrieve columns for "BASE TABLE".

I decided to still use the 3.3 versions and wait to future releases of 4.x. I hope that Symphony Doctrine team can bring back the capacity of inspect views or at least to provide an abstract way of do it.

@amcsi: A possible workaround it may be to create a custom schema manager that extends the "PostgreSQLSchemaManager", then you can override the "selectTableColumns". Please check how to override the schema.

[derrabus]

I hope that Symphony team can bring back the capacity of inspect views or at least to provide an abstract way of do it.

The Symfony team is not doing anything here. This is a Doctrine project.

[juanparati]

@derrabus : I am sorry for the mistake. I always thought that Doctrine project was part of Symphony. It's amazing to see how many projects use so much libraries and code that for many developers it look part of same project.

[DanieliMi]

Hi, I am coming from MySQL/MariaDB and we're having the same issue.

For mysql this is the issue:

Index: vendor/doctrine/dbal/src/Schema/MySQLSchemaManager.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vendor/doctrine/dbal/src/Schema/MySQLSchemaManager.php b/vendor/doctrine/dbal/src/Schema/MySQLSchemaManager.php
--- a/vendor/doctrine/dbal/src/Schema/MySQLSchemaManager.php
+++ b/vendor/doctrine/dbal/src/Schema/MySQLSchemaManager.php	(date 1694683490783)
@@ -429,7 +429,7 @@
         // The schema name is passed multiple times as a literal in the WHERE clause instead of using a JOIN condition
         // in order to avoid performance issues on MySQL older than 8.0 and the corresponding MariaDB versions
         // caused by https://bugs.mysql.com/bug.php?id=81347
-        $conditions = ['c.TABLE_SCHEMA = ?', 't.TABLE_SCHEMA = ?', "t.TABLE_TYPE = 'BASE TABLE'"];
+        $conditions = ['c.TABLE_SCHEMA = ?', 't.TABLE_SCHEMA = ?'];
         $params     = [$databaseName, $databaseName];

         if ($tableName !== null) {

Unfortunately I am not familiar with dbal and wonder how a good solution might be implemented. Would adding a supportViews() method in \Doctrine\DBAL\Schema\AbstractSchemaManager be a viable option?

The idea is that this method would set an supportViews property which is then used in order to add or remove the conflicting parts in the SQL. From what I gathered that should be possible for PostgreSQL too.

If you have an idea how view introspection might be improved, please don't hesitate to work on a proposal.

@derrabus what does a view introspection have to include? Or would it be enough to implement something like listViewColumns (it would be to fix this issue from my point of view)?

[derrabus]

Unfortunately I am not familiar with dbal and wonder how a good solution might be implemented. Would adding a supportViews() method in \Doctrine\DBAL\Schema\AbstractSchemaManager be a viable option?

The idea is that this method would set an supportViews property which is then used in order to add or remove the conflicting parts in the SQL. From what I gathered that should be possible for PostgreSQL too.

So, the schema manager would behave differently depending on whether that property has been set or not? I doubt that this is something we want because that would affect internal calls as well.

If you have an idea how view introspection might be improved, please don't hesitate to work on a proposal.

@derrabus what does a view introspection have to include? Or would it be enough to implement something like listViewColumns (it would be to fix this issue from my point of view)?

Adding such a method sounds reasonable. Give it a try if you like.