Skip to content

Commit 8bfa077

Browse files
[fix](fe) Enforce partition guard for Nereids delete
### What problem does this PR solve? Issue Number: None Related PR: #62944 Problem Summary: Nereids predicate DELETE bypassed delete_without_partition for non-Unique range and list tables after partition pruning moved into the planner. A predicate that did not restrict the target partitions could therefore delete across every partition while the safety switch was false, even though unpartitioned tables must remain allowed. Use the physical scan effective partition-pruning signal to reject only non-Unique partitioned deletes that neither specify nor restrict partitions, while preserving Unique and DELETE USING behavior. ### Release note Nereids DELETE now honors delete_without_partition for non-Unique range and list tables when no effective partition target is specified. ### Check List (For Author) - Test: Unit Test and regression test case - ./run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.commands.DeleteFromCommandTest - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.rewrite.PartitionPrunerTest - Added delete_p0/test_basic_delete_job coverage; not run locally because this worktree has no FE/BE cluster output - Behavior changed: Yes. Unsafe full-partition predicate deletes now require delete_without_partition=true on non-Unique range/list tables; unpartitioned and Unique table behavior is unchanged. - Does this need documentation: No. The change restores the documented safety-switch behavior. Co-authored-by: Siyang Tang <tangsiyang@selectdb.com>
1 parent 18e939b commit 8bfa077

3 files changed

