Skip to content

Commit add7f32

Browse files
jt2594838jt2594838
authored
Fix query failure if a table deletion involves deviceId with null & Fix status code when deleting illegal time range & support is null in deletion (apache#14217)
* Fix query failure if a table deletion involves deviceId with null * add support of filtering null * Fix status code when deleting illegal time range * support is null * disable “= null”
1 parent 864b94e commit add7f32

File tree

6 files changed

+154
-21
lines changed

6 files changed

+154
-21
lines changed

‎integration-test/src/test/java/org/apache/iotdb/relational/it/db/it/IoTDBDeletionTableIT.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747

4848
import static org.junit.Assert.assertEquals;
4949
import static org.junit.Assert.assertFalse;
50+
import static org.junit.Assert.assertTrue;
5051
import static org.junit.Assert.fail;
5152

5253
@RunWith(IoTDBTestRunner.class)
@@ -129,6 +130,24 @@ public void testUnsupportedValueFilter() throws SQLException {
129130
assertEquals("701: The operator of id predicate must be '=' for 'd0'", e.getMessage());
130131
}
131132

133+
try {
134+
statement.execute("DELETE FROM vehicle1 WHERE time < 10 and deviceId is not null");
135+
fail("should not reach here!");
136+
} catch (SQLException e) {
137+
assertEquals(
138+
"701: Unsupported expression: (deviceId IS NOT NULL) in ((time < 10) AND (deviceId IS NOT NULL))",
139+
e.getMessage());
140+
}
141+
142+
try {
143+
statement.execute("DELETE FROM vehicle1 WHERE time < 10 and deviceId = null");
144+
fail("should not reach here!");
145+
} catch (SQLException e) {
146+
assertEquals(
147+
"701: The right hand value of id predicate cannot be null with '=' operator, please use 'IS NULL' instead",
148+
e.getMessage());
149+
}
150+
132151
try (ResultSet set = statement.executeQuery("SELECT s0 FROM vehicle1")) {
133152
int cnt = 0;
134153
while (set.next()) {
@@ -570,6 +589,58 @@ public void testMultiDevice() throws SQLException {
570589
cleanData(testNum);
571590
}
572591

592+
@Test
593+
public void testDeviceIdWithNull() throws SQLException {
594+
int testNum = 14;
595+
try (Connection connection = EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT);
596+
Statement statement = connection.createStatement()) {
597+
statement.execute("use test");
598+
statement.execute(
599+
"create table t" + testNum + " (id1 string id, id2 string id, s1 int32 measurement)");
600+
// id1 is null for this record
601+
statement.execute("insert into t" + testNum + " (time, id2, s1) values (1, '1', 1)");
602+
statement.execute("insert into t" + testNum + " (time, id2, s1) values (2, '', 2)");
603+
statement.execute("insert into t" + testNum + " (time, id2, s1) values (3, NULL, 3)");
604+
statement.execute("flush");
605+
606+
statement.execute("delete from t" + testNum + " where id1 is NULL and time <= 1");
607+
try (ResultSet set = statement.executeQuery("SELECT * FROM t" + testNum)) {
608+
assertTrue(set.next());
609+
assertTrue(set.next());
610+
assertFalse(set.next());
611+
}
612+
613+
statement.execute("delete from t" + testNum + " where id2 is NULL");
614+
try (ResultSet set = statement.executeQuery("SELECT * FROM t" + testNum)) {
615+
assertTrue(set.next());
616+
assertFalse(set.next());
617+
}
618+
619+
statement.execute("delete from t" + testNum);
620+
try (ResultSet set = statement.executeQuery("SELECT * FROM t" + testNum)) {
621+
assertFalse(set.next());
622+
}
623+
624+
statement.execute("drop table t" + testNum);
625+
}
626+
}
627+
628+
@Test
629+
public void testIllegalRange() {
630+
int testNum = 15;
631+
try (Connection connection = EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT);
632+
Statement statement = connection.createStatement()) {
633+
statement.execute("use test");
634+
statement.execute(
635+
"create table t" + testNum + " (id1 string id, id2 string id, s1 int32 measurement)");
636+
637+
statement.execute("delete from t" + testNum + " where time > 10 and time <= 1");
638+
fail("Exception expected");
639+
} catch (SQLException e) {
640+
assertEquals("701: Start time 11 is greater than end time 1", e.getMessage());
641+
}
642+
}
643+
573644
@Ignore
574645
@Test
575646
public void testDeletionWritePerformance() throws SQLException, IOException {

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/AnalyzeUtils.java‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@
3838
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Delete;
3939
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Expression;
4040
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Identifier;
41+
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.IsNullPredicate;
4142
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.LogicalExpression;
4243
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.LogicalExpression.Operator;
4344
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.LongLiteral;
45+
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.NullLiteral;
4446
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.StringLiteral;
4547
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.TimeRange;
4648
import org.apache.iotdb.db.queryengine.plan.statement.crud.InsertBaseStatement;
@@ -448,11 +450,21 @@ private static TableDeletionEntry parsePredicate(Expression expression, TsTable
448450
} else if (currExp instanceof ComparisonExpression) {
449451
idPredicate =
450452
parseComparison(((ComparisonExpression) currExp), timeRange, idPredicate, table);
453+
} else if (currExp instanceof IsNullPredicate) {
454+
idPredicate = parseIsNull((IsNullPredicate) currExp, idPredicate, table);
455+
} else {
456+
throw new SemanticException("Unsupported expression: " + currExp + " in " + expression);
451457
}
452458
}
453459
if (idPredicate != null) {
454460
predicate.setIdPredicate(idPredicate);
455461
}
462+
if (timeRange.getStartTime() > timeRange.getEndTime()) {
463+
throw new SemanticException(
464+
String.format(
465+
"Start time %d is greater than end time %d",
466+
timeRange.getStartTime(), timeRange.getEndTime()));
467+
}
456468

457469
return new TableDeletionEntry(predicate, timeRange.toTsFileTimeRange());
458470
}
@@ -465,6 +477,35 @@ private static void parseAndPredicate(
465477
expressionQueue.addAll(expression.getTerms());
466478
}
467479

480+
private static IDPredicate parseIsNull(
481+
IsNullPredicate isNullPredicate, IDPredicate oldPredicate, TsTable table) {
482+
Expression leftHandExp = isNullPredicate.getValue();
483+
if (!(leftHandExp instanceof Identifier)) {
484+
throw new SemanticException("Left hand expression is not an identifier: " + leftHandExp);
485+
}
486+
String columnName = ((Identifier) leftHandExp).getValue();
487+
int idColumnOrdinal = table.getIdColumnOrdinal(columnName);
488+
if (idColumnOrdinal == -1) {
489+
throw new SemanticException(
490+
"The column '" + columnName + "' does not exist or is not an id column");
491+
}
492+
493+
// the first segment is the table name, so + 1
494+
IDPredicate newPredicate = new SegmentExactMatch(null, idColumnOrdinal + 1);
495+
return combinePredicates(oldPredicate, newPredicate);
496+
}
497+
498+
private static IDPredicate combinePredicates(IDPredicate oldPredicate, IDPredicate newPredicate) {
499+
if (oldPredicate == null) {
500+
return newPredicate;
501+
}
502+
if (oldPredicate instanceof IDPredicate.And) {
503+
((And) oldPredicate).add(newPredicate);
504+
return oldPredicate;
505+
}
506+
return new IDPredicate.And(oldPredicate, newPredicate);
507+
}
508+
468509
private static IDPredicate parseComparison(
469510
ComparisonExpression comparisonExpression,
470511
TimeRange timeRange,
@@ -521,14 +562,7 @@ private static IDPredicate parseComparison(
521562
}
522563

523564
IDPredicate newPredicate = getIdPredicate(comparisonExpression, right, idColumnOrdinal);
524-
if (oldPredicate == null) {
525-
return newPredicate;
526-
}
527-
if (oldPredicate instanceof IDPredicate.And) {
528-
((And) oldPredicate).add(newPredicate);
529-
return oldPredicate;
530-
}
531-
return new IDPredicate.And(oldPredicate, newPredicate);
565+
return combinePredicates(oldPredicate, newPredicate);
532566
}
533567

