trino

Iceberg Catalog show create catalog fails when glue database has extra parameters

#25691

Open

Description

When a Glue database has extra parameters, Iceberg connectors fails with io.trino.spi.TrinoException: No PropertyMetadata for property: error

Additional context and related issues

It's possible for a Glue schema to have extra property metadata, for example below database has extra "CreatedBy" property: ( relevant AWS glue client doc )

raj-manvar:~/Downloads$ aws glue get-database --name rajmanvartest
{
    "Database": {
        "Name": "rajmanvartest",
        "Description": "raj manvar test",
        "LocationUri": "s3a://rajmanvartest/",
        "Parameters": {
            "CreatedBy": "raj-manvar"
        },
        "CreateTime": "2025-04-18T16:36:35-05:00",
        "CreateTableDefaultPermissions": [
            {
                "Principal": {
                    "PrincipalIdentifier": "IAM_ALLOWED_PRINCIPALS"
                },
                "Permissions": [
                    "ALL"
                ]
            }
        ],
        "CatalogId": "1111111111111"
    }

For such cases, when running a show create schema rajmanvartest query fails with the below stacktrace

io.trino.spi.TrinoException: No PropertyMetadata for property: CreatedBy
	at io.trino.metadata.PropertyUtil.lambda$toSqlProperties$0(PropertyUtil.java:197)
	at com.google.common.collect.RegularImmutableMap.forEach(RegularImmutableMap.java:300)
	at io.trino.metadata.PropertyUtil.toSqlProperties(PropertyUtil.java:190)
	at io.trino.sql.rewrite.ShowQueriesRewrite$Visitor.showCreateSchema(ShowQueriesRewrite.java:674)
	at io.trino.sql.rewrite.ShowQueriesRewrite$Visitor.visitShowCreate(ShowQueriesRewrite.java:520)
	at io.trino.sql.rewrite.ShowQueriesRewrite$Visitor.visitShowCreate(ShowQueriesRewrite.java:219)
	at io.trino.sql.tree.ShowCreate.accept(ShowCreate.java:61)
	at io.trino.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.trino.sql.rewrite.ShowQueriesRewrite.rewrite(ShowQueriesRewrite.java:216)
	at io.trino.sql.rewrite.StatementRewrite.rewrite(StatementRewrite.java:54)
	at io.trino.sql.analyzer.Analyzer.analyze(Analyzer.java:93)
	at io.trino.sql.analyzer.Analyzer.analyze(Analyzer.java:87)
	at io.trino.execution.SqlQueryExecution.analyze(SqlQueryExecution.java:289)
	at io.trino.execution.SqlQueryExecution.<init>(SqlQueryExecution.java:222)
	at io.trino.execution.SqlQueryExecution$SqlQueryExecutionFactory.createQueryExecution(SqlQueryExecution.java:892)
	at io.trino.dispatcher.LocalDispatchQueryFactory.lambda$createDispatchQuery$0(LocalDispatchQueryFactory.java:153)
	at io.trino.$gen.Trino_457_20250428_213635_2.call(Unknown Source)
	at com.google.common.util.concurrent.TrustedListenableFutureTask$TrustedFutureInterruptibleTask.runInterruptibly(TrustedListenableFutureTask.java:131)
	at com.google.common.util.concurrent.InterruptibleTask.run(InterruptibleTask.java:76)
	at com.google.common.util.concurrent.TrustedListenableFutureTask.run(TrustedListenableFutureTask.java:82)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1570)

This is because the getSchemaProperties for Iceberg Connector's implementation for Glue catalog type implemented here also adds the Glue database parameters.
The above implementation is called from the Iceberg Connector's Connector Metadata implementation here

However, IcebergSchemaProperties only supports the location of the table as the available property, because of this any additional Glue database parameters fails the show create schema query.

The Hive Connector's implementation for Glue type only queries the table location and doesn't consider Glue database parameters as implemented here.

This MR updates the Iceberg connector to not consider Glue database parameters so that Iceberg Catalog's show create schema for these type of Glue schema succeeds.

Release notes

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

## Section
* Fix some things. ({issue}`issuenumber`)

Please add a test to BaseIcebergConnectorSmokeTest.

Thanks @ebyhr for review. I had tried adding some tests, but the create schema Trino query doesn't support creating an equivalent glue database with extra parameters from what I see.
Can you help show any tests that have the schema used for testing created without running create schema ? Or any guidance on how to create a glue database which can be used for a test cases would be very helpful. Thanks.

https://github.com/trinodb/trino/blob/master/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java This test creates a Glue database, but doesn't run any Trino queries directly. Can't find any test file which created a custom Glue database and also runs Trino query

oh looks like this is a known issue #24744

@ebyhr https://github.com/trinodb/trino/blob/master/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java seems better place to add this test. This test class has glue client for being able to create a glue database with properties and also has queryRunner available to run show create schema queries. Is it okay to add test to this class instead ?

@raj-manvar No, we should test all catalogs.

Ah okay. I'll try to take a look. Why do we need a test for all catalog though ? the changes only affect the glue catalog

@raj-manvar To catch the same mistake in all catalogs.

Ah okay. Sorry I didn't realize BaseIcebergConnectorSmokeTest is common across all catalogs. Though looks like different catalog have a different response to show create catalog. For example the Nessie catalog doesn't return a location for the show create catalog query.
Would those cases require overwriting the common test in their respective implementation of BaseIcebergConnectorSmokeTest class ?

Oh Maybe another alternative is to have the BaseIcebergConnectorSmokeTest test just match a basic regex like

    @Test
    public void testShowCreateSchema()
    {
        String schemaName = getSession().getSchema().orElseThrow();
        assertThat((String) computeScalar("SHOW CREATE SCHEMA " + schemaName))
                .matches("CREATE SCHEMA iceberg." + schemaName);
    }

And then we can updated the TestIcebergGlueCatalogConnectorSmokeTest ( which extends the BaseIcebergConnectorSmokeTest class ) to have a testShowCreateSchema method which would also check is the location property exists for Glue catalog case ? Would that work ?

@ebyhr Can you please help confirm if the above testing method would work.

@ebyhr Ping