Add support for view column comments by findinpath · Pull Request #13800 · trinodb/trino

findinpath · 2022-08-23T17:12:06Z

Description

Is this change a fix, improvement, new feature, refactoring, or other?

New feature

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Hive, Iceberg, Memory, Blackhole connectors

How would you describe this change to a non-technical end user or system administrator?

Support adding comments on view / materialized view columns.

COMMENT ON COLUMN my_view.col1 IS 'col1 description';

Related issues, pull requests, and links

Fixes #10705

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Hive, Iceberg, Memory
* Add support for setting view column comments over `COMMENT ON COLUMN` sql command

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMaterializedViewDefinition.java

testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestComments.java

testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java

core/trino-main/src/test/java/io/trino/execution/TestCommentTask.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/TrinoCatalog.java

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java

plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorViewDefinition.java

ebyhr · 2022-10-13T09:55:28Z

Could you separate into some commits? e.g.

Add support for view column comments
Support view column comments in Hive
Support view column comments in Iceberg
Support view column comments in Memory

findinpath · 2022-10-13T10:39:00Z

Rebasing on top of the latest changes made on 399

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java

findinpath · 2022-10-13T12:15:15Z

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java

Do we need to add @JsonProperty here? BTW the whole class is missing json annotations.

findinpath · 2022-10-19T13:41:17Z

Rebasing to fix conflicts with master.

findinpath · 2022-10-26T12:27:05Z

Rebased on master to keep the PR in sync with the latest changes from Trino.

findinpath · 2022-10-26T16:12:01Z

CI hit #14772

findinpath · 2022-10-27T05:22:19Z

CI hit #14441

findinpath · 2022-10-27T16:26:10Z

CI hit trino-kudu

Caused by: io.trino.testing.QueryFailedException: unable to create native thread: possibly out of memory or process/resource limits reached

findepi · 2023-08-07T16:40:20Z

plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java

+        if (translateHiveViews && !isPrestoView(view)) {
+            throw new HiveViewNotSupportedException(viewName);
+        }
+        return view;


this looks weird -- if (! translateHiveViews && !isPrestoView) would feel more correct

(i am changing this in #18570)

cla-bot bot added the cla-signed label Aug 23, 2022

github-actions bot added the tests:hive label Aug 23, 2022

findinpath force-pushed the hive-view-column-comment branch 3 times, most recently from e02893d to 029a03b Compare August 29, 2022 06:40

findinpath changed the title ~~Draft: Add support for view column comments~~ Add support for view column comments Aug 29, 2022

findinpath force-pushed the hive-view-column-comment branch 7 times, most recently from 997196c to 55057df Compare August 30, 2022 05:43

findinpath marked this pull request as ready for review August 30, 2022 05:43

findinpath requested a review from ebyhr August 30, 2022 12:00

findinpath force-pushed the hive-view-column-comment branch from 55057df to 529f689 Compare August 31, 2022 08:15

findinpath mentioned this pull request Aug 31, 2022

Add test for 'COMMENT ON COLUMN' statement #13938

Merged

findinpath force-pushed the hive-view-column-comment branch from 529f689 to 41aaad3 Compare September 1, 2022 05:31

findinpath force-pushed the hive-view-column-comment branch from 41aaad3 to 2547331 Compare September 9, 2022 14:04

findinpath force-pushed the hive-view-column-comment branch from 2547331 to 0f346bf Compare October 4, 2022 05:46

ebyhr reviewed Oct 6, 2022

View reviewed changes

findinpath force-pushed the hive-view-column-comment branch 4 times, most recently from 17c9850 to 8c74375 Compare October 7, 2022 08:12

findinpath requested a review from ebyhr October 7, 2022 08:12

ebyhr mentioned this pull request Oct 13, 2022

[WIP] Add comments at column level for views #10854

Closed

ebyhr approved these changes Oct 13, 2022

View reviewed changes

findinpath force-pushed the hive-view-column-comment branch from 4aea3d0 to 9bdf3fb Compare October 13, 2022 10:38

findinpath force-pushed the hive-view-column-comment branch from 9bdf3fb to e609d61 Compare October 13, 2022 12:11

findinpath commented Oct 13, 2022

View reviewed changes

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java Outdated Show resolved Hide resolved

findinpath force-pushed the hive-view-column-comment branch from e609d61 to 8a1d666 Compare October 13, 2022 12:14

findinpath commented Oct 13, 2022

View reviewed changes

findinpath force-pushed the hive-view-column-comment branch 3 times, most recently from dbce5aa to afb9e49 Compare October 14, 2022 12:49

findinpath requested a review from ebyhr October 15, 2022 05:14

findinpath force-pushed the hive-view-column-comment branch from afb9e49 to 0f87ad6 Compare October 19, 2022 13:41

findinpath force-pushed the hive-view-column-comment branch from 726fcbe to 73b46ab Compare October 26, 2022 12:26

findinpath added 4 commits October 27, 2022 09:31

Add support for view column comments

1bfc9ae

Support view column comments in Hive

b53b1bd

Support view column comments in Iceberg

6cb4840

Support view column comments in Memory

5853537

findinpath force-pushed the hive-view-column-comment branch from 73b46ab to 5853537 Compare October 27, 2022 07:42

ebyhr merged commit 91c76a0 into trinodb:master Oct 28, 2022

ebyhr mentioned this pull request Oct 28, 2022

Release notes for 402 #14780

Closed

hovaesco mentioned this pull request Oct 28, 2022

Enable column-level comments in views tests starburstdata/dbt-trino#172

Closed

1 task

github-actions bot added this to the 402 milestone Oct 28, 2022

colebow mentioned this pull request Nov 1, 2022

Add Trino 402 release notes #14862

Merged

findepi reviewed Aug 7, 2023

View reviewed changes

Conversation

findinpath commented Aug 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues, pull requests, and links

Documentation

Release notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebyhr commented Oct 13, 2022

Uh oh!

findinpath commented Oct 13, 2022

Uh oh!

Uh oh!

findinpath Oct 13, 2022

Choose a reason for hiding this comment

Uh oh!

findinpath commented Oct 19, 2022

Uh oh!

findinpath commented Oct 26, 2022

Uh oh!

findinpath commented Oct 26, 2022

Uh oh!

findinpath commented Oct 27, 2022

Uh oh!

findinpath commented Oct 27, 2022

Uh oh!

findepi Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

findepi Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

findinpath commented Aug 23, 2022 •

edited

Loading