Add support to redirect table operations from Iceberg to Hive #11356

findinpath · 2022-03-07T20:38:49Z

Description

New property introduced for the Iceberg connector: iceberg.hive-catalog-name

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)

The changes in this PR affect mainly the Iceberg connector.

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

In an environment which makes use of a shared metastore it may come in handy to have table redirects to automatically allow Trino to translate a table name like iceberg.default.hive_table_name towards the name hive.default.hive_table_name.

Note that the translation can happen quite transparently when the user connects to a predefined catalog and schema (e.g. : iceberg.default) and the table operation looks like: SELECT * FROM hive_table_name.

Related issues, pull requests, and links

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:

# Iceberg
* Add support to redirect table operations from Iceberg to Hive

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

...product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergRedirectionToHive.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java

...t-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergHiveTablesCompatibility.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSessionProperties.java

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

alexjo2144 · 2022-03-11T15:18:32Z

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java

@@ -732,6 +735,67 @@ public void renameMaterializedView(ConnectorSession session, SchemaTableName sou
        return listNamespaces(session);
    }

+    @Override
+    public Optional<CatalogSchemaTableName> redirectTable(ConnectorSession session, SchemaTableName tableName)


This doesn't seem like an Iceberg-catalog level method. Can we narrow this down to just the things that are dependent on the catalog implementation and move the rest up to IcebergMetadata?

This doesn't seem like an Iceberg-catalog level method.

Initially I created a TableRedirectionHandler abstraction which was created at the same time as the TrinoCatalog instance, but I dropped afterwards the idea because the redirection is tightly linked to the metastore implementation (hive/glue).

The same boilerplate code used for creating the TrinoCatalog would need to be done for redirection handling as well.

On a second thought, it probably makes sense to take out this operation from TrinoCatalog in hindsight of JDBC / REST catalogs which will not have anymore Hive related content in them.

I've modified the code again so that the redirectTable method lives in the TrinoHiveCatalog and TrinoGlueCatalog, but it is not exposed in the TrinoCatalog interface in order to keep the interface free from the concept of table redirection, but still be able to provide this functionality for hive & glue.

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

findinpath · 2022-03-31T06:56:51Z

...product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergRedirectionToHive.java


-        // test via redirection with just schema filter
+        // test via redirection with just schema filter - consistent with the functionality of the command `SHOW TABLES` command on the Iceberg connector


Note the change in functionality here for the Iceberg connector. Please provide feedback whether the provided functionality is incorrect from your perspective.

findinpath · 2022-03-31T08:52:00Z

Rebased on master to adapt the redirect test io.trino.tests.product.iceberg.TestIcebergRedirectionToHive#testAlterTableRename

findepi · 2022-04-01T07:09:19Z

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

