JDBC function predicate pushdown with PostgreSQL LIKE pushdown #11045

findepi · 2022-02-15T12:22:45Z

Add support for complex function pushdown in JDBC connectors. This comes
with example implementation for LIKE pushdown in PostgreSQL.

Relates to #18

findepi · 2022-02-15T12:23:06Z

Marking as draft as it's currently based on #7994.

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java

slhmy

👍 This implementation of Predicate Pushdown looks good to me.

Currently I have no objection on these codes, only a few path problem I discovered.

I'll try projection pushdown based on this PR.
Specifically I want to try if I can deal with projection like unsupported_fn(supported_fn(a)) to make supported_fn(a) being pushed down.

If I meet any problem I'll reopen the review.

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/aggregation/RewriteLike.java

findepi · 2022-02-16T09:19:15Z

Specifically I want to try if I can deal with projection like unsupported_fn(supported_fn(a)) to make supported_fn(a) being pushed down.

we're pushing predicates so we operate on conjuncts level

i was thinking about reusing the machinery for projection pushdown as well (e.g. pushdown of substring), but this is a non-goal for now for me

findepi · 2022-02-17T11:59:59Z

rebased on current #7994 state

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java

findepi · 2022-03-02T09:55:36Z

Rebased after #7994 merged, no other changes.

...lugin-toolkit/src/main/java/io/trino/plugin/base/expression/ConnectorExpressionPatterns.java

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java

hashhar

Looks good to me.

Some TODOs could use issues though since we'd want to actually resolve them - most important being lack of aggregation pushdown once expression pushdown is completed.

findepi · 2022-03-02T12:29:08Z

most important being lack of aggregation pushdown once expression pushdown is completed.

covered by #11083

findepi

(some notes)

findepi · 2022-03-02T12:30:13Z

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

