console: support materialized views #4204

nicuveo · 2020-03-26T17:26:44Z

Description

This is the frontend part of #91. WARNING: it is an ugly workaround!

The problem is as follows: this query used information_schema to retrieve information about the underlying postgres database. Unfortunately, neither information_schema.tables nor information_schema.columns contain data about materialized views. What this PR does is to change the query to instead rely on postgres internals, such as pg_class and pg_attribute. It has a few upsides; notably, retrieving comments is much easier now, since we have access to the oid of each element, and we can just use col_description and obj_description.

However, the previous version of that query returned every single column of the columns view, unmodified. To emulate that behaviour, I had to recreate the entire columns view in the query, while fixing it so that it includes materialized views.

I obtained the definition of columns by running psql -E and running \dS+ information_schema.columns. The only change is as follows:

before: c.relkind = ANY (ARRAY['r'::"char", 'v'::"char", 'f'::"char", 'p'::"char"])
after: c.relkind = ANY (ARRAY['r'::"char", 'v'::"char", 'm'::"char"])

Affected components

Console

Limitations, known bugs & workarounds

There are several things that are missing from this PR, in its current state:

currently, the view_info part of the query is computed using the information_schema.views view, which... suffers from the exact same problem: not listing materialized views. I am not sure whether we actually want to treat them as views as far as the console is concerned, but that's something to check,
I have NOT checked that the console properly supports this new MATERIALIZED VIEW table type everywhere; from my limited testing, it treats is as a view (the name is italicized for instance),
I have NOT done regression testing beyond some very simple examples on my test database,
I have NOT written new tests, nor done extensive tests in the console; I have simply tested, on one materialized view:
- tracking it
- untracking it
- running a query against it in GraphiQL
- checking that the data is correct (both rows and columns) in the Data tab

Ideally, this would be rewritten to call a backend function, and none of the postgres internals would be in the frontend. But in recent discussions we have agreed that it was preferable to do this workaround before doing the necessary server work.

nicuveo · 2020-03-27T13:10:24Z

An update on this:

I had used the definition of information_schema.columns from postgres 12, which is not backwards compatible. That's why the tests were failing on CircleCI: our tests use postgres 10.

I pushed a new commit that rewrites the query, based on how it was defined in posgres 9.5, which is the oldest version we support. I stripped some of the fields that either had no information (as in: they were declared as cast NULL as ...) or that relied on internal functions that no longer exist in version 12.

Importantly, this is still a WORK IN PROGRESS. I am currently running all cypress tests on versions 12 and 10, and will perform some additional manual ones / will add cypress tests for materialized views.

hasura-bot · 2020-03-31T00:44:46Z

Review app for commit acb8467 deployed to Heroku: https://hge-ci-pull-4204.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4204-acb8467c

Instead of sticking to the information_schema tables, this now uses pg internals. This is not a regression, as the query was already relying on some of them. Furthermore, this heavily simplifies the comment subqueries, since we have access to the oid.

This is horribly verbose; but it does the job. The giant expression of `isc` is almost exactly the definition of the `information_schema.columns` view, with one major restriction: it also list columns present in materialized views. This is very ugly and could be simplified heavily. For technical details: I obtained the details of the view by running `psql -E` and using `\dS+ information_schema.columns`. The only difference is this: before: `c.relkind = ANY (ARRAY['r'::"char", 'v'::"char", 'f'::"char", 'p'::"char"])` after: `c.relkind = ANY (ARRAY['r'::"char", 'v'::"char", 'm'::"char"])`

To be more precise: I have used the definition of `information_schema.columns` as it was in [version 9.5](https://github.com/postgres/postgres/blob/REL9_5_STABLE/src/backend/catalog/information_schema.sql), which is the oldest version we support. I tested it on my local database, which is on version 12. To make sure it worked, I had to remove several functions call that did not exist anymore. The result is a trimmed-down version, that has only the most essential fields. I am going to run more tests to see whether this is enough or not.

hasura-bot · 2020-04-01T08:16:05Z

Review app for commit 7fb9bdb deployed to Heroku: https://hge-ci-pull-4204.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4204-7fb9bdb7

nicuveo · 2020-04-01T11:48:07Z

Sorry for the noise on this PR: I was blocked by what turned out to be #4250. I'll close this PR and open a new one when I have something a bit cleaner.

hasura-bot · 2020-04-01T11:48:10Z

Review app https://hge-ci-pull-4204.herokuapp.com is deleted

nicuveo requested a review from beerose as a code owner March 26, 2020 17:26