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]