@@ -126,17 +126,18 @@ public ConnectorToSqlExpressionTranslator(Session session, PlannerContext planne
            if (call.getFunctionName().getCatalogSchema().isPresent()) {
                return Optional.empty();
            }
+
+            // TODO Support ESCAPE character


Is this included in #11117 @wendigo?

findepi · 2022-03-02T12:30:54Z

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java

+        assertConditionallyPushedDown(
+                getSession(),
+                "SELECT regionkey, sum(nationkey) FROM nation WHERE name LIKE '%N%' GROUP BY regionkey",
+                false, // TODO: hasBehavior(SUPPORTS_PREDICATE_EXPRESSION_PUSHDOWN_WITH_LIKE), --  currently, applyAggregation is not invoked after applyFilter with expression


covered by #11083

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java

findepi · 2022-03-02T12:32:35Z

...n/trino-elasticsearch/src/main/java/io/trino/plugin/elasticsearch/ElasticsearchMetadata.java

@@ -544,7 +544,7 @@ public ConnectorTableProperties getTableProperties(ConnectorSession session, Con
            if (expression instanceof Call) {
                Call call = (Call) expression;
                // TODO Support ESCAPE character when it's pushed down by the engine


@assaf2 i think this was added in #7994. Did you create an issue for this?

losipiuk · 2022-03-02T17:50:13Z

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

@@ -275,7 +276,7 @@ public SqlToConnectorExpressionTranslator(Session session, Map<NodeRef<Expressio
                Optional<ConnectorExpression> value = process(node.getValue());
                Optional<ConnectorExpression> pattern = process(node.getPattern());
                if (value.isPresent() && pattern.isPresent()) {
-                    return Optional.of(new Call(typeOf(node), new FunctionName(LIKE_PATTERN_FUNCTION_NAME), List.of(value.get(), pattern.get())));
+                    return Optional.of(new Call(typeOf(node), StandardFunctions.LIKE_PATTERN_FUNCTION_NAME, List.of(value.get(), pattern.get())));


static import here and in other places?

in this class, SPI's StandardFunctions.LIKE_PATTERN_FUNCTION_NAME and (internal) LikeFunctions.LIKE_PATTERN_FUNCTION_NAME ar eused.

losipiuk · 2022-03-02T18:16:36Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcClient.java

+                getAdditionalPredicate(table.getConstraintExpressions(), split.flatMap(JdbcSplit::getAdditionalPredicate))));
+    }
+
+    protected static Optional<String> getAdditionalPredicate(List<String> constraintExpressions, Optional<String> splitPredicate)


nit: appendConjunct?

no, it doesn't append.

losipiuk · 2022-03-02T18:21:11Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java

                handle.getSortOrder(),
                handle.getLimit(),
                handle.getColumns(),
                handle.getOtherReferencedTables(),
                handle.getNextSyntheticColumnId());

-        return Optional.of(new ConstraintApplicationResult<>(handle, remainingFilter, false));
+        return Optional.of(


feels like exposing private ConstraintApplicationResult(T handle, TupleDomain<ColumnHandle> remainingFilter, Optional<ConnectorExpression> remainingExpression, boolean precalculateStatistics) as public would make this call a bit nicer to read.

This is not a big deal.
Also, the two constructors are supposed not to be used together in the long run. I.e. either connector supports expression pushdown (and uses new constructor) or does not (and uses the old one).
Having a kill switch in the connector (like here), complicates things, as connector's support becomes conditional.

I hope we will remove the switch some day.

gaurav8297 · 2022-03-03T00:04:19Z

core/trino-spi/src/main/java/io/trino/spi/connector/Constraint.java

+        StringJoiner stringJoiner = new StringJoiner(", ", Constraint.class.getSimpleName() + "[", "]");
+        stringJoiner.add("summary=" + summary);
+        stringJoiner.add("expression=" + expression);
+        if (predicate.isPresent()) {


Maybe predicate.ifPresent(p -> stringJoiner.add("predicate=" + p)) and other places too?

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

findepi · 2022-03-03T14:45:09Z

CI #10772

Connectors should be able to refer to known constants for builtin functions. This is especially true for function calls resulting from desugaring, i.e. otherwise undocumented functions.

Add support for complex function pushdown in JDBC connectors. This comes with example implementation for LIKE pushdown in PostgreSQL.

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

ci / test (plugin/trino-kudu) (pull_request) Failing after 54m — test (plugin/trino-kudu)

kudu failure isn't a surprise to anyone, sadly

findepi requested review from martint, wendigo, losipiuk, assaf2 and hashhar February 15, 2022 12:22

findepi added enhancement New feature or request performance labels Feb 15, 2022

findepi force-pushed the findepi/sql-function-pushdown branch from 9b35035 to 8ac5c33 Compare February 15, 2022 15:45

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

findepi force-pushed the findepi/sql-function-pushdown branch 2 times, most recently from 1fce2e0 to a87cc77 Compare February 15, 2022 16:12

findepi mentioned this pull request Feb 15, 2022

ConnectorExpression pushdown #7994

Merged

grantatspothero reviewed Feb 15, 2022

View reviewed changes

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java Outdated Show resolved Hide resolved

findepi force-pushed the findepi/sql-function-pushdown branch from a87cc77 to ba73ff4 Compare February 16, 2022 07:59

slhmy suggested changes Feb 16, 2022

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/aggregation/RewriteLike.java Outdated Show resolved Hide resolved

findepi force-pushed the findepi/sql-function-pushdown branch from 8d58630 to be3bdf6 Compare February 17, 2022 11:59

findepi force-pushed the findepi/sql-function-pushdown branch from be3bdf6 to d152b52 Compare February 17, 2022 12:13

findepi mentioned this pull request Feb 17, 2022

Fix expression predicate pushdown not firing before AddExchanges #11083

Merged

findepi force-pushed the findepi/sql-function-pushdown branch 3 times, most recently from 3b0322c to 915aa8c Compare February 17, 2022 14:42

findepi mentioned this pull request Feb 17, 2022

OR expression pushdown #11086

Merged

findepi force-pushed the findepi/sql-function-pushdown branch 4 times, most recently from 79c95e8 to 536e0bc Compare February 18, 2022 08:59

hashhar reviewed Feb 28, 2022

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java Show resolved Hide resolved

findepi force-pushed the findepi/sql-function-pushdown branch from 536e0bc to 160a994 Compare March 2, 2022 09:55

findepi marked this pull request as ready for review March 2, 2022 09:55

findepi requested a review from hashhar March 2, 2022 09:55

hashhar reviewed Mar 2, 2022

View reviewed changes

...lugin-toolkit/src/main/java/io/trino/plugin/base/expression/ConnectorExpressionPatterns.java Outdated Show resolved Hide resolved

findepi force-pushed the findepi/sql-function-pushdown branch from 160a994 to 31b7545 Compare March 2, 2022 10:35

Fix indentation

50f52ad

hashhar reviewed Mar 2, 2022

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java Show resolved Hide resolved

findepi force-pushed the findepi/sql-function-pushdown branch 2 times, most recently from b209c42 to 7f14b64 Compare March 2, 2022 11:01

hashhar approved these changes Mar 2, 2022

View reviewed changes

findepi commented Mar 2, 2022

View reviewed changes

losipiuk reviewed Mar 2, 2022

View reviewed changes

gaurav8297 reviewed Mar 3, 2022

View reviewed changes

findepi added 3 commits March 3, 2022 16:34

Implement Constraint.toString

8b2497d

Define constant for the LIKE function call

4b9bd8a

Connectors should be able to refer to known constants for builtin functions. This is especially true for function calls resulting from desugaring, i.e. otherwise undocumented functions.

JDBC function predicate pushdown

13e52e8

Add support for complex function pushdown in JDBC connectors. This comes with example implementation for LIKE pushdown in PostgreSQL.

findepi force-pushed the findepi/sql-function-pushdown branch from 7f14b64 to 13e52e8 Compare March 3, 2022 15:35

findepi merged commit a58bb8e into trinodb:master Mar 4, 2022

findepi deleted the findepi/sql-function-pushdown branch March 4, 2022 09:33

github-actions bot added this to the 373 milestone Mar 4, 2022

mosabua mentioned this pull request Mar 4, 2022

Add Trino 373 release notes #11290

Merged

This was referenced Mar 4, 2022

Release notes for 373 #11288

Closed

LIKE predicate pushdown #1580

Open

findepi mentioned this pull request Mar 14, 2022

Make ConnectorExpression JSON serializable #11447

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDBC function predicate pushdown with PostgreSQL LIKE pushdown #11045

JDBC function predicate pushdown with PostgreSQL LIKE pushdown #11045

findepi commented Feb 15, 2022 •

edited

Loading

findepi commented Feb 15, 2022

slhmy left a comment

findepi commented Feb 16, 2022

findepi commented Feb 17, 2022

findepi commented Mar 2, 2022

hashhar left a comment

findepi commented Mar 2, 2022

findepi left a comment

findepi Mar 2, 2022

findepi Mar 2, 2022

findepi Mar 2, 2022

losipiuk Mar 2, 2022

findepi Mar 3, 2022

losipiuk Mar 2, 2022

findepi Mar 3, 2022

losipiuk Mar 2, 2022

findepi Mar 3, 2022

gaurav8297 Mar 3, 2022

findepi Mar 3, 2022

findepi commented Mar 3, 2022

findepi commented Mar 4, 2022

JDBC function predicate pushdown with PostgreSQL LIKE pushdown #11045

JDBC function predicate pushdown with PostgreSQL LIKE pushdown #11045

Conversation

findepi commented Feb 15, 2022 • edited Loading

findepi commented Feb 15, 2022

slhmy left a comment

Choose a reason for hiding this comment

findepi commented Feb 16, 2022

findepi commented Feb 17, 2022

findepi commented Mar 2, 2022

hashhar left a comment

Choose a reason for hiding this comment

findepi commented Mar 2, 2022

findepi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findepi commented Mar 3, 2022

findepi commented Mar 4, 2022

findepi commented Feb 15, 2022 •

edited

Loading