Support setting compression_codec table property for Iceberg #25755

SongChujun · 2025-05-08T18:34:24Z

This PR makes compression_codec a table property and drops support for compression_codec iceberg session property for simplicity.

Users are able to run the following command now to specify the compression_codec to create a new table with compression_codec set to ZSTD.

CREATE TABLE example (
    c1 INTEGER,
    c2 DATE,
    c3 DOUBLE
)
WITH (
    format = 'AVRO',
    partitioning = ARRAY['c1', 'c2'],
    sorted_by = ARRAY['c3'],
    compression_codec = 'ZSTD'
);

Users are able able to change the compression_codec via statement Alter Table Set Properties.

Example

ALTER TABLE example SET PROPERTIES compression_codec = 'GZIP';

The write_compression users specify will replace the session variable iceberg.compression-codec. If the user change file_format without changing write_compression, it will inherit the write_compression set for the original file format, if the write_compression for the previous file format is set.

If the user is trying to set a write_compression that is inconsistent with the file format, the system will throw an exception.

The compatibility matrix for write_compression and file format is the following

	Orc	Parquet	Avro
NONE	✅	✅	✅
SNAPPY	✅	✅	✅
LZ4	✅	❌	❌
ZSTD	✅	✅	✅
GZIP	✅	✅	✅

Description

Additional context and related issues

Release notes

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

## Iceberg connector
* Add `compression_codec` table property and remove `compression_codec` session property. ({issue}`25755`)

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

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

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

pettyjamesm · 2025-05-09T17:15:48Z

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

Is this related to the iceberg parquet compression handling bug? If so, we'll want an inline comment explaining what's going on here

Could you clarify what is the bug being referring to here ? I responded at #24851 (comment) to the concern raised there.

If we let Iceberg write the table property write.parquet.compression-codec to zstd by default for us, then this PR will introduce some non-backward-compatible behavior. Suppose we have a user that didn't set write.parquet.compression-codec in their table property, if they are setting write.parquet.compression-codec to snappy in their iceberg config/iceberg session property, when they write a new file, on a previous version of Trino, the compression-codec being used would be snappy. But because this change will try to also read compression-codec from table properties when writing a file, this time the compression-codec being used would be zstd instead of snappy, which is not backward compatible. So we want to disable Iceberg from setting this table property if that's not being set by user.

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

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

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

Copilot

Pull Request Overview

This PR adds support for specifying the write_compression and compression_level table properties for the Iceberg connector and updates related tests and compression property validations.

Adds new DDL support and property validations for write_compression and compression_level.
Updates tests to verify new expected compression property values.
Adjusts compression property handling across table creation, metadata, and file writer implementations.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergV2.java	Updated expected values in JSON assertions.
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergSystemTables.java	Modified test expectations for column_sizes and compression properties.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java	Revised table properties creation and transaction handling.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java	Introduced new methods for compression level and codec extraction.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableProperties.java	Added new table properties and validation for compression settings.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java	Updated table property and compression handling during metadata updates.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergFileWriterFactory.java	Adjusted writer creation to incorporate storage properties for compression.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergAvroFileWriter.java	Updated Avro writer to accept and use a compression level.

Comments suppressed due to low confidence (1)

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java:897

The method 'createTableProperties' is expected to return a Map<String, String>, but it is returning a Transaction object. Please update the return value to the actual table properties map or adjust the method signature accordingly to ensure type consistency.

return txn;

raunaqmorarka · 2025-05-20T06:15:18Z

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

    public static final String LOCATION_PROPERTY = "location";
    public static final String FORMAT_VERSION_PROPERTY = "format_version";
+
+    public static final String WRITE_COMPRESSION = "write_compression";


Property name should be compression_codec to maintain consistency with equivalent iceberg table property and the connector session property.

The order of precedence should be: session property > table property > catalog config property

I can change the property name. But I don't understand why session property should have a higher precedence over table property, if a user set table property for a particular table, it should have higher precedence compared to the default one from session property right?

