Skip to content

Conversation

@mdesmet
Copy link
Contributor

@mdesmet mdesmet commented Dec 16, 2024

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.
( ) Release notes are required, with the following suggested text:

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

@cla-bot cla-bot bot added the cla-signed label Dec 16, 2024
@github-actions github-actions bot added the iceberg Iceberg connector label Dec 16, 2024
@findinpath
Copy link
Contributor

Please add the description to the PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you confirm Iceberg library if we can simplify this transform logic? https://github.com/apache/iceberg/tree/main/api/src/main/java/org/apache/iceberg/expressions

I mean updating ExpressionConverter like this instead of converting the value in applyFilter.

greaterThanOrEqual(Expressions.day("ts"), Literal.of(ts).to(Types.DateType.get()))

Just an idea, I haven't verified the feasibility yet.

@mdesmet mdesmet force-pushed the feat/iceberg-partition-pushdown branch from 6f7ad80 to eb4280e Compare December 31, 2024 08:34
Arguments.of("truncate(d, 4)", 2));
}

@ParameterizedTest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK the use of Parameterized tests is discouraged in trinodb/trino.
Use io.trino.plugin.iceberg.BaseIcebergConnectorTest#testDecimal as reference to change the code.

@findinpath
Copy link
Contributor

Update the PR description to showcase that we're dealing with a test to showcase the partial pushdown of the partition transform functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed iceberg Iceberg connector

Development

Successfully merging this pull request may close these issues.

3 participants