Skip to content

Support attribute filters in table deletion#18079

Merged
jt2594838 merged 17 commits into
masterfrom
delete_with_attribute
Jul 6, 2026
Merged

Support attribute filters in table deletion#18079
jt2594838 merged 17 commits into
masterfrom
delete_with_attribute

Conversation

@jt2594838

Copy link
Copy Markdown
Contributor

Summary

  • Support table deletion with attribute filters by converting matched devices into deletion predicates
  • Support IS NOT NULL for attribute filters and tag filters
  • Add a SegmentNotNull TagPredicate for tag IS NOT NULL without device schema fetch
  • Add integration tests for attribute/tag/time condition combinations and operator coverage

Tests

  • mvn spotless:apply -pl iotdb-core/datanode,integration-test -P with-integration-tests
  • mvn verify -DskipUTs "-Dit.test=IoTDBDeletionTableIT#testDeleteDataByTagIsNotNullFilter" "-DfailIfNoTests=false" "-Dfailsafe.failIfNoSpecifiedTests=false" "-Drat.skip=true" -pl integration-test -am -PTableSimpleIT -P with-integration-tests
  • mvn verify -DskipUTs "-Dit.test=IoTDBDeletionTableIT#testDeleteDataByAttributeFilterWithOtherOperators" "-DfailIfNoTests=false" "-Dfailsafe.failIfNoSpecifiedTests=false" "-Drat.skip=true" -pl integration-test -am -PTableSimpleIT -P with-integration-tests

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 43.27869% with 173 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.83%. Comparing base (71cb6e6) to head (6717e22).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...otdb/db/queryengine/plan/analyze/AnalyzeUtils.java 22.50% 124 Missing ⚠️
...geengine/dataregion/modification/TagPredicate.java 63.80% 38 Missing ⚠️
.../iotdb/db/storageengine/dataregion/DataRegion.java 50.00% 3 Missing ⚠️
...ine/dataregion/modification/DeletionPredicate.java 82.35% 3 Missing ⚠️
...ne/dataregion/modification/TableDeletionEntry.java 0.00% 2 Missing ⚠️
...ne/plan/relational/analyzer/StatementAnalyzer.java 0.00% 1 Missing ⚠️
...ryengine/plan/relational/sql/ast/DeleteDevice.java 0.00% 1 Missing ⚠️
...g/apache/iotdb/db/storageengine/StorageEngine.java 87.50% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18079      +/-   ##
============================================
+ Coverage     41.81%   41.83%   +0.02%     
  Complexity      318      318              
============================================
  Files          5296     5297       +1     
  Lines        373384   373797     +413     
  Branches      48210    48269      +59     
============================================
+ Hits         156134   156387     +253     
- Misses       217250   217410     +160     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Caideyipi

Copy link
Copy Markdown
Collaborator

Thanks for the work on supporting attribute filters in table deletion. I found a few issues that should be addressed before merge:

  1. IS NOT NULL tag predicates are dropped when they are combined with attribute filters.

    In AnalyzeUtils.parsePredicate, the IsNotNullPredicate branch only handles shouldQueryDevice(). Unlike the ComparisonExpression and IsNullPredicate branches, it does not add tag-only predicates to deviceFilterExpressions when shouldFilterDevice() is true. Later, when any attribute predicate exists, tagPredicate is replaced by new DeviceIn(deviceIDs), so the original SegmentNotNull predicate is lost.

    For example:

    DELETE FROM t WHERE site IS NOT NULL AND attr1 = 'red'

    may delete data from devices whose attr1 = 'red' but whose site tag is NULL. Please either include tag IS NOT NULL in the schema fetch predicate list, or preserve/intersect the original tag predicate when converting matched devices to DeviceIn.

  2. The existing integration test expectation for deviceId IS NOT NULL looks stale.

    testDeleteData still expects:

    DELETE FROM vehicle1 WHERE time < 10 AND deviceId IS NOT NULL

    to fail with "Unsupported expression", but this PR adds tag IS NOT NULL support via parseIsNotNull. This test should now fail unless the expectation and subsequent data validation are updated.

  3. Attribute comparison predicates need explicit validation before being sent to the schema fetcher.

    parseComparison returns PredicateParseResult.attribute(...) without checking the operator or right-hand value type. Inputs such as attr1 = 1, attr1 = null, or unsupported comparison operators can flow into later schema-filter conversion code and trigger casts/internal exceptions instead of a clear semantic error. Please validate attribute predicates here, or make the schema predicate conversion layer return a proper SemanticException for invalid attribute predicates.

jt2594838 added 9 commits July 1, 2026 16:45
# Conflicts:
#	iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/AnalyzeUtils.java
#	iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java
#	iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/DataRegion.java
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@jt2594838 jt2594838 merged commit 7bfd440 into master Jul 6, 2026
42 of 44 checks passed
@jt2594838 jt2594838 deleted the delete_with_attribute branch July 6, 2026 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants