Skip to content

Add listMaterializedViews and getMaterializedViews SPI#8113

Merged
sopel39 merged 6 commits intotrinodb:masterfrom
raunaqmorarka:list-mvs
Jun 7, 2021
Merged

Add listMaterializedViews and getMaterializedViews SPI#8113
sopel39 merged 6 commits intotrinodb:masterfrom
raunaqmorarka:list-mvs

Conversation

@raunaqmorarka
Copy link
Copy Markdown
Member

Used listMaterializedViews to list MV names for information_schema.tables, system.jdbc.tables and SHOW TABLES
Used getMaterializedViews to list MV columns in information_schema.columns, system.jdbc.columns and SHOW COLUMNS

Fixed DESCRIBE, SHOW COLUMNS, information_schema.columns, system.jdbc.columns for MVs in Iceberg.

Table type used for MVs in information_schema.tables is left unchanged as BASE TABLE.
MVs are not listed in information_schema.views.
getMaterializedViews can be used in future to introduce information_schema.trino_materialized_views or system.metadata.materialized_views for showing detailed information about materialized view definitions.

@findepi
Copy link
Copy Markdown
Member

findepi commented May 28, 2021

@raunaqmorarka i am adding more tests, inclduding a bit of information_schema coverage for MVs in #8117
Can you please check how your PR interacts with the tests added there?

@raunaqmorarka
Copy link
Copy Markdown
Member Author

@raunaqmorarka i am adding more tests, inclduding a bit of information_schema coverage for MVs in #8117
Can you please check how your PR interacts with the tests added there?

Looked at it, since i've tested similar cases in this PR, I think it should result in the TODO cases of that PR getting solved.

@raunaqmorarka
Copy link
Copy Markdown
Member Author

@findepi I've rebased this to master and added changes to BaseConnectorTest#testMaterializedView

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to add test to TestInformationSchemaConnector or TestInformationSchemaMetadata?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is covered by BCT column listing tests

Copy link
Copy Markdown
Member

@sopel39 sopel39 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% small comments.

Should #8153 land first?

@sopel39
Copy link
Copy Markdown
Member

sopel39 commented Jun 3, 2021

please rebase on top of #8153

@raunaqmorarka
Copy link
Copy Markdown
Member Author

please rebase on top of #8153

done

Copy link
Copy Markdown
Member

@sopel39 sopel39 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm % minor comments

Fixed and added tests for DESCRIBE, SHOW COLUMNS,
information_schema.columns and system.jdbc.columns
@sopel39 sopel39 merged commit e4cae79 into trinodb:master Jun 7, 2021
@raunaqmorarka raunaqmorarka deleted the list-mvs branch June 7, 2021 15:11
{
Set<SchemaTableName> tables = listTables(session, metadata, accessControl, prefix);
Set<SchemaTableName> views = listViews(session, metadata, accessControl, prefix);
// TODO (https://github.com/trinodb/trino/issues/8207) define a type for materialized views
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should be separate commit

@sopel39 sopel39 mentioned this pull request Jun 7, 2021
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants