From 4b0408df9012e4912c6c0de04dc738a925248cdb Mon Sep 17 00:00:00 2001 From: Marius Grama Date: Fri, 19 Aug 2022 18:05:22 +0200 Subject: [PATCH] Remove empty `UPDATE` Replace the `UPDATE` plan node with an empty `VALUES` node when the predicate is optimized to `false`. --- .../io/trino/sql/planner/PlanOptimizers.java | 4 +- .../iterative/rule/RemoveEmptyUpdate.java | 72 +++++++++++++++++++ .../iterative/rule/TestRemoveEmptyUpdate.java | 59 +++++++++++++++ .../io/trino/tests/TestMockConnector.java | 4 +- 4 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/RemoveEmptyUpdate.java create mode 100644 core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestRemoveEmptyUpdate.java diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/PlanOptimizers.java b/core/trino-main/src/main/java/io/trino/sql/planner/PlanOptimizers.java index 374cf1f6c216..b7f07d7ffea7 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/PlanOptimizers.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/PlanOptimizers.java @@ -194,6 +194,7 @@ import io.trino.sql.planner.iterative.rule.RemoveEmptyGlobalAggregation; import io.trino.sql.planner.iterative.rule.RemoveEmptyTableExecute; import io.trino.sql.planner.iterative.rule.RemoveEmptyUnionBranches; +import io.trino.sql.planner.iterative.rule.RemoveEmptyUpdate; import io.trino.sql.planner.iterative.rule.RemoveFullSample; import io.trino.sql.planner.iterative.rule.RemoveRedundantDistinctLimit; import io.trino.sql.planner.iterative.rule.RemoveRedundantEnforceSingleRowNode; @@ -864,8 +865,9 @@ public PlanOptimizers( statsCalculator, costCalculator, ImmutableSet.>builder() - // Run RemoveEmptyDeleteRuleSet and RemoveEmptyTableExecute after table scan is removed by PickTableLayout/AddExchanges + // Run RemoveEmptyDeleteRuleSet, RemoveEmptyUpdate and RemoveEmptyTableExecute after table scan is removed by PickTableLayout/AddExchanges .addAll(RemoveEmptyDeleteRuleSet.rules()) + .add(new RemoveEmptyUpdate()) .add(new RemoveEmptyTableExecute()) .build())); diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/RemoveEmptyUpdate.java b/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/RemoveEmptyUpdate.java new file mode 100644 index 000000000000..c8d75cb4bfcd --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/RemoveEmptyUpdate.java @@ -0,0 +1,72 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.sql.planner.iterative.rule; + +import com.google.common.collect.ImmutableList; +import io.trino.matching.Captures; +import io.trino.matching.Pattern; +import io.trino.sql.planner.iterative.Rule; +import io.trino.sql.planner.plan.TableFinishNode; +import io.trino.sql.planner.plan.ValuesNode; +import io.trino.sql.tree.GenericLiteral; +import io.trino.sql.tree.Row; + +import static io.trino.sql.planner.plan.Patterns.emptyValues; +import static io.trino.sql.planner.plan.Patterns.exchange; +import static io.trino.sql.planner.plan.Patterns.source; +import static io.trino.sql.planner.plan.Patterns.tableFinish; +import static io.trino.sql.planner.plan.Patterns.update; + +/** + * If the predicate for an update is optimized to false, the target table + * scan of the update will be replaced with an empty values node. + * This type of plan cannot be executed and is meaningless anyway, so we + * replace the entire plan node with a `values` node. + *

+ * Transforms + * 

+ *  - TableFinish
+ *    - Exchange
+ *      - Update
+ *        - empty Values
+ *
+ * into + * 
+ *  - Values (0)
+ *
+ */ +public class RemoveEmptyUpdate + implements Rule +{ + private static final Pattern PATTERN = tableFinish() + .with(source().matching(exchange() + .with(source().matching(update() + .with(source().matching(emptyValues())))))); + + @Override + public Pattern getPattern() + { + return PATTERN; + } + + @Override + public Result apply(TableFinishNode node, Captures captures, Context context) + { + return Result.ofPlanNode( + new ValuesNode( + node.getId(), + node.getOutputSymbols(), + ImmutableList.of(new Row(ImmutableList.of(new GenericLiteral("BIGINT", "0")))))); + } +} diff --git a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestRemoveEmptyUpdate.java b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestRemoveEmptyUpdate.java new file mode 100644 index 000000000000..4d903723a79b --- /dev/null +++ b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestRemoveEmptyUpdate.java @@ -0,0 +1,59 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.sql.planner.iterative.rule; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.trino.metadata.TableHandle; +import io.trino.plugin.tpch.TpchTableHandle; +import io.trino.plugin.tpch.TpchTransactionHandle; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.type.BigintType; +import io.trino.sql.planner.assertions.PlanMatchPattern; +import io.trino.sql.planner.iterative.rule.test.BaseRuleTest; +import org.testng.annotations.Test; + +import static io.trino.testing.TestingHandles.TEST_CATALOG_HANDLE; + +public class TestRemoveEmptyUpdate + extends BaseRuleTest +{ + @Test + public void testRuleDoesNotFireOnTableScan() + { + tester().assertThat(new RemoveEmptyUpdate()) + .on(p -> p.tableUpdate( + new SchemaTableName("sch", "tab"), + p.tableScan( + new TableHandle(TEST_CATALOG_HANDLE, new TpchTableHandle("sf1", "nation", 1.0), TpchTransactionHandle.INSTANCE), + ImmutableList.of(), + ImmutableMap.of()), + p.symbol("a", BigintType.BIGINT), + ImmutableList.of(p.symbol("a", BigintType.BIGINT)))) + .doesNotFire(); + } + + @Test + public void testRuleFiresWhenAppliedOnEmptyValuesNode() + { + tester().assertThat(new RemoveEmptyUpdate()) + .on(p -> p.tableUpdate( + new SchemaTableName("sch", "tab"), + p.values(), + p.symbol("a", BigintType.BIGINT), + ImmutableList.of(p.symbol("a", BigintType.BIGINT)))) + .matches( + PlanMatchPattern.values(ImmutableMap.of("a", 0))); + } +} diff --git a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java index 05d8d5d35e26..af765d1c66cd 100644 --- a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java +++ b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java @@ -214,9 +214,7 @@ public void testUpdate() assertQuery("SELECT count(*) FROM mock.default.nation WHERE name = 'ALGERIA'", "SELECT 1"); assertUpdate("UPDATE mock.default.nation SET name = 'ALGERIA'", 25); assertUpdate("UPDATE mock.default.nation SET name = 'ALGERIA' WHERE nationkey = 1", 1); - assertThatThrownBy(() -> assertUpdate("UPDATE mock.default.nation SET name = 'x' WHERE false", 0)) - // TODO https://github.com/trinodb/trino/issues/8855 - UPDATE with WHERE false currently is not supported - .hasMessage("Invalid descendant for DeleteNode or UpdateNode: io.trino.sql.planner.plan.ExchangeNode"); + assertUpdate("UPDATE mock.default.nation SET name = 'x' WHERE false", 0); // Mock connector only pretends support for UPDATE, it does not manipulate any data assertQuery("SELECT count(*) FROM mock.default.nation WHERE name = 'ALGERIA'", "SELECT 1"); }