From f14f303d1307e6b37809ac01392a89981fbae575 Mon Sep 17 00:00:00 2001 From: Gary Helmling Date: Thu, 26 Jun 2025 22:55:36 -0700 Subject: [PATCH] Make rowid optional in DELETE query plan (#25284) Summary: Pull Request resolved: https://github.com/prestodb/presto/pull/25284 Currently, the $row_id column is automatically inserted into the output variables for DELETE queries by QueryPlanner. If a connector does not actually use $row_id to implement DELETE, then we should not require it. This makes $row_id optional. If the Optional is empty, then we don't need to project the output variable. Differential Revision: D76325048 --- .../facebook/presto/hive/HiveMetadata.java | 4 +- .../iceberg/IcebergAbstractMetadata.java | 8 ++-- .../facebook/presto/kudu/KuduMetadata.java | 4 +- .../metadata/DelegatingMetadataManager.java | 8 ++-- .../facebook/presto/metadata/Metadata.java | 4 +- .../presto/metadata/MetadataManager.java | 8 ++-- .../sql/planner/LocalExecutionPlanner.java | 6 ++- .../presto/sql/planner/QueryPlanner.java | 48 +++++++++++-------- .../PruneUnreferencedOutputs.java | 2 +- .../optimizations/PushdownSubfields.java | 2 +- .../presto/sql/planner/plan/UpdateNode.java | 8 ++-- .../sanity/ValidateDependenciesChecker.java | 7 ++- .../presto/metadata/AbstractMockMetadata.java | 4 +- .../iterative/rule/test/PlanBuilder.java | 2 +- .../core/presto_protocol_core.h | 2 +- .../presto_protocol/tests/DeleteTest.cpp | 3 +- .../spi/connector/ConnectorMetadata.java | 29 +++++++++-- .../ClassLoaderSafeConnectorMetadata.java | 16 +++++++ .../facebook/presto/spi/plan/DeleteNode.java | 8 ++-- 19 files changed, 114 insertions(+), 59 deletions(-) diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java b/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java index d2427834ad2f3..8f7e202fde63f 100644 --- a/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java +++ b/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java @@ -2611,9 +2611,9 @@ public ConnectorDeleteTableHandle beginDelete(ConnectorSession session, Connecto } @Override - public ColumnHandle getDeleteRowIdColumnHandle(ConnectorSession session, ConnectorTableHandle tableHandle) + public Optional getDeleteRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle) { - return updateRowIdHandle(); + return Optional.of(updateRowIdHandle()); } @Override diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java index 1166a0ef4ab85..035d7bcccd5b2 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java @@ -695,9 +695,9 @@ private void handleFinishData(CommitTaskData task, Table icebergTable, Partition } @Override - public ColumnHandle getDeleteRowIdColumnHandle(ConnectorSession session, ConnectorTableHandle tableHandle) + public Optional getDeleteRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle) { - return IcebergColumnHandle.create(ROW_POSITION, typeManager, REGULAR); + return Optional.of(IcebergColumnHandle.create(ROW_POSITION, typeManager, REGULAR)); } @Override @@ -1271,7 +1271,7 @@ else if (tableVersion.getVersionExpressionType() instanceof VarcharType) { * @return A column handle for the Row ID update column. */ @Override - public ColumnHandle getUpdateRowIdColumnHandle(ConnectorSession session, ConnectorTableHandle tableHandle, List updatedColumns) + public Optional getUpdateRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle, List updatedColumns) { List unmodifiedColumns = new ArrayList<>(); unmodifiedColumns.add(ROW_POSITION); @@ -1287,7 +1287,7 @@ public ColumnHandle getUpdateRowIdColumnHandle(ConnectorSession session, Connect } } NestedField field = NestedField.required(UPDATE_ROW_DATA.getId(), UPDATE_ROW_DATA.getColumnName(), Types.StructType.of(unmodifiedColumns)); - return IcebergColumnHandle.create(field, typeManager, SYNTHESIZED); + return Optional.of(IcebergColumnHandle.create(field, typeManager, SYNTHESIZED)); } @Override diff --git a/presto-kudu/src/main/java/com/facebook/presto/kudu/KuduMetadata.java b/presto-kudu/src/main/java/com/facebook/presto/kudu/KuduMetadata.java index fa9744d560681..b5f63ae0b1323 100755 --- a/presto-kudu/src/main/java/com/facebook/presto/kudu/KuduMetadata.java +++ b/presto-kudu/src/main/java/com/facebook/presto/kudu/KuduMetadata.java @@ -368,9 +368,9 @@ public Optional finishCreateTable(ConnectorSession sess } @Override - public ColumnHandle getDeleteRowIdColumnHandle(ConnectorSession session, ConnectorTableHandle tableHandle) + public Optional getDeleteRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle) { - return KuduColumnHandle.ROW_ID_HANDLE; + return Optional.of(KuduColumnHandle.ROW_ID_HANDLE); } @Override diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java index 9c68582cf9042..5fa2b5e23a08d 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java @@ -361,15 +361,15 @@ public Optional finishInsert( } @Override - public ColumnHandle getDeleteRowIdColumnHandle(Session session, TableHandle tableHandle) + public Optional getDeleteRowIdColumn(Session session, TableHandle tableHandle) { - return delegate.getDeleteRowIdColumnHandle(session, tableHandle); + return delegate.getDeleteRowIdColumn(session, tableHandle); } @Override - public ColumnHandle getUpdateRowIdColumnHandle(Session session, TableHandle tableHandle, List updatedColumns) + public Optional getUpdateRowIdColumn(Session session, TableHandle tableHandle, List updatedColumns) { - return delegate.getUpdateRowIdColumnHandle(session, tableHandle, updatedColumns); + return delegate.getUpdateRowIdColumn(session, tableHandle, updatedColumns); } @Override diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java index 52a9948dfa0e1..4658215860f75 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java @@ -309,12 +309,12 @@ public interface Metadata /** * Get the row ID column handle used with UpdatablePageSource#deleteRows. */ - ColumnHandle getDeleteRowIdColumnHandle(Session session, TableHandle tableHandle); + Optional getDeleteRowIdColumn(Session session, TableHandle tableHandle); /** * Get the row ID column handle used with UpdatablePageSource. */ - ColumnHandle getUpdateRowIdColumnHandle(Session session, TableHandle tableHandle, List updatedColumns); + Optional getUpdateRowIdColumn(Session session, TableHandle tableHandle, List updatedColumns); /** * @return whether delete without table scan is supported diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java index aae8ddebf04a7..ab5fb961c103c 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java @@ -886,19 +886,19 @@ public Optional finishInsert(Session session, InsertTab } @Override - public ColumnHandle getDeleteRowIdColumnHandle(Session session, TableHandle tableHandle) + public Optional getDeleteRowIdColumn(Session session, TableHandle tableHandle) { ConnectorId connectorId = tableHandle.getConnectorId(); ConnectorMetadata metadata = getMetadata(session, connectorId); - return metadata.getDeleteRowIdColumnHandle(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle()); + return metadata.getDeleteRowIdColumn(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle()); } @Override - public ColumnHandle getUpdateRowIdColumnHandle(Session session, TableHandle tableHandle, List updatedColumns) + public Optional getUpdateRowIdColumn(Session session, TableHandle tableHandle, List updatedColumns) { ConnectorId connectorId = tableHandle.getConnectorId(); ConnectorMetadata metadata = getMetadata(session, connectorId); - return metadata.getUpdateRowIdColumnHandle(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle(), updatedColumns); + return metadata.getUpdateRowIdColumn(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle(), updatedColumns); } @Override diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java index 757c35e9854a2..c67372951387c 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java @@ -2929,8 +2929,10 @@ public PhysicalOperation visitTableFinish(TableFinishNode node, LocalExecutionPl public PhysicalOperation visitDelete(DeleteNode node, LocalExecutionPlanContext context) { PhysicalOperation source = node.getSource().accept(this, context); - - OperatorFactory operatorFactory = new DeleteOperatorFactory(context.getNextOperatorId(), node.getId(), source.getLayout().get(node.getRowId()), tableCommitContextCodec); + if (!node.getRowId().isPresent()) { + throw new PrestoException(NOT_SUPPORTED, "DELETE is not supported by this connector"); + } + OperatorFactory operatorFactory = new DeleteOperatorFactory(context.getNextOperatorId(), node.getId(), source.getLayout().get(node.getRowId().get()), tableCommitContextCodec); Map layout = ImmutableMap.builder() .put(node.getOutputVariables().get(0), 0) diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java index 71149793bd3d8..7199daeb239f1 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java @@ -253,8 +253,6 @@ public DeleteNode plan(Delete node) { RelationType descriptor = analysis.getOutputDescriptor(node.getTable()); TableHandle handle = analysis.getTableHandle(node.getTable()); - ColumnHandle rowIdHandle = metadata.getDeleteRowIdColumnHandle(session, handle); - Type rowIdType = metadata.getColumnMetadata(session, handle, rowIdHandle).getType(); // add table columns ImmutableList.Builder outputVariablesBuilder = ImmutableList.builder(); @@ -268,11 +266,16 @@ public DeleteNode plan(Delete node) } // add rowId column - Field rowIdField = Field.newUnqualified(node.getLocation(), Optional.empty(), rowIdType); - VariableReferenceExpression rowIdVariable = variableAllocator.newVariable(getSourceLocation(node), "$rowId", rowIdField.getType()); - outputVariablesBuilder.add(rowIdVariable); - columns.put(rowIdVariable, rowIdHandle); - fields.add(rowIdField); + Optional rowIdHandle = metadata.getDeleteRowIdColumn(session, handle); + Optional rowIdField = Optional.empty(); + if (rowIdHandle.isPresent()) { + Type rowIdType = metadata.getColumnMetadata(session, handle, rowIdHandle.get()).getType(); + rowIdField = Optional.of(Field.newUnqualified(node.getLocation(), Optional.empty(), rowIdType)); + VariableReferenceExpression rowIdVariable = variableAllocator.newVariable(getSourceLocation(node), "$rowId", rowIdType); + outputVariablesBuilder.add(rowIdVariable); + columns.put(rowIdVariable, rowIdHandle.get()); + fields.add(rowIdField.get()); + } // create table scan List outputVariables = outputVariablesBuilder.build(); @@ -290,12 +293,14 @@ public DeleteNode plan(Delete node) } // create delete node - VariableReferenceExpression rowId = new VariableReferenceExpression(Optional.empty(), builder.translate(new FieldReference(relationPlan.getDescriptor().indexOf(rowIdField))).getName(), rowIdField.getType()); + PlanBuilder finalBuilder = builder; + Optional rowId = rowIdField.map(f -> + new VariableReferenceExpression(Optional.empty(), finalBuilder.translate(new FieldReference(relationPlan.getDescriptor().indexOf(f))).getName(), f.getType())); List deleteNodeOutputVariables = ImmutableList.of( variableAllocator.newVariable("partialrows", BIGINT), variableAllocator.newVariable("fragment", VARBINARY)); - return new DeleteNode(getSourceLocation(node), idAllocator.getNextId(), builder.getRoot(), rowId, deleteNodeOutputVariables, Optional.empty()); + return new DeleteNode(getSourceLocation(node), idAllocator.getNextId(), finalBuilder.getRoot(), rowId, deleteNodeOutputVariables, Optional.empty()); } public UpdateNode plan(Update node) @@ -312,8 +317,6 @@ public UpdateNode plan(Update node) .map(Map.Entry::getValue) .collect(toImmutableList()); handle = metadata.beginUpdate(session, handle, updatedColumns); - ColumnHandle rowIdHandle = metadata.getUpdateRowIdColumnHandle(session, handle, updatedColumns); - Type rowIdType = metadata.getColumnMetadata(session, handle, rowIdHandle).getType(); List targetColumnNames = node.getAssignments().stream() .map(assignment -> assignment.getName().getValue()) @@ -338,11 +341,16 @@ public UpdateNode plan(Update node) List orderedColumnValues = orderedColumnValuesBuilder.build(); // add rowId column - Field rowIdField = Field.newUnqualified(node.getLocation(), Optional.empty(), rowIdType); - VariableReferenceExpression rowIdVariable = variableAllocator.newVariable(getSourceLocation(node), "$rowId", rowIdField.getType()); - outputVariablesBuilder.add(rowIdVariable); - columns.put(rowIdVariable, rowIdHandle); - fields.add(rowIdField); + Optional rowIdHandle = metadata.getUpdateRowIdColumn(session, handle, updatedColumns); + Optional rowIdField = Optional.empty(); + if (rowIdHandle.isPresent()) { + Type rowIdType = metadata.getColumnMetadata(session, handle, rowIdHandle.get()).getType(); + rowIdField = Optional.of(Field.newUnqualified(node.getLocation(), Optional.empty(), rowIdType)); + VariableReferenceExpression rowIdVariable = variableAllocator.newVariable(getSourceLocation(node), "$rowId", rowIdType); + outputVariablesBuilder.add(rowIdVariable); + columns.put(rowIdVariable, rowIdHandle.get()); + fields.add(rowIdField.get()); + } // create table scan List outputVariables = outputVariablesBuilder.build(); @@ -365,8 +373,10 @@ public UpdateNode plan(Update node) ImmutableList.Builder updatedColumnValuesBuilder = ImmutableList.builder(); orderedColumnValues.forEach(columnValue -> updatedColumnValuesBuilder.add(planAndMappings.get(columnValue))); - VariableReferenceExpression rowId = new VariableReferenceExpression(Optional.empty(), builder.translate(new FieldReference(relationPlan.getDescriptor().indexOf(rowIdField))).getName(), rowIdField.getType()); - updatedColumnValuesBuilder.add(rowId); + PlanBuilder finalBuilder = builder; + Optional rowId = rowIdField.map(f -> + new VariableReferenceExpression(Optional.empty(), finalBuilder.translate(new FieldReference(relationPlan.getDescriptor().indexOf(f))).getName(), f.getType())); + rowId.ifPresent(r -> updatedColumnValuesBuilder.add(r)); List outputs = ImmutableList.of( variableAllocator.newVariable("partialrows", BIGINT), @@ -379,7 +389,7 @@ public UpdateNode plan(Update node) return new UpdateNode( getSourceLocation(node), idAllocator.getNextId(), - builder.getRoot(), + finalBuilder.getRoot(), rowId, updatedColumnValuesBuilder.build(), outputs); diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java index 5b2f793fbb7d3..b085e1baad7bb 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java @@ -816,7 +816,7 @@ public PlanNode visitTableFinish(TableFinishNode node, RewriteContext> context) { ImmutableSet.Builder builder = ImmutableSet.builder(); - builder.add(node.getRowId()); + node.getRowId().ifPresent(r -> builder.add(r)); if (node.getInputDistribution().isPresent()) { builder.addAll(node.getInputDistribution().get().getInputVariables()); } diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java index f3ff6423807fc..a4483ea0d6d9e 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java @@ -420,7 +420,7 @@ public PlanNode visitDelete(DeleteNode node, RewriteContext context) if (node.getInputDistribution().isPresent()) { context.get().variables.addAll(node.getInputDistribution().get().getInputVariables()); } - context.get().variables.add(node.getRowId()); + node.getRowId().ifPresent(r -> context.get().variables.add(r)); return context.defaultRewrite(node, context.get()); } diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/UpdateNode.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/UpdateNode.java index 7a50e3aa0da05..6f03a09111fc0 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/UpdateNode.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/UpdateNode.java @@ -34,7 +34,7 @@ public class UpdateNode extends InternalPlanNode { private final PlanNode source; - private final VariableReferenceExpression rowId; + private final Optional rowId; private final List columnValueAndRowIdSymbols; private final List outputVariables; @@ -43,7 +43,7 @@ public UpdateNode( Optional sourceLocation, @JsonProperty("id") PlanNodeId id, @JsonProperty("source") PlanNode source, - @JsonProperty("rowId") VariableReferenceExpression rowId, + @JsonProperty("rowId") Optional rowId, @JsonProperty("columnValueAndRowIdSymbols") List columnValueAndRowIdSymbols, @JsonProperty("outputVariables") List outputVariables) { @@ -55,7 +55,7 @@ public UpdateNode( PlanNodeId id, Optional statsEquivalentPlanNode, PlanNode source, - VariableReferenceExpression rowId, + Optional rowId, List columnValueAndRowIdSymbols, List outputVariables) { @@ -74,7 +74,7 @@ public PlanNode getSource() } @JsonProperty - public VariableReferenceExpression getRowId() + public Optional getRowId() { return rowId; } diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java index 2a658708324f7..3d402a9108a39 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java @@ -614,7 +614,9 @@ public Void visitDelete(DeleteNode node, Set boundV PlanNode source = node.getSource(); source.accept(this, boundVariables); // visit child - checkArgument(source.getOutputVariables().contains(node.getRowId()), "Invalid node. Row ID symbol (%s) is not in source plan output (%s)", node.getRowId(), node.getSource().getOutputVariables()); + node.getRowId().ifPresent(rowid -> + checkArgument(source.getOutputVariables().contains(rowid), + "Invalid node. Row ID symbol (%s) is not in source plan output (%s)", rowid, node.getSource().getOutputVariables())); return null; } @@ -624,7 +626,8 @@ public Void visitUpdate(UpdateNode node, Set boundV { PlanNode source = node.getSource(); source.accept(this, boundVariables); // visit child - checkArgument(source.getOutputVariables().contains(node.getRowId()), "Invalid node. Row ID symbol (%s) is not in source plan output (%s)", node.getRowId(), node.getSource().getOutputVariables()); + node.getRowId().ifPresent(r -> + checkArgument(source.getOutputVariables().contains(r), "Invalid node. Row ID symbol (%s) is not in source plan output (%s)", node.getRowId(), node.getSource().getOutputVariables())); checkArgument(source.getOutputVariables().containsAll(node.getColumnValueAndRowIdSymbols()), "Invalid node. Some UPDATE SET expression symbols (%s) are not contained in the outputSymbols (%s)", node.getColumnValueAndRowIdSymbols(), source.getOutputVariables()); return null; diff --git a/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java b/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java index de337d7f44fb2..ff1a136662c7c 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java +++ b/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java @@ -394,13 +394,13 @@ public Optional finishInsert(Session session, InsertTab } @Override - public ColumnHandle getDeleteRowIdColumnHandle(Session session, TableHandle tableHandle) + public Optional getDeleteRowIdColumn(Session session, TableHandle tableHandle) { throw new UnsupportedOperationException(); } @Override - public ColumnHandle getUpdateRowIdColumnHandle(Session session, TableHandle tableHandle, List updatedColumns) + public Optional getUpdateRowIdColumn(Session session, TableHandle tableHandle, List updatedColumns) { throw new UnsupportedOperationException(); } diff --git a/presto-main-base/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java b/presto-main-base/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java index 591337dd0614b..97eac7475f8ea 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java +++ b/presto-main-base/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java @@ -590,7 +590,7 @@ public TableFinishNode tableDelete(SchemaTableName schemaTableName, PlanNode del deleteSource.getSourceLocation(), idAllocator.getNextId(), deleteSource, - deleteRowId, + Optional.of(deleteRowId), ImmutableList.of(deleteRowId), Optional.empty())) .addInputsSet(deleteRowId) diff --git a/presto-native-execution/presto_cpp/presto_protocol/core/presto_protocol_core.h b/presto-native-execution/presto_cpp/presto_protocol/core/presto_protocol_core.h index c63562a5a06d8..53a4433a743c4 100644 --- a/presto-native-execution/presto_cpp/presto_protocol/core/presto_protocol_core.h +++ b/presto-native-execution/presto_cpp/presto_protocol/core/presto_protocol_core.h @@ -943,7 +943,7 @@ void from_json(const json& j, DeleteHandle& p); namespace facebook::presto::protocol { struct DeleteNode : public PlanNode { std::shared_ptr source = {}; - VariableReferenceExpression rowId = {}; + std::shared_ptr rowId = {}; List outputVariables = {}; std::shared_ptr inputDistribution = {}; diff --git a/presto-native-execution/presto_cpp/presto_protocol/tests/DeleteTest.cpp b/presto-native-execution/presto_cpp/presto_protocol/tests/DeleteTest.cpp index 980d51037598b..2bc68e83b2e8a 100644 --- a/presto-native-execution/presto_cpp/presto_protocol/tests/DeleteTest.cpp +++ b/presto-native-execution/presto_cpp/presto_protocol/tests/DeleteTest.cpp @@ -105,7 +105,8 @@ TEST_F(DeleteTest, jsonRoundtrip) { ASSERT_EQ(d.outputVariables[0].name, "$row_group_id"); ASSERT_EQ(d.outputVariables[1].name, "$row_number"); - ASSERT_EQ(d.rowId.name, "$rowid"); + ASSERT_NE(d.rowId, nullptr); + ASSERT_EQ(d.rowId->name, "$rowid"); testJsonRoundtrip(j, d); } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java index 7372a4170ff97..9477fa76513f6 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java @@ -524,20 +524,43 @@ default Optional finishInsert(ConnectorSession session, } /** - * Get the column handle that will generate row IDs for the delete operation. - * These IDs will be passed to the {@code deleteRows()} method of the - * {@link com.facebook.presto.spi.UpdatablePageSource} that created them. + * @deprecated Replaced by {@link #getDeleteRowIdColumn(ConnectorSession, ConnectorTableHandle)} */ + @Deprecated default ColumnHandle getDeleteRowIdColumnHandle(ConnectorSession session, ConnectorTableHandle tableHandle) { throw new PrestoException(NOT_SUPPORTED, "This connector does not support deletes"); } + /** + * Get the column handle that will generate row IDs for the delete operation, if this connector requires row IDs to support delete. + * These IDs will be passed to the {@code deleteRows()} method of the + * {@link com.facebook.presto.spi.UpdatablePageSource} that created them. If the connector does not require row IDs to perform deletes, + * then {@code Optional.empty()} may be returned. + */ + default Optional getDeleteRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle) + { + return Optional.ofNullable(getDeleteRowIdColumnHandle(session, tableHandle)); + } + + /** + * @deprecated Replaced by {@link #getUpdateRowIdColumn(ConnectorSession, ConnectorTableHandle, List)} + */ + @Deprecated default ColumnHandle getUpdateRowIdColumnHandle(ConnectorSession session, ConnectorTableHandle tableHandle, List updatedColumns) { throw new PrestoException(NOT_SUPPORTED, "This connector does not support updates"); } + /** + * Get the column handle that will generate row IDs for the update operation, if this connector requires rowIDs to support update. If the connector + * does not require row IDs to perform updates, then {@code Optional.empty()} may be returned. + */ + default Optional getUpdateRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle, List updatedColumns) + { + return Optional.ofNullable(getUpdateRowIdColumnHandle(session, tableHandle, updatedColumns)); + } + /** * Begin delete query */ diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java index 1427ffa396053..2a62f19d6b54d 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java @@ -502,6 +502,14 @@ public ColumnHandle getDeleteRowIdColumnHandle(ConnectorSession session, Connect } } + @Override + public Optional getDeleteRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + return delegate.getDeleteRowIdColumn(session, tableHandle); + } + } + @Override public void dropView(ConnectorSession session, SchemaTableName viewName) { @@ -596,6 +604,14 @@ public ColumnHandle getUpdateRowIdColumnHandle(ConnectorSession session, Connect } } + @Override + public Optional getUpdateRowIdColumn(ConnectorSession session, ConnectorTableHandle tableHandle, List updatedColumns) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + return delegate.getUpdateRowIdColumn(session, tableHandle, updatedColumns); + } + } + @Override public ConnectorDeleteTableHandle beginDelete(ConnectorSession session, ConnectorTableHandle tableHandle) { diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/plan/DeleteNode.java b/presto-spi/src/main/java/com/facebook/presto/spi/plan/DeleteNode.java index f5e713eaab673..ead4407389a21 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/plan/DeleteNode.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/plan/DeleteNode.java @@ -33,7 +33,7 @@ public final class DeleteNode extends PlanNode { private final PlanNode source; - private final VariableReferenceExpression rowId; + private final Optional rowId; private final List outputVariables; private final Optional inputDistribution; @@ -42,7 +42,7 @@ public DeleteNode( Optional sourceLocation, @JsonProperty("id") PlanNodeId id, @JsonProperty("source") PlanNode source, - @JsonProperty("rowId") VariableReferenceExpression rowId, + @JsonProperty("rowId") Optional rowId, @JsonProperty("outputVariables") List outputVariables, @JsonProperty("inputDistribution") Optional inputDistribution) { @@ -54,7 +54,7 @@ public DeleteNode( PlanNodeId id, Optional statsEquivalentPlanNode, PlanNode source, - VariableReferenceExpression rowId, + Optional rowId, List outputVariables, Optional inputDistribution) { @@ -73,7 +73,7 @@ public PlanNode getSource() } @JsonProperty - public VariableReferenceExpression getRowId() + public Optional getRowId() { return rowId; }