Skip to content

Unified Materialized View Table Type in information_schema#15350

Merged
findepi merged 1 commit intotrinodb:masterfrom
jklamer:jklamer/MVInfoSchema
Jan 26, 2023
Merged

Unified Materialized View Table Type in information_schema#15350
findepi merged 1 commit intotrinodb:masterfrom
jklamer:jklamer/MVInfoSchema

Conversation

@jklamer
Copy link
Copy Markdown
Member

@jklamer jklamer commented Dec 8, 2022

Description

Fixes #8207
Supersedes #10745

Additional context and related issues

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(X) Release notes are required, with the following suggested text:

# Section
* fixes #8207 
* Materialized Views now show up within information_schema.tables with the table_type of `MATERIALIZED VIEW`

@cla-bot cla-bot Bot added the cla-signed label Dec 8, 2022
Copy link
Copy Markdown
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Did we already agree on using MATERIALIZED VIEW as the table type? I didn't find such conversations in GitHub or Trino Slack.

If we want to follow spec, we should return "VIEW" for materialized views #10745 (comment)

@findepi Does spec explicitly mention "the table type should be VIEW for materialized views"? Or that's because spec doesn't yet define materialized views?

@jklamer
Copy link
Copy Markdown
Member Author

jklamer commented Dec 9, 2022

@ebyhr I don't believe the spec specifies what Materialized Views should be which makes this tricky. This was just based on other projects and products, so it's by no means final, but I think the conversation should be started and decided on. I mostly drafted this to run all tests and see if there were other places in the code that were relying on materialized views as "Views"

@jklamer jklamer force-pushed the jklamer/MVInfoSchema branch from bea2953 to dbabdae Compare December 9, 2022 15:14
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.

@martint Could you confirm whether returning MATERIALIZED VIEW as a table type is okay?

Copy link
Copy Markdown
Member

@martint martint Jan 26, 2023

Choose a reason for hiding this comment

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

No, as it is incompatible with the SQL specification. It may affect tools or clients that expect this table to have return specific values.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jan 13, 2023

Relevent Slack thread https://trinodb.slack.com/archives/CP1MUNEUX/p1670951882153739

@findepi
Copy link
Copy Markdown
Member

findepi commented Jan 19, 2023

@jklamer is this work based on @duoluodexiaokeke's #10745

if yes, can you please restore authorship?

@jklamer
Copy link
Copy Markdown
Member Author

jklamer commented Jan 19, 2023

@findepi it was not. I only saw the issue as references in the tests when I started working on it. Let me add him as a co-author because his contribution should be marked and this cause is as much his.

@jklamer jklamer force-pushed the jklamer/MVInfoSchema branch from dbabdae to 6889d74 Compare January 23, 2023 21:19
@findepi
Copy link
Copy Markdown
Member

findepi commented Jan 24, 2023

I restarted the CI because sqlserver tests failed.

@jklamer jklamer force-pushed the jklamer/MVInfoSchema branch from a61f9f9 to 75d076a Compare January 24, 2023 16:26
Co-authored-by: liul <liulei@deepexi.com>
@jklamer jklamer force-pushed the jklamer/MVInfoSchema branch from 75d076a to 45d1217 Compare January 24, 2023 16:32
@findepi findepi merged commit ba5beab into trinodb:master Jan 26, 2023
@github-actions github-actions Bot added this to the 407 milestone Jan 26, 2023
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.

Define information_schema.tables's TABLE_TYPE for materialized views

4 participants