@raunaqmorarka - I would tend to agree with @SongChujun that the precedence order should prefer the table defined codec over the session property (when present) since the table definition seems more specific. I guess you could make an argument that the order should be: session property (if present, but default value is empty and not the default value from catalog config), table property (if present), catalog config default value. Is that what you're suggesting here?

Usually session property has precedence when it is set, because that is meant to allow users to override configured behaviour on a per query level. Per query write operation is more granular that per table, so if the session property exists it should get higher precedence.
I haven't been able to find a similar scenario in the code base where something is a session and catalog property while also being a table property. I think trying to be both a session property and table property at the same time is making this code and resulting behaviour quite complex, even though it is a seemingly simple thing.
I think we should consider dropping the existing session property entirely and just having table property and catalog config. I can't think of a great use case for being able to override compression per query for a table. Getting rid of the session property should simplify things here.

raunaqmorarka · 2025-05-20T06:15:58Z

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

        return (IcebergFileFormat) tableProperties.get(FILE_FORMAT_PROPERTY);
    }

+    public static Optional<HiveCompressionCodec> getHiveCompressionCodec(Map<String, Object> inputProperties)


getCompressionCodec

will update the name

raunaqmorarka · 2025-05-20T06:24:12Z

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

        }
-        return catalog.newCreateTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, Optional.ofNullable(tableLocation), createTableProperties(session, tableMetadata, allowedExtraProperties));
+
+        // If user doesn't set compression-codec for parquet, we need to remove write.parquet.compression-codec property,


If user doesn't set anything, our default for iceberg.compression-codec catalog config property should apply.

Already replied in a previous comment.

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

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

raunaqmorarka · 2025-05-20T07:06:42Z

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

Could you clarify what is the bug being referring to here ? I responded at #24851 (comment) to the concern raised there.

github-actions · 2025-06-13T17:03:17Z

This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack.

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

pettyjamesm · 2025-06-17T14:39:37Z

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

    public static final String LOCATION_PROPERTY = "location";
    public static final String FORMAT_VERSION_PROPERTY = "format_version";
+
+    public static final String WRITE_COMPRESSION = "write_compression";


@raunaqmorarka - I would tend to agree with @SongChujun that the precedence order should prefer the table defined codec over the session property (when present) since the table definition seems more specific. I guess you could make an argument that the order should be: session property (if present, but default value is empty and not the default value from catalog config), table property (if present), catalog config default value. Is that what you're suggesting here?

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

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

raunaqmorarka

lgtm overall

raunaqmorarka · 2025-07-15T13:49:49Z

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

        }

+        IcebergFileFormat oldFileFormat = getFileFormat(icebergTable.properties());
+        IcebergFileFormat newFileFormat = getFileFormat(icebergTable.properties());


newFileFormat = oldFileFormat ?

changed it here.

raunaqmorarka · 2025-07-15T14:37:48Z

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

                .map(column -> toTrinoType(column.type(), typeManager))
                .collect(toImmutableList());

+        HiveCompressionCodec compressionCodec = getHiveCompressionCodec(IcebergFileFormat.AVRO, storageProperties)


nit: static import AVRO here and in the other files

changed all IcebergFileFormat.[Parquet, Orc, Avro] to [Parquet, Orc, Avro]

docs/src/main/sphinx/connector/iceberg.md

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

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

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

...g/src/main/java/io/trino/plugin/iceberg/catalog/snowflake/IcebergSnowflakeCatalogModule.java

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

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

pettyjamesm

LGTM, thanks @SongChujun and @ebyhr!

Makes compression_codec a table property and drops support for compression_codec session property for simplicity

cla-bot bot added the cla-signed label May 8, 2025

github-actions bot added the iceberg Iceberg connector label May 8, 2025

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch 3 times, most recently from 36c996a to b5e8d55 Compare May 8, 2025 22:16

SongChujun added the enhancement New feature or request label May 8, 2025

SongChujun requested a review from pettyjamesm May 8, 2025 23:45

pettyjamesm reviewed May 9, 2025

View reviewed changes

pettyjamesm requested a review from raunaqmorarka May 9, 2025 17:22

