BETWEEN connector expression translation

[assaf2]

Translate BETWEEN predicate to connector expression and rewrite it in PostgreSQL connector

[findepi]

Do we need a new function?
I would prefer to model this as $and(x >= y, x <= z).

BETWEEN has important property that it evaluates the LHS value once, but that doesn't matter for deterministic predicates, and we don't want to handle non-deterministic predicates in connector expressions.

(That's also why we could model boolean AND as an ordinary function. Lazy-evaluation (or lack thereof) isn't defined for connector expressions.)

[assaf2]

After writing this PR, I've noticed that BETWEEN is already modeled the way you said at DomainTranslator, so the implementation will only be reflected in the projection pushdown. Do you think I should implement the conversion to an $and right now or just wait with this PR?

[findepi]

What for would you want to wait for here?

[assaf2]

I thought I might need to wait for a connector that will actually use this, but seems like we're ok with just adding this (with tests) so I'll continue

[findepi]

PostgreSQL would use this already, so you can try add tests there.
see io.trino.plugin.postgresql.TestPostgreSqlConnectorTest#test*PredicatePushdown

[assaf2]

GREATER_THAN_OR_EQUAL and LESS_THAN_OR_EQUAL are currently not supported in PostgreSQL:

trino/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java

Lines 302 to 303 in 8b6888c

	// TODO allow all comparison operators for numeric types
	.add(new RewriteComparison(RewriteComparison.ComparisonOperator.EQUAL, RewriteComparison.ComparisonOperator.NOT_EQUAL))

[findepi]

i guess we should just add them there?

[assaf2]

Since:

1. connectorExpressionRewriter is created at the constructor.
2. RewriteComparison is part of io.trino.plugin.jdbc.expression and not of io.trino.plugin.postgresql.
3. My understanding of this:
   

   trino/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java

   Lines 991 to 998 in 8b6888c

   	// PostgreSQL is case sensitive by default, but orders varchars differently
   	JoinCondition.Operator operator = joinCondition.getOperator();
   	switch (operator) {
   	case LESS_THAN:
   	case LESS_THAN_OR_EQUAL:
   	case GREATER_THAN:
   	case GREATER_THAN_OR_EQUAL:
   	return isEnableStringPushdownWithCollate(session);

I think that my best option is to add a subclass to RewriteComparison at io.trino.plugin.postgresql and return Optional.empty() on rewrite if the operator is one of these and io.trino.plugin.postgresql#isEnableStringPushdownWithCollate(context.getSession()) == false. Maybe I can also check if the operands are of String type (how do you think I should do that?).
Another option is to create two connectorExpressionRewriters (with these operators and without) and dynamically choose between the two according to the result of isEnableStringPushdownWithCollate.
WDYT?

[findepi]

cc @hashhar on the above

in any case, let's split the engine part (handling of BETWEEN)
and PostgreSQL part (handling of inequalities) into separate PRs.

thanks

[findepi]

CI #11016