Caideyipi commented on PR #18079:
URL: https://github.com/apache/iotdb/pull/18079#issuecomment-4851853090

   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:
      ```sql
      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:
      ```sql
      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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to