534568
private static IDPredicate getIdPredicate(
@@ -540,13 +574,15 @@ private static IDPredicate getIdPredicate(
540574
String rightHandValue;
541575
if (right instanceof StringLiteral) {
542576
rightHandValue = ((StringLiteral) right).getValue();
577+
} else if (right instanceof NullLiteral) {
578+
throw new SemanticException(
579+
"The right hand value of id predicate cannot be null with '=' operator, please use 'IS NULL' instead");
543580
} else {
544581
throw new SemanticException(
545582
"The right hand value of id predicate must be a string: " + right);
546583
}
547584
// the first segment is the table name, so + 1
548-
IDPredicate newPredicate = new SegmentExactMatch(rightHandValue, idColumnOrdinal + 1);
549-
return newPredicate;
585+
return new SegmentExactMatch(rightHandValue, idColumnOrdinal + 1);
550586
}
551587

552588
public interface DataPartitionQueryFunc {

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/modification/IDPredicate.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -247,11 +247,15 @@ public SegmentExactMatch() {
247247

248248
@Override
249249
public int serializedSize() {
250-
byte[] bytes = pattern.getBytes(TSFileConfig.STRING_CHARSET);
251-
return super.serializedSize()
252-
+ ReadWriteForEncodingUtils.varIntSize(bytes.length)
253-
+ bytes.length * Character.BYTES
254-
+ ReadWriteForEncodingUtils.varIntSize(segmentIndex);
250+
if (pattern != null) {
251+
byte[] bytes = pattern.getBytes(TSFileConfig.STRING_CHARSET);
252+
return super.serializedSize()
253+
+ ReadWriteForEncodingUtils.varIntSize(bytes.length)
254+
+ bytes.length * Character.BYTES
255+
+ ReadWriteForEncodingUtils.varIntSize(segmentIndex);
256+
} else {
257+
return ReadWriteForEncodingUtils.varIntSize(-1);
258+
}
255259
}
256260

257261
@Override
@@ -284,7 +288,7 @@ public void deserialize(ByteBuffer buffer) {
284288

285289
@Override
286290
public boolean matches(IDeviceID deviceID) {
287-
return deviceID.segmentNum() > segmentIndex && pattern.equals(deviceID.segment(segmentIndex));
291+
return Objects.equals(pattern, deviceID.segment(segmentIndex));
288292
}
289293

290294
@Override

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/metadata/path/PatternTreeMapTest.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,17 @@
2222
import org.apache.iotdb.commons.path.MeasurementPath;
2323
import org.apache.iotdb.commons.path.PartialPath;
2424
import org.apache.iotdb.commons.path.PatternTreeMap;
25+
import org.apache.iotdb.db.storageengine.dataregion.modification.DeletionPredicate;
26+
import org.apache.iotdb.db.storageengine.dataregion.modification.IDPredicate.NOP;
2527
import org.apache.iotdb.db.storageengine.dataregion.modification.ModEntry;
28+
import org.apache.iotdb.db.storageengine.dataregion.modification.TableDeletionEntry;
2629
import org.apache.iotdb.db.storageengine.dataregion.modification.TreeDeletionEntry;
2730
import org.apache.iotdb.db.utils.datastructure.PatternTreeMapFactory;
2831
import org.apache.iotdb.db.utils.datastructure.PatternTreeMapFactory.ModsSerializer;
2932
import org.apache.iotdb.db.utils.datastructure.PatternTreeMapFactory.StringSerializer;
3033

34+
import org.apache.tsfile.file.metadata.IDeviceID.Factory;
35+
import org.apache.tsfile.read.common.TimeRange;
3136
import org.junit.Assert;
3237
import org.junit.Test;
3338

@@ -37,6 +42,9 @@
3742
import java.util.List;
3843
import java.util.Set;
3944

45+
import static org.junit.Assert.assertEquals;
46+
import static org.junit.Assert.assertTrue;
47+
4048
public class PatternTreeMapTest {
4149

4250
@Test
@@ -213,12 +221,25 @@ public void modificationPatternTreeMapTest() throws IllegalPathException {
213221
new TreeDeletionEntry(new MeasurementPath("root.sg1.d1.*.d3.s4"), 4, 6)));
214222
}
215223

224+
@Test
225+
public void testDeviceIdWithNull() {
226+
PatternTreeMap<ModEntry, ModsSerializer> patternTreeMap =
227+
PatternTreeMapFactory.getModsPatternTreeMap();
228+
ModEntry modEntry =
229+
new TableDeletionEntry(new DeletionPredicate("table1", new NOP()), new TimeRange(0, 100));
230+
patternTreeMap.append(modEntry.keyOfPatternTree(), modEntry);
231+
List<ModEntry> result =
232+
patternTreeMap.getOverlapped(
233+
Factory.DEFAULT_FACTORY.create(new String[] {"table1", null, "id2"}), "s1");
234+
assertEquals(Collections.singletonList(modEntry), result);
235+
}
236+
216237
private <T> void checkOverlapped(
217238
PatternTreeMap<T, ?> patternTreeMap, PartialPath partialPath, List<T> expectedList) {
218239
Set<T> resultSet = new HashSet<>(patternTreeMap.getOverlapped(partialPath));
219240
Assert.assertEquals(expectedList.size(), resultSet.size());
220241
for (T o : expectedList) {
221-
Assert.assertTrue(resultSet.contains(o));
242+
assertTrue(resultSet.contains(o));
222243
}
223244
}
224245

@@ -234,7 +255,7 @@ private <T> void checkOverlappedByDeviceMeasurements(
234255
Set<T> actualSubSet = new HashSet<>(actualList.get(i));
235256
Assert.assertEquals(expectedSubList.size(), actualSubSet.size());
236257
for (T o : expectedSubList) {
237-
Assert.assertTrue(actualSubSet.contains(o));
258+
assertTrue(actualSubSet.contains(o));
238259
}
239260
}
240261
}
@@ -244,7 +265,7 @@ private <T> void checkOverlappedByDevice(
244265
Set<T> resultSet = new HashSet<>(patternTreeMap.getDeviceOverlapped(devicePath));
245266
Assert.assertEquals(expectedList.size(), resultSet.size());
246267
for (T o : expectedList) {
247-
Assert.assertTrue(resultSet.contains(o));
268+
assertTrue(resultSet.contains(o));
248269
}
249270
}
250271
}

‎iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/path/PartialPath.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ public PartialPath(IDeviceID device) throws IllegalPathException {
7272
System.arraycopy(tableNameSegments, 0, nodes, 0, tableNameSegments.length);
7373
// copy non-table-name segments
7474
for (int i = 0; i < device.segmentNum() - 1; i++) {
75-
nodes[i + tableNameSegments.length] = device.segment(i + 1).toString();
75+
nodes[i + tableNameSegments.length] =
76+
device.segment(i + 1) != null ? device.segment(i + 1).toString() : null;
7677
}
7778
this.fullPath = getFullPath();
7879
}

‎iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/path/PatternTreeMap.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public List<V> getOverlapped(IDeviceID deviceID, String measurement) {
143143
// TODO change this way
144144
PartialPath devicePath;
145145
try {
146-
devicePath = new PartialPath(deviceID.toString());
146+
devicePath = new PartialPath(deviceID);
147147
} catch (IllegalPathException e) {
148148
throw new RuntimeException(e);
149149
}

0 commit comments

Comments
 (0)