Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for materialized view column comments #18016

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Add support for materialized view column comments #18016

merged 3 commits into from
Aug 2, 2023

Conversation

vlad-lyutenko
Copy link
Contributor

@vlad-lyutenko vlad-lyutenko commented Jun 22, 2023 •
edited by ebyhr
Loading

Release notes

(x) Release notes are required, with the following suggested text:

# General, Iceberg
* Add support for commenting on columns in materialized views. ({issue}`18016`)

@cla-bot cla-bot bot added the cla-signed label Jun 22, 2023
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 3 times, most recently from 974b05b to 01ac856 Compare June 22, 2023 18:16
@github-actions github-actions bot added hive Hive connector iceberg Iceberg connector tests:hive labels Jun 22, 2023
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch from 01ac856 to e7f2713 Compare June 22, 2023 19:21
@vlad-lyutenko
Copy link
Contributor Author

Will add more tests

@ebyhr ebyhr self-requested a review June 22, 2023 22:27
ebyhr
ebyhr reviewed Jun 22, 2023
View reviewed changes
Copy link
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.

Just skimmed.

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java Outdated Show resolved Hide resolved
core/trino-main/src/main/java/io/trino/metadata/Metadata.java Outdated Show resolved Hide resolved
core/trino-main/src/main/java/io/trino/execution/CommentTask.java Show resolved Hide resolved
core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java Outdated Show resolved Hide resolved
core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java Outdated Show resolved Hide resolved
...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMaterializedViewDefinition.java Outdated Show resolved Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java Outdated Show resolved Hide resolved
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch from e7f2713 to e017960 Compare June 23, 2023 08:29
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 3 times, most recently from fac52c5 to 5e2d468 Compare June 25, 2023 11:30
raunaqmorarka
raunaqmorarka reviewed Jun 28, 2023
View reviewed changes
Copy link
Member

@raunaqmorarka raunaqmorarka left a comment

Choose a reason for hiding this comment

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

Please update docs to include an example usage of this

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java Outdated Show resolved Hide resolved
core/trino-main/src/main/java/io/trino/execution/CommentTask.java Outdated Show resolved Hide resolved
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 4 times, most recently from 17b8697 to 2d880cb Compare June 28, 2023 10:04
@github-actions github-actions bot added the docs label Jun 28, 2023
@vlad-lyutenko
Copy link
Contributor Author

Looks like pinot test is flaky #14239

ebyhr
ebyhr reviewed Jul 10, 2023
View reviewed changes
core/trino-main/src/main/java/io/trino/execution/CommentTask.java Show resolved Hide resolved
core/trino-main/src/test/java/io/trino/connector/MockConnector.java Outdated Show resolved Hide resolved
core/trino-main/src/test/java/io/trino/execution/TestCommentTask.java Outdated Show resolved Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java Outdated Show resolved Hide resolved
testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java Outdated Show resolved Hide resolved
testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java Outdated Show resolved Hide resolved
testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java Outdated Show resolved Hide resolved
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 2 times, most recently from 103b9a1 to 0da345e Compare July 10, 2023 11:04
ebyhr
ebyhr reviewed Jul 10, 2023
View reviewed changes
Copy link
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.

Basic support for materialized view column comments
Iceberg support for materialized view column comments

Please follow the commit message guideline.

https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#format-git-commit-messages

  1. Use the imperative mood in the subject line

testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java Outdated Show resolved Hide resolved
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 2 times, most recently from 04e6597 to eb0dd4c Compare July 29, 2023 12:48
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch from eb0dd4c to 5fc5469 Compare July 31, 2023 09:55
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 4 times, most recently from b782019 to 468298a Compare July 31, 2023 12:59
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch 3 times, most recently from 60e027e to ad02c79 Compare August 1, 2023 11:42
@vlad-lyutenko vlad-lyutenko mentioned this pull request Aug 1, 2023
Open
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch from ad02c79 to e57a7e2 Compare August 1, 2023 15:39
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch from e57a7e2 to 4f19297 Compare August 1, 2023 15:42
@vlad-lyutenko vlad-lyutenko force-pushed the vlad-lyutenko/materialized-view-column-comment branch from 4f19297 to 4696802 Compare August 2, 2023 07:55
@Praveen2112 Praveen2112 merged commit 6168881 into trinodb:master Aug 2, 2023
@Praveen2112
Copy link
Member

Thanks for working on this

@github-actions github-actions bot added this to the 423 milestone Aug 2, 2023
This was referenced Aug 2, 2023
Merged
Merged
@vlad-lyutenko vlad-lyutenko mentioned this pull request Aug 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findepi findepi findepi left review comments

@Praveen2112 Praveen2112 Praveen2112 approved these changes

@ebyhr ebyhr ebyhr approved these changes

@raunaqmorarka raunaqmorarka raunaqmorarka approved these changes

@kokosing kokosing Awaiting requested review from kokosing

Assignees
No one assigned
Labels
cla-signed docs hive Hive connector iceberg Iceberg connector
Milestone
423
Development

Successfully merging this pull request may close these issues.
5 participants
@vlad-lyutenko @ebyhr @Praveen2112 @findepi @raunaqmorarka