Postgres: support materialized views #214
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- only use pg_catalog tables and functions (faster, cleaner) - fetch comments in-line with tables and columns - use the oid to get related info (columns, indexes, etc.) - use format_type() to produce the correct type name. This means arrays now show up as their contained type with `[]`, and custom domains are reflected correctly. This is groundwork laid to enable other relationship types (e.g. MATERIALIZED VIEW)
|
@mjpieters THANK YOU FOR YOUR GREAT COMMIT !!! 🚀 🚀 🚀 |
k1LoW
approved these changes
Apr 25, 2020
| WHEN cls.relkind = 'f' THEN 'FOREIGN TABLE' | ||
| END AS table_type, | ||
| ns.nspname AS table_schema, | ||
| descr.description AS table_comment |
| @@ -146,12 +130,12 @@ AND table_schema = $3; | |||
| if err != nil { | |||
| return errors.WithStack(err) | |||
| } | |||
| table.Def = fmt.Sprintf("CREATE VIEW %s AS (\n%s\n)", tableName, strings.TrimRight(tableDef.String, ";")) | |||
| table.Def = fmt.Sprintf("CREATE %s %s AS (\n%s\n)", tableType, tableName, strings.TrimRight(tableDef.String, ";")) | |||
| pg_get_expr(def.adbin, def.adrelid) AS column_default, | ||
| NOT (attr.attnotnull OR tp.typtype = 'd' AND tp.typnotnull) AS is_nullable, | ||
| CASE | ||
| WHEN attr.atttypid::regtype = ANY(ARRAY['character varying'::regtype, 'character varying[]'::regtype]) THEN |
There was a problem hiding this comment.
Yeah, I’m not that happy with this part, it’ll fail for a multi-dimensional array. Looking over pg_type again this should really be a test for
attr.atttypid::regtype = 'character varying'::regtype
OR (tp.typarray != 0 AND tp.typarray::regtype = 'character varying'::regtype)Or, just drop the whole notion of transforming character varying to varchar altogether ;-)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is my proposed implementation for #212
I first refactored the way the postgres driver queries for table information. The original code relied mostly on
information_schema(the SQL-92 standard system catalog) but added in joins against the Postgres-specificpg_catalogtables, my version uses just thepg_catalogsystem catalog. There isn't really much point in usinginformation_schemaunless you are trying to write standards-compliant SQL that is easily portable to other databases. Materialized view information is largely absent from theinfromation_schemacatalog and so not refactoring would have lead to quite convoluted joins or union queries.This is mostly resulting in the exact same output, except for array types and domains. The
information_schemaview on attribute types is quite limited and make it quite hard to get proper type info out, but this is not the case when using thepg_attributeandpg_typetables in conjunction with theformat_type()function.For arrays, rather than
arrayyou now get the contained type and dimensions, sotext[](array of strings),double precision[][](array of arrays of floating point values), etc. For domains (which are 'reusable constraints', attaching the constraint to a type rather than to a column or table), instead of the constrained type, you get the domain name. IIRC, for certain variable-length types you'll now get the table-specific length as well where that wasn't reflected before.I've retained the preference for
varchar(...)overcharacter varying(...), however, applying that change at the query level.Further changes in this refactor:
tableOideverywhere to fetch information about the table, rather than the schema and table name strings.I've run the full test suite with this, as well as manually tested this agains AWS Redshift (see separate issue I found that's independent of this pull request, #213).