Add Iceberg properties in SHOW CREATE MATERIALIZED VIEW

[polaris6]

Description

Show Iceberg location and format_version in SHOW CREATE MATERIALIZED VIEW.

Related issues, pull requests, and links

Fixes #12504

Documentation

(x) No documentation is needed.

Release notes

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

# Iceberg
* Show Iceberg location and format_version in `SHOW CREATE MATERIALIZED VIEW`. ({issue}`12504 `)

[alexjo2144]

Thanks!

[findepi]

Create the MV as it used to be created and update only the SHOW CREATE ... assertion:

Path schemaDirectory = getDistributedQueryRunner().getCoordinator().getBaseDataDir().resolve("iceberg_data/tpch");
        assertThat((String) computeScalar("show create materialized view  materialized_view_window"))
                .matches("\\QCREATE MATERIALIZED VIEW " + qualifiedMaterializedViewName + "\n" +
                        "WITH (\n" +
                        "   format = 'ORC',\n" +
                        "   format_version = 2,\n" +
                        "   location = '" + schemaDirectory + "/st_\\E[0-9a-f]+\\Q',\n" +
                        "   partitioning = ARRAY['_date']\n" +
                        ") AS\n" +
                        "SELECT\n" +
                        "  _date\n" +
                        ", sum(_bigint) OVER (PARTITION BY _date ORDER BY _date ASC) sum_ints\n" +
                        "FROM\n" +
                        "  base_table1");

[findepi]

same in the other test

[polaris6]

Thanks for your advice, I updated the commit.

[findepi]

can icebergTable.location() be an empty string? when?

[polaris6]

This location is actually the location of the storage table, when the storage table is dropped and a storage table with an empty location with the same name is created, the location will be empty.
Full Screenshot:

In addition, does it need to be consistent with spark? Show location regardless of whether location is empty or not.

[findepi]

This location is actually the location of the storage table, when the storage table is dropped and a storage table with an empty location with the same name is created, the location will be empty.

This is not a valid situation we need to care about.
If a storage table is manually replaced with something else, all sorts of problems may occur.
SHOW CREATE MATERIALIZED VIEW may fail in such case.

[findepi]

thus, please remove the condition

[polaris6]

done, thanks!

Does this need to be modified? if so, I will submit another pr to modify it.
https://github.com/trinodb/trino/blob/master/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java#L1296

[raunaqmorarka]

On latest mater we're now also missing ORC_BLOOM_FILTER_COLUMNS and ORC_BLOOM_FILTER_FPP.
I would suggest extracting common code for this from IcebergMetadata#getTableMetadata and re-using it here to avoid the need to update in two places.

[polaris6]

Thanks for your advice!
I have another question, does SHOW CREATE TABLE need to show orc_bloom_filter_columns and orc_bloom_filter_fpp properties? These two properties are specific to the orc format and are not generic, I personally think they should be shown through $properties instead of SHOW CREATE TABLE, of course $properties can already show them now.
https://trino.io/docs/current/connector/iceberg.html#properties-table

[raunaqmorarka]

It looks like the bloom filter properties already don't show up in SHOW CREATE MV if bloom filter is not configured (correct me if I'm wrong). That behaviour seems ok to me.
It would be great if we also include a test in BaseIcebergMaterializedViewTest which uses the orc bloom filter properties.

[polaris6]

Thanks!

Currently, SHOW CREATE TABLE shows orc_bloom_filter_columns and orc_bloom_filter_fpp :

I will add test cases in BaseIcebergMaterializedViewTest.

[polaris6]

@raunaqmorarka I updated the testShowCreate() as I felt no need to add a separate test case to test this situation.

[polaris6]

Hi @findepi, could you please help me review or merge this PR when you have time? Thank you very much!