Lines changed: 91 additions & 0 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,14 @@ private List<Partition> getSelectedPartitions(
302302
if (olapTable.getPartitionInfo().getType().equals(PartitionType.UNPARTITIONED)) {
303303
return Lists.newArrayList(olapTable.getPartitions());
304304
}
305+
if (olapTable.getKeysType() != KeysType.UNIQUE_KEYS
306+
&& partitionNames.isEmpty()
307+
&& !scan.hasPartitionPredicate()
308+
&& !ConnectContext.get().getSessionVariable().isDeleteWithoutPartition()) {
309+
throw new AnalysisException("This is a range or list partitioned table."
310+
+ " You should specify partition in delete stmt,"
311+
+ " or set delete_without_partition to true");
312+
}
305313
List<Slot> partitionSlots = Lists.newArrayList();
306314
for (Column c : olapTable.getPartitionColumns()) {
307315
Slot partitionSlot = null;

fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommandTest.java

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,24 @@
1717

1818
package org.apache.doris.nereids.trees.plans.commands;
1919

20+
import org.apache.doris.catalog.KeysType;
21+
import org.apache.doris.catalog.OlapTable;
22+
import org.apache.doris.catalog.Partition;
23+
import org.apache.doris.catalog.PartitionInfo;
24+
import org.apache.doris.catalog.PartitionType;
2025
import org.apache.doris.nereids.exceptions.AnalysisException;
26+
import org.apache.doris.nereids.trees.plans.physical.PhysicalFilter;
27+
import org.apache.doris.nereids.trees.plans.physical.PhysicalOlapScan;
28+
import org.apache.doris.qe.ConnectContext;
2129

2230
import org.junit.jupiter.api.Assertions;
2331
import org.junit.jupiter.api.Test;
32+
import org.mockito.Mockito;
2433

2534
import java.lang.reflect.InvocationTargetException;
2635
import java.lang.reflect.Method;
2736
import java.util.Collections;
37+
import java.util.List;
2838

2939
public class DeleteFromCommandTest {
3040

@@ -68,6 +78,57 @@ public void testBuildDeleteFallbackExceptionFallsBackToThrowableToString() throw
6878
Assertions.assertSame(initialException, mergedException.getSuppressed()[0]);
6979
}
7080

81+
@Test
82+
public void testUnpartitionedDeleteDoesNotRequireSessionOverride() throws Exception {
83+
DeleteFromCommand command = new DeleteFromCommand(Collections.emptyList(), null,
84+
false, Collections.emptyList(), null);
85+
OlapTable table = Mockito.mock(OlapTable.class);
86+
PartitionInfo partitionInfo = Mockito.mock(PartitionInfo.class);
87+
Partition partition = Mockito.mock(Partition.class);
88+
Mockito.when(table.getPartitionInfo()).thenReturn(partitionInfo);
89+
Mockito.when(partitionInfo.getType()).thenReturn(PartitionType.UNPARTITIONED);
90+
Mockito.when(table.getPartitions()).thenReturn(Collections.singletonList(partition));
91+
92+
List<Partition> selectedPartitions = invokeGetSelectedPartitions(command, table,
93+
null, null, Collections.emptyList());
94+
95+
Assertions.assertEquals(Collections.singletonList(partition), selectedPartitions);
96+
}
97+
98+
@Test
99+
public void testRejectPartitionedDeleteWithoutEffectivePartition() throws Exception {
100+
ConnectContext previousContext = ConnectContext.get();
101+
ConnectContext connectContext = new ConnectContext();
102+
connectContext.getSessionVariable().deleteWithoutPartition = false;
103+
connectContext.setThreadLocalInfo();
104+
try {
105+
DeleteFromCommand command = new DeleteFromCommand(Collections.emptyList(), null,
106+
false, Collections.emptyList(), null);
107+
OlapTable table = Mockito.mock(OlapTable.class);
108+
PartitionInfo partitionInfo = Mockito.mock(PartitionInfo.class);
109+
PhysicalOlapScan scan = Mockito.mock(PhysicalOlapScan.class);
110+
Mockito.when(table.getPartitionInfo()).thenReturn(partitionInfo);
111+
Mockito.when(partitionInfo.getType()).thenReturn(PartitionType.RANGE);
112+
Mockito.when(table.getKeysType()).thenReturn(KeysType.DUP_KEYS);
113+
Mockito.when(scan.hasPartitionPredicate()).thenReturn(false);
114+
115+
InvocationTargetException exception = Assertions.assertThrows(InvocationTargetException.class,
116+
() -> invokeGetSelectedPartitions(command, table, null, scan, Collections.emptyList()));
117+
118+
Assertions.assertTrue(exception.getCause() instanceof AnalysisException);
119+
Assertions.assertEquals("This is a range or list partitioned table."
120+
+ " You should specify partition in delete stmt,"
121+
+ " or set delete_without_partition to true",
122+
exception.getCause().getMessage());
123+
} finally {
124+
if (previousContext == null) {
125+
ConnectContext.remove();
126+
} else {
127+
previousContext.setThreadLocalInfo();
128+
}
129+
}
130+
}
131+
71132
// Use reflection to validate the helper without exposing it only for tests.
72133
private AnalysisException invokeBuildDeleteFallbackException(DeleteFromCommand command,
73134
Exception initialException, Exception fallbackException)
@@ -77,4 +138,13 @@ private AnalysisException invokeBuildDeleteFallbackException(DeleteFromCommand c
77138
method.setAccessible(true);
78139
return (AnalysisException) method.invoke(command, initialException, fallbackException);
79140
}
141+
142+
private List<Partition> invokeGetSelectedPartitions(DeleteFromCommand command,
143+
OlapTable table, PhysicalFilter<?> filter, PhysicalOlapScan scan,
144+
List<String> partitionNames) throws Exception {
145+
Method method = DeleteFromCommand.class.getDeclaredMethod("getSelectedPartitions",
146+
OlapTable.class, PhysicalFilter.class, PhysicalOlapScan.class, List.class);
147+
method.setAccessible(true);
148+
return (List<Partition>) method.invoke(command, table, filter, scan, partitionNames);
149+
}
80150
}

regression-test/suites/delete_p0/test_basic_delete_job.groovy

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ suite("test_basic_delete_job") {
3737
"""
3838
sql """insert into ${unpartitionTable} values (1, "a"), (2, "b"), (3, "c")"""
3939
qt_unpartition1 """select * from ${unpartitionTable} order by id"""
40+
sql """set delete_without_partition = false"""
4041
sql """delete from ${unpartitionTable} where id < 0"""
4142
qt_unpartition2 """select * from ${unpartitionTable} order by id"""
4243
sql """delete from ${unpartitionTable} where id = 1"""
@@ -70,6 +71,11 @@ suite("test_basic_delete_job") {
7071
qt_one_range4 """select * from ${oneRangeColumnTable} order by id"""
7172
sql """delete from ${oneRangeColumnTable} partition(p0) where id < 22"""
7273
qt_one_range5 """select * from ${oneRangeColumnTable} order by id"""
74+
test {
75+
sql """delete from ${oneRangeColumnTable} where name = "d" """
76+
exception "This is a range or list partitioned table. You should specify partition in delete stmt, or set delete_without_partition to true"
77+
}
78+
sql """set delete_without_partition = true"""
7379
sql """delete from ${oneRangeColumnTable} where name = "d" """
7480
qt_one_range6 """select * from ${oneRangeColumnTable} order by id"""
7581
sql """delete from ${oneRangeColumnTable} partition(p2) where name = "g" """
@@ -117,6 +123,7 @@ suite("test_basic_delete_job") {
117123
qt_two_range9 """select * from ${twoRangeColumnTable} order by id1, id2"""
118124

119125
// Test one list partition column
126+
sql """set delete_without_partition = false"""
120127
sql """DROP TABLE IF EXISTS ${oneListColumnTable} """
121128
sql """CREATE TABLE ${oneListColumnTable} (
122129
`id` int NOT NULL,
@@ -142,6 +149,11 @@ suite("test_basic_delete_job") {
142149
qt_one_list4 """select * from ${oneListColumnTable} order by id"""
143150
sql """delete from ${oneListColumnTable} partition(p0) where id < 22"""
144151
qt_one_list5 """select * from ${oneListColumnTable} order by id"""
152+
test {
153+
sql """delete from ${oneListColumnTable} where name = "d" """
154+
exception "This is a range or list partitioned table. You should specify partition in delete stmt, or set delete_without_partition to true"
155+
}
156+
sql """set delete_without_partition = true"""
145157
sql """delete from ${oneListColumnTable} where name = "d" """
146158
qt_one_list6 """select * from ${oneListColumnTable} order by id"""
147159
sql """delete from ${oneListColumnTable} partition(p2) where name = "g" """
@@ -187,4 +199,5 @@ suite("test_basic_delete_job") {
187199
qt_two_list8 """select * from ${twoListColumnTable} order by id1, id2"""
188200
sql """delete from ${twoListColumnTable} where id2 > 300"""
189201
qt_two_list9 """select * from ${twoListColumnTable} order by id1, id2"""
202+
sql """set delete_without_partition = false"""
190203
}

0 commit comments

Comments
 (0)