pettyjamesm reviewed May 9, 2025

View reviewed changes

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

SongChujun marked this pull request as ready for review May 9, 2025 18:28

SongChujun self-assigned this May 9, 2025

raunaqmorarka requested review from SemionPar, Copilot, ebyhr, findinpath and pajaks May 12, 2025 17:49

Copilot AI reviewed May 12, 2025

View reviewed changes

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch 2 times, most recently from ffaf8b7 to 4d27522 Compare May 16, 2025 15:58

raunaqmorarka reviewed May 20, 2025

View reviewed changes

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch from 4d27522 to 4c0b43f Compare May 22, 2025 15:05

github-actions bot added the docs label May 22, 2025

SongChujun changed the title ~~Support setting write_compression and compression_level table properties for Iceberg Connector~~ Support setting compression_codec table properties for Iceberg Connector May 22, 2025

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch 2 times, most recently from 5d80c4b to e53d72e Compare May 22, 2025 20:33

SongChujun requested a review from raunaqmorarka May 23, 2025 14:03

github-actions bot added the stale label Jun 13, 2025

pettyjamesm reviewed Jun 17, 2025

View reviewed changes

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch from 753c41b to ce188eb Compare July 9, 2025 21:41

SongChujun requested a review from pettyjamesm July 9, 2025 23:48

pettyjamesm reviewed Jul 11, 2025

View reviewed changes

raunaqmorarka changed the title ~~Support setting compression_codec table properties for Iceberg Connector~~ Support setting compression-codec table property for Iceberg Jul 15, 2025

raunaqmorarka changed the title ~~Support setting compression-codec table property for Iceberg~~ Support setting compression_codec table property for Iceberg Jul 15, 2025

raunaqmorarka reviewed Jul 15, 2025

View reviewed changes

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch from ce188eb to f0318e7 Compare July 15, 2025 20:13

ebyhr reviewed Jul 15, 2025

View reviewed changes

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch 2 times, most recently from 3c73d7c to 873a2b1 Compare July 18, 2025 15:38

SongChujun requested review from ebyhr, pettyjamesm and raunaqmorarka July 18, 2025 17:38

ebyhr reviewed Jul 21, 2025

View reviewed changes

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch 2 times, most recently from 7e5f587 to 423b13d Compare July 28, 2025 21:14

ebyhr reviewed Jul 28, 2025

View reviewed changes

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

SongChujun force-pushed the iceberg_avro_write_compression_level_support branch from 423b13d to f25b1b4 Compare July 29, 2025 00:38

SongChujun requested a review from ebyhr July 29, 2025 14:12

ebyhr force-pushed the iceberg_avro_write_compression_level_support branch 2 times, most recently from 4e03a75 to 9ff9c3d Compare August 4, 2025 22:05

pettyjamesm approved these changes Aug 6, 2025

View reviewed changes

SongChujun added 2 commits August 7, 2025 07:47

Support setting compression_codec table property in Iceberg

e415abc

Makes compression_codec a table property and drops support for compression_codec session property for simplicity

Remove redundant type qualifications for NestedField and PrimitiveType

237f0a6

ebyhr force-pushed the iceberg_avro_write_compression_level_support branch from 9ff9c3d to 237f0a6 Compare August 6, 2025 22:48

ebyhr merged commit bed467a into trinodb:master Aug 7, 2025
100 of 101 checks passed

github-actions bot added this to the 477 milestone Aug 7, 2025

This was referenced Aug 7, 2025

Fix broken test in Iceberg #26357

Merged

Add Trino 477 release notes #26350

Merged

ebyhr mentioned this pull request Oct 28, 2025

Support reading compression property from table properties for CTAS for Iceberg Connector #25675

Closed

Support setting compression_codec table property for Iceberg #25755

Support setting compression_codec table property for Iceberg #25755

Uh oh!

Conversation

SongChujun commented May 8, 2025 • edited by ebyhr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

raunaqmorarka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SongChujun commented May 8, 2025 •

edited by ebyhr

Loading