Support materialized views in PostgreSQL dialect

[Erich Seifert]

At the moment, with SQLAlchemy 0.8.4, introspecting a materialized view in a PostgreSQL 9.3.2 database fails with:

  File "/tmp/test/meterializedview.py", line 16, in __init__
    self.MaterializedView = Table('mytable', MetaData(), autoload=True, autoload_with=self.db)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/schema.py", line 332, in __new__
    table._init(name, metadata, *args, **kw)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/schema.py", line 396, in _init
    self._autoload(metadata, autoload_with, include_columns)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/schema.py", line 413, in _autoload
    self, include_columns, exclude_columns
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1595, in run_callable
    return conn.run_callable(callable_, *args, **kwargs)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1118, in run_callable
    return callable_(self, *args, **kwargs)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/default.py", line 262, in reflecttable
    return insp.reflecttable(table, include_columns, exclude_columns)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/reflection.py", line 414, in reflecttable
    for col_d in self.get_columns(table_name, schema, **tblkw):
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/reflection.py", line 254, in get_columns
    **kw)
  File "<string>", line 1, in <lambda>
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/reflection.py", line 49, in cache
    ret = fn(self, con, *args, **kw)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/dialects/postgresql/base.py", line 1678, in get_columns
    info_cache=kw.get('info_cache'))
  File "<string>", line 1, in <lambda>
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/reflection.py", line 49, in cache
    ret = fn(self, con, *args, **kw)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/dialects/postgresql/base.py", line 1593, in get_table_oid
    raise exc.NoSuchTableError(table_name)




