[HUDI-9480] Add Expression Index Support by voonhous · Pull Request #25 · onehouseinc/trino

voonhous · 2025-05-30T13:05:14Z

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`)

yihua · 2025-06-13T04:17:47Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/HudiConfig.java

Could the user also explicitly disable the expression index if there is an issue or regression? If so, let's revise the config description.

Yes, all configurations that are added can be explicitly disabled.

yihua · 2025-06-13T05:06:15Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Use Preconditions#checkArgument

yihua · 2025-06-13T05:07:15Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

nit: pull arguments.size() == 1 out of the condition check and put it into Preconditions#checkArgument; similar for other else if branches.

yihua · 2025-06-13T05:09:36Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Could unit tests be added on this class given this is critical?

yihua · 2025-06-13T05:11:31Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Is there any inefficiency of having two levels of expression during predicate evaluation, instead of sth like a Predicates.neq?

This should be negligible as it will only be evaluated at most once. This is written the way it is as Predicates belongs to Hudi project scope: org.apache.hudi.expression.Predicates and does not have the neq function.

yihua · 2025-06-13T05:25:13Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

In the case of "IN"(column, ARRAY[val1, val2...]), does the second argument only contain ARRAY[val1, val2...] and Hudi's Predicates supports it?

Or should ARRAY[val1, val2...] be unfolded to adapt to Hudi's Predicates?

Addressed this in the new commit. However, IN operators are not used for expression index file skipping for now, i.e. not used in io.trino.plugin.hudi.expression.HudiColumnStatsIndexEvaluator

yihua · 2025-06-13T05:30:53Z

...trino-hudi/src/main/java/io/trino/plugin/hudi/expression/HudiTrinoDayFunctionExpression.java

String name can be remove as not used.

yihua · 2025-06-13T05:32:12Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Use getName for matching? A static import of TRINO_FN_NAME can easily make it confusing.

Suggested change

else if (HudiTrinoDayFunctionExpression.TRINO_FN_NAME.equalsIgnoreCase(functionName.getName())) {

else if (HudiTrinoDayFunctionExpression.getName().equalsIgnoreCase(functionName.getName())) {

This is called before initializing an instance, so, a static function is required regardless. Will change TRINO_FN_NAME to getTrinoFnName().

yihua · 2025-06-13T05:32:27Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Similar for the rest.

yihua · 2025-06-13T05:34:34Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Does this general expression get evaluated without problem?

This is suppose to be a catch-all similar to the default function which will be evaluated, but will not be used downstream.

I am currently trying to get nested types to be evaluated into this branch.

TLDR: There are some issues that will cause this general expression to not be evaluated/reached.

yihua · 2025-06-13T05:36:33Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

For the specific functions for expression index, should they be put into a separate method for handling so it's more clear?

Was thinking of this, but not really sure how to do it as initialisation of the concrete implementation is currently a one liner. It is concise and readable and i can't think of any better way to make it more readable.

yihua · 2025-06-13T05:38:13Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Does this collection only contain one or no entry based on the logic right now?

tryAddAsCandidate will attempt to add candidates. It will contain contain at least one or no entry based on the logic.

The current smoke tests that were added are example/cases where there are more than one entry.

yihua · 2025-06-13T05:39:16Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

Could the ExpressionConverter be a singleton and expose a method to convert the expression, and let the upper layer to handle whether they should be used with expression index?

yihua · 2025-06-13T05:42:14Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/HudiMetadata.java

So basically, all the predicates or expressions are still kept and re-evaluated by Trino, correct?

Yes, what we have added is outside the scope of what uses, so, it will not affect Trino's optimizations.

yihua · 2025-06-13T05:47:44Z

plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/expression/ExpressionConverter.java

If the predicate is in the form of day(ts) > 1 and day(ts) < 10, it seems like it is not a candidate for the expression index. Only day(ts) > 1 is. Is that the case, i.e., complex or nested predicate with expression wrapped might not be eligible for expression index pruning?

From my tests, nested predicates can be handled.

voonhous · 2025-06-20T13:02:43Z

Resolved merge conflicts, going through the comments incrementally.

- Fix rebase errors - Added unit tests for ExpressionConverter - Fix checkstyle - Address comments

github-actions bot added the hudi label May 30, 2025

voonhous force-pushed the expression_index branch from 75effda to f79b19d Compare May 30, 2025 13:10

voonhous marked this pull request as draft May 30, 2025 13:11

voonhous force-pushed the expression_index branch from 1371d38 to 15d408f Compare June 6, 2025 08:29

voonhous marked this pull request as ready for review June 6, 2025 08:30

voonhous force-pushed the expression_index branch from 11c470a to 5a58655 Compare June 6, 2025 11:41

voonhous closed this Jun 6, 2025

voonhous reopened this Jun 6, 2025

yihua reviewed Jun 13, 2025

View reviewed changes

voonhous force-pushed the expression_index branch 2 times, most recently from 149e93d to f4ee420 Compare June 20, 2025 13:02

voonhous force-pushed the expression_index branch from f4ee420 to c2ccd5d Compare June 20, 2025 13:54

voonhous closed this Jun 23, 2025

voonhous reopened this Jun 23, 2025

[HUDI-9472] Added working PoC for Expression Index

0dc5a64

voonhous force-pushed the expression_index branch from 07472af to c13b6e6 Compare June 25, 2025 10:54

Added fixes:

ba37413

- Fix rebase errors - Added unit tests for ExpressionConverter - Fix checkstyle - Address comments

voonhous force-pushed the expression_index branch 2 times, most recently from 81d441d to 769e0a5 Compare June 26, 2025 09:05

Added unit test for HudiColumnStatsIndexEvaluator

338f18d

voonhous force-pushed the expression_index branch from 769e0a5 to 338f18d Compare June 26, 2025 11:03

hudi-bot mentioned this pull request Nov 30, 2025

Add Expression Index Support for Trino connector apache/hudi#17037

Open

	else if (HudiTrinoDayFunctionExpression.TRINO_FN_NAME.equalsIgnoreCase(functionName.getName())) {
	else if (HudiTrinoDayFunctionExpression.getName().equalsIgnoreCase(functionName.getName())) {

Comments

Conversation

voonhous commented May 30, 2025

Description

Additional context and related issues

Release notes

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!

voonhous Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

voonhous Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

voonhous Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

voonhous Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

voonhous commented Jun 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

voonhous Jun 23, 2025 •

edited

Loading

voonhous Jun 23, 2025 •

edited

Loading

voonhous Jun 23, 2025 •

edited

Loading

voonhous Jun 23, 2025 •

edited

Loading