OR expression pushdown #11086

findepi · 2022-02-17T14:52:41Z

This includes OR pushdown framework for JDBC connectors, implemented
initially for PostgreSQL.
Along with this, there comes support for comparison expressions,
effective literals, generic literals (used e.g. for BIGINT), to support
a minimal viable usage example for the OR pushdown.

Relates to #18

findepi · 2022-02-17T14:52:53Z

Currently based on #11045

findepi · 2022-03-04T09:51:26Z

Rebased after #11045 merged.

findepi · 2022-03-04T12:28:55Z

CI ci / test (plugin/trino-hive, test-failure-recovery) - #11196 / #10631

(green otherwise)

core/trino-main/src/main/java/io/trino/sql/planner/ConnectorExpressionTranslator.java

assaf2 · 2022-03-07T15:31:55Z

core/trino-main/src/main/java/io/trino/sql/planner/ConnectorExpressionTranslator.java

+        @Override
+        protected Optional<ConnectorExpression> visitCast(Cast node, Void context)
+        {
+            if (isEffectivelyLiteral(plannerContext, session, node)) {


add if (!isComplexExpressionPushdown(session)) {

Not sure about this. What's complex about a constant? The explicit literals don't have such a check.

core/trino-main/src/main/java/io/trino/sql/planner/ConnectorExpressionTranslator.java

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/expression/RewriteOr.java

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/expression/RewriteComparison.java

findepi · 2022-03-09T11:11:52Z

(just rebased)

findepi · 2022-03-09T12:54:00Z

AC. Changes as fixups (inline) + new commit Remove redundant argument being field.

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java

hashhar · 2022-03-09T13:12:28Z

reminder to squash fixups before merge

findepi · 2022-03-09T15:36:02Z

squashed, rebased, resolved conflicts and added Simplify TestConnectorExpressionTranslator commit to do a TODO comment i am adding in an earlier commit

This includes OR pushdown framework for JDBC connectors, implemented initially for PostgreSQL. Along with this, there comes support for comparison expressions to support a minimal viable usage example for the OR pushdown.

Use `assertTranslationRoundTrips` where possible... i.e. always.

hashhar

LGTM from me.

hashhar · 2022-06-30T14:26:12Z

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java

+    {
+        assertThat(query("SELECT * FROM nation WHERE nationkey != 3 OR regionkey = 4")).isFullyPushedDown();
+        assertThat(query("SELECT * FROM nation WHERE nationkey != 3 OR regionkey != 4")).isFullyPushedDown();
+        assertThat(query("SELECT * FROM nation WHERE name = 'ALGERIA' OR regionkey = 4")).isFullyPushedDown();


Does the expression rewrite depend on how predicates are ordered/grouped. I see that if add another case like

assertThat(query("SELECT * FROM nation WHERE (name = 'ALGERIA' AND regionkey != 10) OR (regionkey = 4 AND name != 'FOOBAR')")).isFullyPushedDown();

it fails and we get a ScanFilterNode.

cc: @wendigo

That's unrelated to OR. That's because != isn't subsumed:

trino:tpch> EXPLAIN SELECT * FROM nation WHERE name != 'FOOBAR' ; Query Plan -------------------------------------------------------------------------------------------------------------------------------- .... Fragment 1 [SOURCE] Output layout: [nationkey, name, regionkey, comment] Output partitioning: SINGLE [] ScanFilter[table = postgresql:tpch.nation tpch.nation, filterPredicate = ("name" <> CAST('FOOBAR' AS varchar(25)))] Layout: [nationkey:bigint, name:varchar(25), regionkey:bigint, comment:varchar(152)] Estimates: {rows: 25 (3.13kB), cpu: 3.13k, memory: 0B, network: 0B}/{rows: ? (?), cpu: 6.25k, memory: 0B, network: 0B} nationkey := nationkey:bigint:int8 regionkey := regionkey:bigint:int8 name := name:varchar(25):varchar comment := comment:varchar(152):varchar

cla-bot bot added the cla-signed label Feb 17, 2022

findepi force-pushed the findepi/sql-or-pushdown branch 2 times, most recently from 01e25d6 to fedfff7 Compare February 17, 2022 16:23

findepi mentioned this pull request Feb 17, 2022

ConnectorExpression pushdown #7994

Merged

findepi force-pushed the findepi/sql-or-pushdown branch from fedfff7 to f81ceb6 Compare February 18, 2022 09:53

assaf2 mentioned this pull request Feb 20, 2022

Extend expression pushdown support #9506

Open

findepi mentioned this pull request Feb 21, 2022

DSL for JDBC expression rewrites #11125

Merged

findepi force-pushed the findepi/sql-or-pushdown branch from 70390c5 to 745dfc1 Compare March 4, 2022 09:51

findepi requested review from losipiuk, assaf2, hashhar and martint March 4, 2022 09:51

assaf2 reviewed Mar 7, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/sql/planner/ConnectorExpressionTranslator.java Show resolved Hide resolved

assaf2 reviewed Mar 7, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/sql/planner/ConnectorExpressionTranslator.java Show resolved Hide resolved

hashhar reviewed Mar 8, 2022

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/expression/RewriteOr.java Outdated Show resolved Hide resolved

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java Outdated Show resolved Hide resolved

losipiuk reviewed Mar 8, 2022

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/expression/RewriteComparison.java Outdated Show resolved Hide resolved

losipiuk reviewed Mar 8, 2022

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/expression/RewriteComparison.java Outdated Show resolved Hide resolved

losipiuk approved these changes Mar 8, 2022

View reviewed changes

findepi force-pushed the findepi/sql-or-pushdown branch from 745dfc1 to 5a911f8 Compare March 9, 2022 11:11

findepi force-pushed the findepi/sql-or-pushdown branch from 5a911f8 to 042c452 Compare March 9, 2022 12:53

hashhar reviewed Mar 9, 2022

View reviewed changes

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java Show resolved Hide resolved

hashhar approved these changes Mar 9, 2022

View reviewed changes

losipiuk approved these changes Mar 9, 2022

View reviewed changes

findepi mentioned this pull request Mar 9, 2022

Translate IN predicate to connector expression #11396

Closed

findepi added 5 commits March 9, 2022 16:17

Remove redundant ImmutableList.copyOf call

5cd3412

Translate all effective literals to connector expression

e289c43

Translate comparisons to connnector expression

2655af6

Translate logical expression to connector expression

7f43c48

Replace potentially lossy cast with checked conversion

d764273

findepi force-pushed the findepi/sql-or-pushdown branch from 042c452 to 64988d6 Compare March 9, 2022 15:35

findepi added 3 commits March 9, 2022 16:44

OR expression pushdown into PostgreSQL

1a883e6

This includes OR pushdown framework for JDBC connectors, implemented initially for PostgreSQL. Along with this, there comes support for comparison expressions to support a minimal viable usage example for the OR pushdown.

Remove redundant argument being field

52be19c

Simplify TestConnectorExpressionTranslator

5b65402

Use `assertTranslationRoundTrips` where possible... i.e. always.

findepi force-pushed the findepi/sql-or-pushdown branch from 64988d6 to 5b65402 Compare March 9, 2022 15:44

hashhar approved these changes Mar 10, 2022

View reviewed changes

findepi merged commit 6b6c938 into trinodb:master Mar 10, 2022

findepi deleted the findepi/sql-or-pushdown branch March 10, 2022 07:55

findepi mentioned this pull request Mar 10, 2022

Release notes for 374 #11402

Closed

github-actions bot added this to the 374 milestone Mar 10, 2022

mosabua mentioned this pull request Mar 10, 2022

Add Trino 374 release notes #11417

Merged

findepi mentioned this pull request Mar 11, 2022

Fix TestHivePlans after logical merge conflict #11433

Merged

hashhar reviewed Jun 30, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OR expression pushdown #11086

OR expression pushdown #11086

findepi commented Feb 17, 2022 •

edited

Loading

findepi commented Feb 17, 2022

findepi commented Mar 4, 2022

findepi commented Mar 4, 2022 •

edited

Loading

assaf2 Mar 7, 2022

findepi Mar 8, 2022

findepi commented Mar 9, 2022

findepi commented Mar 9, 2022

hashhar commented Mar 9, 2022

findepi commented Mar 9, 2022

hashhar left a comment

hashhar Jun 30, 2022 •

edited

Loading

findepi Jul 20, 2022

OR expression pushdown #11086

OR expression pushdown #11086

Conversation

findepi commented Feb 17, 2022 • edited Loading

findepi commented Feb 17, 2022

findepi commented Mar 4, 2022

findepi commented Mar 4, 2022 • edited Loading

assaf2 Mar 7, 2022

Choose a reason for hiding this comment

findepi Mar 8, 2022

Choose a reason for hiding this comment

findepi commented Mar 9, 2022

findepi commented Mar 9, 2022

hashhar commented Mar 9, 2022

findepi commented Mar 9, 2022

hashhar left a comment

Choose a reason for hiding this comment

hashhar Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

findepi Jul 20, 2022

Choose a reason for hiding this comment

findepi commented Feb 17, 2022 •

edited

Loading

findepi commented Mar 4, 2022 •

edited

Loading

hashhar Jun 30, 2022 •

edited

Loading