@@ -205,7 +206,9 @@ public IcebergMetadata(
    public IcebergTableHandle getTableHandle(ConnectorSession session, SchemaTableName tableName)
    {
        IcebergTableName name = IcebergTableName.from(tableName.getTableName());
-        verify(name.getTableType() == DATA, "Wrong table type: " + name.getTableNameWithType());
+        if (name.getTableType() != DATA) {
+            return null;


When the table redirection towards Hive connector is not enabled,
in case of trying to query on the Iceberg connector
a metadata table of a Hive connector table,
the user will receive a table not found exception.

From io.trino.tests.product.iceberg.TestIcebergHiveTablesCompatibility#testIcebergSelectFromHiveTable

assertQueryFailure(() -> onTrino().executeQuery("SELECT * FROM iceberg.default.\"" + tableName + "$data\"")) .hasMessageMatching("Query failed \$#\\w+\$:\\Q Not an Iceberg table: default." + tableName); assertQueryFailure(() -> onTrino().executeQuery("SELECT * FROM iceberg.default.\"" + tableName + "$files\"")) .hasMessageMatching("Query failed \$#\\w+\$:\\Q line 1:15: Table 'iceberg.default." + tableName + "$files' does not exist");

keeping verify in such case seems "OK" too, wdyt?

Keeping verify would issue for the statement:

onTrino().executeQuery("SELECT * FROM iceberg.default.\"" + hiveTableName + "$files\"")

error messages like the following:

Wrong table type: test_iceberg_select_from_hive_63u5u11q3c70$files

I tend to say that the error message

Table 'iceberg.default." + hiveTableName + "$files' does not exist"

fits better (not ideal, but better).

This requires code comment

Just to make it clear from the code perspective:

io.trino.plugin.iceberg.IcebergMetadata#getTableHandle is reached by a select from hive_table_name$files because there is no system table in Iceberg found for the hive_table_name (IcebergMetadata#getSystemTable returns Optional.empty() in such cases).

In StatementAnalyzer#visitTable the logic of the method assumes that if the identifier doesn't correspond to a MV or to a view, it is certainly a table. For this reason IcebergMetadata#getTableHandle is called with the rather unexpected argument hive_table_name$files.

Probably a refactoring of StatementAnalyzer#visitTable method which verifies in the beginning whether we're dealing with an redirected table and acts accordingly would be the right way to go, but such a change (if it makes sense) would rather fit in a different PR.

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataFactory.java

findepi · 2022-04-01T07:14:50Z

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

+        if (isIcebergTable(table.get())) {
+            // After redirecting, use the original table name, with "$partitions" and similar suffixes
+            return targetCatalogName.map(catalog -> new CatalogSchemaTableName(catalog, tableName));
+        }


we should redirect non-Iceberg tables, rather than redirect Iceberg tables

I missed this bug because I didn't know I can run the Glue tests locally.
@findepi Once the PR is in a good shape, please run it against the CI with AWS GLue secrets.

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

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java

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

...t-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergHiveTablesCompatibility.java

findinpath · 2022-04-01T10:18:06Z

Rebased on master to make use of b395286

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestSharedHiveMetastore.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

alexjo2144 · 2022-04-01T15:03:22Z

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

+        }
+        if (!isIcebergTable(table.get())) {
+            // After redirecting, use the original table name, with "$partitions" and similar suffixes
+            return targetCatalogName.map(catalog -> new CatalogSchemaTableName(catalog, tableName));


Nit, I'd use get instead of map here, makes it clear that an empty catalog name shouldn't ever show up at this point.

We do have a few lines above:

Optional<String> targetCatalogName = getHiveCatalogName(session); if (targetCatalogName.isEmpty()) { return Optional.empty(); }

Also note that the method returns an Optional, so I'd have to do .get() and then wrap the result back to Optional.

alexjo2144 · 2022-04-01T15:08:15Z

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

+
+        Optional<com.amazonaws.services.glue.model.Table> table = getTable(new SchemaTableName(tableNameBase.getSchemaName(), tableNameBase.getTableName()));
+
+        if (table.isEmpty()) {


Now that Glue views are merged we should probably match what the Hive catalog has for this line: (table.isEmpty() || VIRTUAL_VIEW.name().equals(table.get().getTableType()))

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java

.

findinpath · 2022-04-04T10:19:40Z

Rebased on top of master to make use of the changes from #11617

findepi · 2022-04-05T12:22:00Z

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

@@ -205,7 +206,10 @@ public IcebergMetadata(
    public IcebergTableHandle getTableHandle(ConnectorSession session, SchemaTableName tableName)
    {
        IcebergTableName name = IcebergTableName.from(tableName.getTableName());
-        verify(name.getTableType() == DATA, "Wrong table type: " + name.getTableNameWithType());
+        if (name.getTableType() != DATA) {
+            // Avoid dealing with non DATA table types


// Pretend the table does not exist to produce better message in case of redirects to Hive

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

findinpath · 2022-04-05T12:37:28Z

Rebased on top of master to deal with the refactoring of the glue tests.

The Iceberg connector can make use of the `iceberg.hive-catalog-name` configuration property for enable table redirects towards the Hive tables. When the table redirection towards Hive connector is not enabled, in case of trying to query on the Iceberg connector a metadata table of a Hive connector table, the user will receive a table not found exception.

cla-bot bot added the cla-signed label Mar 7, 2022

github-actions bot added the tests:hive label Mar 7, 2022

findinpath commented Mar 7, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java Outdated Show resolved Hide resolved

findinpath commented Mar 7, 2022

View reviewed changes

...product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergRedirectionToHive.java Outdated Show resolved Hide resolved

findinpath commented Mar 7, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java Outdated Show resolved Hide resolved

findinpath commented Mar 7, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java Outdated Show resolved Hide resolved

findinpath marked this pull request as draft March 8, 2022 11:28

findepi requested review from homar and alexjo2144 March 9, 2022 10:10

findinpath force-pushed the iceberg-redirect-table-to-hive branch 2 times, most recently from 9501f3e to ebd2fbb Compare March 9, 2022 16:10

findinpath marked this pull request as ready for review March 9, 2022 16:13

findinpath requested a review from jklamer March 9, 2022 16:13

findinpath commented Mar 9, 2022

View reviewed changes

...t-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergHiveTablesCompatibility.java Outdated Show resolved Hide resolved

findinpath requested review from phd3 and findepi March 11, 2022 10:26

findinpath force-pushed the iceberg-redirect-table-to-hive branch from 0d33751 to acc1604 Compare March 11, 2022 10:27

alexjo2144 reviewed Mar 11, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSessionProperties.java Show resolved Hide resolved

alexjo2144 reviewed Mar 11, 2022

View reviewed changes

jklamer reviewed Mar 11, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java Outdated Show resolved Hide resolved

findinpath force-pushed the iceberg-redirect-table-to-hive branch from acc1604 to 6b75248 Compare March 14, 2022 12:08

findinpath requested a review from alexjo2144 March 14, 2022 12:08

findinpath force-pushed the iceberg-redirect-table-to-hive branch from 6b75248 to d863ff6 Compare March 14, 2022 14:31

findinpath commented Mar 14, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java Outdated Show resolved Hide resolved

findinpath force-pushed the iceberg-redirect-table-to-hive branch 6 times, most recently from 152eafb to 909f1cc Compare March 17, 2022 10:25

findinpath commented Mar 31, 2022

View reviewed changes

findinpath force-pushed the iceberg-redirect-table-to-hive branch from 1058105 to 50e3430 Compare March 31, 2022 08:51

findepi reviewed Apr 1, 2022

View reviewed changes

findinpath force-pushed the iceberg-redirect-table-to-hive branch from 50e3430 to 13196c8 Compare April 1, 2022 10:17

findinpath commented Apr 1, 2022

View reviewed changes

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestSharedHiveMetastore.java Outdated Show resolved Hide resolved

findinpath force-pushed the iceberg-redirect-table-to-hive branch 2 times, most recently from 863c172 to bc81a1a Compare April 1, 2022 10:47

findinpath requested a review from findepi April 1, 2022 10:50

alexjo2144 reviewed Apr 1, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java Show resolved Hide resolved

alexjo2144 reviewed Apr 1, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java Outdated Show resolved Hide resolved

findinpath force-pushed the iceberg-redirect-table-to-hive branch from bc81a1a to 58426ff Compare April 4, 2022 10:18

findepi reviewed Apr 5, 2022

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java Outdated Show resolved Hide resolved

findinpath force-pushed the iceberg-redirect-table-to-hive branch 2 times, most recently from fd3366f to 101291d Compare April 5, 2022 12:36

findinpath force-pushed the iceberg-redirect-table-to-hive branch from 101291d to b2a73ff Compare April 5, 2022 13:34

findepi approved these changes Apr 5, 2022

View reviewed changes

findepi merged commit 7562be6 into trinodb:master Apr 6, 2022

findepi mentioned this pull request Apr 6, 2022

Release notes for 376 #11692

Closed

github-actions bot added this to the 376 milestone Apr 6, 2022

mosabua mentioned this pull request Apr 6, 2022

Add Trino 376 release notes #11691

Merged

ebyhr mentioned this pull request Jan 18, 2023

Add Trino 406 release notes #15625

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to redirect table operations from Iceberg to Hive #11356

Add support to redirect table operations from Iceberg to Hive #11356

findinpath commented Mar 7, 2022

alexjo2144 Mar 11, 2022

findinpath Mar 14, 2022

findinpath Mar 14, 2022

findinpath Mar 17, 2022

findinpath Mar 31, 2022

findinpath commented Mar 31, 2022

findepi Apr 1, 2022

findinpath Apr 1, 2022

findepi Apr 1, 2022

findinpath Apr 1, 2022

findepi Apr 1, 2022

findinpath Apr 4, 2022

findepi Apr 1, 2022

findinpath Apr 1, 2022

findinpath commented Apr 1, 2022

alexjo2144 Apr 1, 2022

findinpath Apr 4, 2022

alexjo2144 Apr 1, 2022

findinpath commented Apr 4, 2022

findepi Apr 5, 2022

findinpath commented Apr 5, 2022


		// test via redirection with just schema filter
		// test via redirection with just schema filter - consistent with the functionality of the command `SHOW TABLES` command on the Iceberg connector


		Optional<com.amazonaws.services.glue.model.Table> table = getTable(new SchemaTableName(tableNameBase.getSchemaName(), tableNameBase.getTableName()));

		if (table.isEmpty()) {

Add support to redirect table operations from Iceberg to Hive #11356

Add support to redirect table operations from Iceberg to Hive #11356

Conversation

findinpath commented Mar 7, 2022

Description

Related issues, pull requests, and links

Documentation

Release notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findinpath commented Mar 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findinpath commented Apr 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findinpath commented Apr 4, 2022

Choose a reason for hiding this comment

findinpath commented Apr 5, 2022