{{{#!sql
SELECT c.oid
FROM pg_catalog.pg_class c
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
WHERE (%s)
AND c.relname = :table_name AND c.relkind in ('r','v')

However, a new {{{relkind}}} value {{{m}}} was introduced for materialized views, which isn't handled at the moment.

To (partly) solve the problem for me, i just added {{{m}}} to the where clause:

AND c.relname = :table_name AND c.relkind in ('r','v','m')

There are several other cases where {{{relkind}}} is used ({{{get_table_names}}}, {{{get_view_names}}}, {{{get_indexes}}}, etc.) where it isn't clear how to deal with materialized view because they are a hybrid of tables and views and can have their own indexes and table spaces.

As far as I can see, the same issue exists in SQLAlchemy 0.9 (browser:lib/sqlalchemy/dialects/postgresql/base.py, rev 164bff0).

[Mike Bayer]

OK so we think adding 'm' is all that's needed? can you either attach some short test case (e.g. including a PG MATERIALIZED VIEW create), or a pullrequest with such? this is a one liner code change but we'd need a test in test/dialects/postgresql/test_reflection.py for it.

[Mike Bayer]

additionally, as this seems to be new in pg 9.3, I should add a new @requirements rule that checks for PG version 9.3, not to mention need to upgrade PG versions locally and on jenkins...

[Rodrigo Ignacio Gamboa García]

It's would be nice adding support for autoreflect Foreign Tables, filtering relkind with string 'f'. Thanks.

[Mike Bayer]

OK adding all these types makes me a little nervous. I looked at "foreign tables" and it has a server name also, this is something special.

Since we have support for special table args with reflection I think we should take advantage of these:

Table('x', m, autoload=True, postgresql_relkind='materialized')

Table('x', m, autoload=True, postgresql_relkind='foreign')

'postgresql_relkind' would accept the 'M', 'F' values as well as descriptive words.

[Rodrigo Ignacio Gamboa García]

That's a nice solution. Thanks.

[Rodrigo Menezes]

Hi - I'm currently working on this patch. Stuck on tests in two different places:

1) We don't exactly have a framework for testing foreign tables. Foreign tables require two different DBs being used at the same time during tests. Would it be simpler to:

• Test foreign tables to another PSQL db. Require that developers make another Postgres db (possibly called test2 that user 'scott' has access to) to run tests

• Test foreign table to a another kind of db. Require that these tests are run with both MySQL and PSQL installed.

• Test all combinations of foreign tables from Postgres to SQLAlchemy's different support databases. This may not necessary because Postgres tests the actual functionality of its foreign tables and we're testing that we can access the view it creates. If it works from Postgres to one DB, it probably already works from Postgres to another type of DB.

The second option seems the simplest.

If we are doing the second option, do I have to test both Postgres/MySQL AND Postgres/MSSQL? Should the test run on either one (whichever one it detects first?). It would be simpler, I think, just to test Postgres to MySQL.

How do I make sure that the test runner will require both DBs? Any advice on what the syntax should be? __requires_other_db__ = 'x'?

2) Testing reflection is one thing - what about testing queries on these new relations? Should all our query tests run on these different kinds of relations? If so, how would I make our InsertTests skip mviews and whatever other tests that can't run on these relations? I imagine this would be the right direction to go in.

If it wouldn't be wise to use our existing query tests, do I make separate testing files test/dialect/postgresql/test_mview.py and test/dialect/postgresql/test_foreign_tables.py? How far should I go in replicating the tests from test_query.py?

Could use a little insight - never contributed before. Thanks!

[Rodrigo Menezes]

Also - it seems that foreign tables only need a server name when being created... Unless we're supporting the creation of foreign tables out of the box, it doesn't seem server_name would be a parameter anywhere in SQLAlchemy.

I suppose I should have asked this before I starting dev'ing on it - what are the benefits of having the postgresql_relkind='x' syntax then? We're probably going to treat all these tables/views the same. Wouldn't it be simpler to just extend "c.relkind in ('r','v')"? Theoretically, your mviews shouldn't have the same names as your tables, right?

[Mike Bayer]

OK, so:

1. testing. we will follow a convention similar to that set up for Oracle testing DBLINK tables (pretty much the same thing). Right now there is a name in setup.cfg "oracle_db_link = test_link". We will add another value for Postgresql foreign tables: postgresql_foreign_database = <name>. Within setup.cfg, it should probably be commented out; adding a second file "test.cfg" to a build environment allows values to be overridden here. When this name is available, it is assumed that the user has issued CREATE SERVER on the target Postgresql database with this name. So it theoretically shouldn't matter much what target backend they used. When I run this test I'll likely make it the same PG server. We then add a rule in test/requirements.py "postgresql_foreign_database", that is written just like "def oracle_test_dblink()" in that same file. The test class itself will include __requires__ = 'postgresql_foreign_database'. The test will emit "CREATE FOREIGN TABLE", which at first can be by a straight execute("CREATE FOREIGN TABLE ..."). If we add support for CREATE FOREIGN TABLE directly we can use that instead (and actually we should because we will want to include the same parameter within reflection, however this should be a separate pull request).

2. testing queries. The table here exists just like any other in the same namespace, there are no changes in syntax or in the behavior of the DBAPI, so assuming those are true then we don't need to test queries here. If such changes are needed, then we'd need to add tests. But if there's no extra code added to support querying such features then there's nothing really to test. The test suite that includes reflection can do perhaps a simple round-trip INSERT/SELECT test of a few rows for sanity.

3. where to put tests. The reflection of the views/foreign tables would be in test/dialect/postgresql/test_reflection.py. If we later want to add support for CREATE FOREIGN TABLE, we would add string-compilation tests for this syntax to test/dialect/postgresql/test_compiler.py. The test_reflection test can also use the Table() construct for creation of the target table which will serve as an unofficial integration test.

4. extending "c.relkind in ('r', 'v')", since these names aren't allowed to overlap anyway. Sure, but we then target this at 1.0. I'd prefer that.

5. representing the type here. When we get at "relkind" here in our reflection query, we should be storing this in the reflection. So for Postgresql, we should be adding a method that is supported by the system called "get_table_options()". The MySQL dialect has this here: https://bitbucket.org/zzzeek/sqlalchemy/src/2c9ff9115c72f25cfcc500cd9f42e1035e54e582/lib/sqlalchemy/dialects/mysql/base.py?at=master#cl-2481. this would return a dictionary: {"postgresql_relkind": "r"} or whatever it is for the target table. To support this being present, we also add "postgresql_relkind" to the PGDialect.construct_arguments dictionary.

6. We can support CREATE FOREIGN TABLE using yet another argument, "postgresql_foreign_server", which takes the string server name. When present, the PG dialect can get at this by adding prefixes and "post" instructions using the post_create_table() method and a new "get_table_prefixes()" method, add this to compiler.py like this:

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index e45510a..8d9198e 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -2362,6 +2362,7 @@ class DDLCompiler(Compiled):

         text = "\n" + " ".join(['CREATE'] +
                                table._prefixes +
+                               self.get_table_prefixes(table),
                                ['TABLE',
                                 preparer.format_table(table),
                                 "("])
@@ -2532,6 +2533,9 @@ class DDLCompiler(Compiled):
             colspec += " NOT NULL"
         return colspec

+    def get_table_prefixes(self, table):
+        return ''
+
     def post_create_table(self, table):
         return ''

we would add "get_table_prefixes()" and "post_create_table()" to PGDDLCompiler.

So a bunch of steps but it is mostly just turning the crank, thanks very much for volunteering to contribute! If you send PRs to github I can have them automatically tested on our jenkins server.

[Rodrigo Menezes]

Thanks for the quick response! Lemme make sure I'm interpreting what you said at 4) correctly. I should make two pull requests:

1) Targeted for 0.9.xx

Extending c.relkind from "('r', 'v')" to "('r','v','m','f')". Include reflection tests for both mviews and foreign tables. Add postgresql_foreign_database to setup.cfg / requirements.py for testing foreign tables. Add a few round-trip INSERT/SELECT tests for sanity.

2) Targeted for 1.0.xx

Support CREATE FOREIGN TABLE. Add postgres_relkind and the postgres_foreign_server options to the Table() construct. Add "get_table_options()" to refer to these params as the MySQL dialect does. Add string-compilation tests for this syntax to test/dialect/postgresql/test_compiler.py.

Is this correct?

[Mike Bayer]

I was proposing that everything is against 1.0. In the 0.9 series we're at 0.9.8 now, it's kind of late for new features that have the slightest change in existing behavior.

[Mike Bayer]

otherwise everything checks out! we can keep amending the pull request til it's right.

[Rodrigo Menezes]

Okay, sounds good!

[Mike Bayer]

Just a note, we're doing just reflection for now. the table_options stuff and the CREATE TABLE features I don't think are a priority for now.

[Mike Bayer]

this is all up as of 3a6cd72beaeed49b49619e49

[Mike Bayer]

Issue #3205 was marked as a duplicate of this issue.