fix: reject bare NULL literal in expressions instead of misparsing it as a field (#50889)

issue: #50882

## Problem

`NULL` is not a grammar token in the plan parser, so the lexer treats a
bare `NULL`/`null` as an ordinary identifier. When it appears where a
value is expected — e.g. inside an `in [...]` value list:

```
id in [6560, NULL, 6722, -7856, -6757]
```

`VisitIdentifier` runs a field lookup on `NULL`:

- **without a dynamic field** it fails with the misleading `field NULL
not exist`, which misleads users into thinking they must add a column
named `NULL` to their schema;
- **with a dynamic field** it is silently mistaken for a JSON key and
accepted.

This affects both `delete()` and `query()` (they share the same
`ParseExpr` path).

## Fix

Treat bare `null`/`NULL` (case-insensitive) as a reserved word in
`VisitIdentifier` and reject it up-front with an actionable message,
regardless of whether a dynamic field exists:

```
NULL literal is not supported in expressions; use '<field> is null' or '<field> is not null' instead
```

This implements option 1 (robust rejection) from the issue. `<field> is
null` / `is not null` are unaffected (they parse via dedicated tokens,
not `VisitIdentifier`).

**The guard is schema-aware for backward compatibility** (review
feedback): `null` only becomes a create-time keyword in this PR, so a
legacy collection may own a field literally named `null`, and the bare
identifier is the only syntax that can reference a top-level scalar
field. The rejection is therefore gated on a strict `GetFieldFromName`
lookup — a real declared field named `null` resolves exactly as before
this PR (including `null is null`, which now parses as a valid predicate
on that field), while the dynamic-field fallback (the source of the
original misparse) still rejects. A JSON **sub-key** literally named
`null` additionally remains reachable via quoting, e.g. `field["null"]`
/ `$meta["null"]`, whose base identifier is the field name, not `null`.

## Test

Added `TestExpr_NullLiteral` covering rejection of `NULL` inside `in
[...]`, as a comparison operand, and as a left operand, plus
confirmation that `is null` / `is not null` still work. Added
`TestExpr_NullLiteral_LegacyNullField` locking the legacy path: a scalar
field named `null` stays queryable via the bare identifier, a JSON field
named `null` stays reachable as `null["x"]`, and any casing that doesn't
name a real field keeps the reserved-word rejection. Full `planparserv2`
package tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

## Note: field-name validation tightened

Alongside the expression-level rejection, `validateFieldName` (proxy)
and
`validateAddedStructFieldName` (rootcoord) now go through
`common.IsFieldNameKeyword`,
which rejects **any casing** of `null` (`null`/`NULL`/`nUlL`/…) as a
field name —
consistent with how a bare `NULL` literal is rejected in expressions.
Previously
`FieldNameKeywords` had no `null` entry, so a field literally named
`null` could be
created. This only affects **new** field creation (validation runs at
create time);
existing collections/fields are unaffected — and thanks to the
schema-aware guard
above, a legacy field literally named `null` also stays queryable.

---------

Signed-off-by: xiaofanluan <xf@hjjaq.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
James
2026-07-07 10:42:30 +08:00
committed by GitHub
co-authored by xiaofanluan Claude Opus 4.8
parent dd9c95749b
commit 590a14fdf7
8 changed files with 392 additions and 121 deletions
@@ -56,12 +56,43 @@ func (v *ParserVisitor) VisitParens(ctx *parser.ParensContext) interface{} {
return ctx.Expr().Accept(v)
}
// errNullLiteral rejects a bare `null`/`NULL` used as an identifier. `NULL` is
// lexed as an ordinary identifier (it is not a grammar token), so a literal NULL
// where a column is expected — inside an `in [...]` list, a comparison/range
// operand, a function argument (`array_length(NULL)`), an `is null` target, a
// JSON/array subscript base (`NULL["x"]`), etc. — reaches a field lookup. Without
// a dynamic field it fails an opaque "field NULL not exist"; with a dynamic field
// it is silently mistaken for a JSON key named NULL. Treat bare `null`/`NULL` as a
// reserved word (issue #50882).
//
// The guard is schema-aware for backward compatibility: "null" only became a
// create-time keyword in this change, so a legacy collection may own a field
// literally named "null", and the bare identifier is the only syntax that can
// reference a top-level scalar field. A strict GetFieldFromName (NOT the
// DefaultJSON variant, whose dynamic-field fallback is exactly what produced
// the original misparse) decides: a real declared field resolves as before,
// anything else is rejected. A JSON key literally named "null" additionally
// stays reachable via quoting, e.g. `field["null"]` / `$meta["null"]`, whose
// base identifier is the field name, not "null".
func errNullLiteral() error {
return merr.WrapErrParameterInvalidMsg(
"NULL literal is not supported in expressions; use '<field> is null' or '<field> is not null' instead")
}
func (v *ParserVisitor) translateIdentifier(identifier string) (*ExprWithType, error) {
return v.translateIdentifierWithText(identifier, false)
}
func (v *ParserVisitor) translateIdentifierWithText(identifier string, allowText bool) (*ExprWithType, error) {
identifier = decodeUnicode(identifier)
if strings.EqualFold(identifier, "null") {
// Schema-aware: honor a legacy declared field literally named "null";
// strict lookup so a dynamic-field collection still rejects
// bare-null-as-JSON-key. See errNullLiteral.
if _, err := v.schema.GetFieldFromName(identifier); err != nil {
return nil, errNullLiteral()
}
}
field, err := v.schema.GetFieldFromNameDefaultJSON(identifier)
if err != nil {
return nil, err
@@ -1950,6 +1981,15 @@ func (v *ParserVisitor) getColumnInfoFromJSONIdentifier(identifier string) (*pla
// the field name and normal (non-raw) keys individually below instead.
rawFieldName := strings.Split(identifier, "[")[0]
fieldName := decodeUnicode(rawFieldName)
// Reject a bare `null`/`NULL` base (e.g. `NULL["x"]`, `NULL[0]`) here too —
// this lookup bypasses translateIdentifierWithText. Schema-aware like the
// guard there: a legacy field literally named "null" resolves. See
// errNullLiteral.
if strings.EqualFold(fieldName, "null") {
if _, err := v.schema.GetFieldFromName(fieldName); err != nil {
return nil, errNullLiteral()
}
}
nestedPath := make([]string, 0)
field, err := v.schema.GetFieldFromNameDefaultJSON(fieldName)
if err != nil {
@@ -137,6 +137,148 @@ func TestExpr_Term(t *testing.T) {
}
}
// assertNullLiteralRejected checks that the expression is rejected specifically
// because of the bare-NULL reserved-word guard, i.e. it returns the actionable
// message rather than the misleading "field NULL not exist" (issue #50882).
func assertNullLiteralRejected(t *testing.T, helper *typeutil.SchemaHelper, exprStr string) {
_, err := ParseExpr(helper, exprStr, nil)
if assert.Error(t, err, exprStr) {
assert.Contains(t, err.Error(), "NULL literal is not supported in expressions", exprStr)
}
}
func TestExpr_NullLiteral(t *testing.T) {
// NULL is not a value literal. It is lexed as a bare identifier, so it must be
// rejected wherever it appears in column/value position (issue #50882). The
// behavior must be identical whether or not a dynamic field is present: without
// one a bare NULL fails a field lookup; with one it would otherwise be silently
// mistaken for a dynamic JSON key named "NULL". Use `<field> is null` /
// `is not null` to compare against null instead.
rejected := []string{
// inside an `in [...]` value list (the exact shape from the issue)
`Int64Field in [6560, NULL, 6722, -7856, -6757]`,
`Int64Field in [null]`,
`Int64Field in [NULL]`,
`Int64Field not in [1, Null]`,
`VarCharField in ["a", null, "b"]`,
`(not (not ((Int64Field is not null) and (Int64Field in [1, NULL, 2]))))`,
// NULL as the tested column on the left of `in`
`NULL in [1, 2]`,
// binary comparison, either side
`Int64Field == NULL`,
`NULL == Int64Field`,
`NULL > 5`,
`Int64Field != NULL`,
// range comparison
`1 < NULL < 5`,
`Int64Field < NULL`,
// logical / unary operands
`NULL and Int64Field > 0`,
`Int64Field > 0 or NULL`,
`not NULL`,
// function arguments
`array_length(NULL) > 0`,
`array_contains(NULL, 1)`,
// `is null` / `is not null` with NULL as the target (nonsensical)
`NULL is null`,
`NULL is not null`,
// NULL as a JSON / array subscript base — a separate lookup path
// (getColumnInfoFromJSONIdentifier) that bypasses translateIdentifier
`NULL["x"] == 1`,
`NULL[0] > 1`,
// case-insensitive: any casing of the reserved word is rejected
`Int64Field == Null`,
`Int64Field == nUlL`,
`Int64Field == NuLL`,
}
// Valid expressions that must NOT be affected by the guard.
validBoth := []string{
// the real "is null" predicates the guard points users to
`Int64Field is null`,
`Int64Field is not null`,
`JSONField["a"] is null`,
// a JSON key literally named "null" stays reachable via quoting: the base
// identifier is the field name, not "null"
`JSONField["null"] == 1`,
`JSONField['null'] == 1`,
// the string literal "null" is a value, not an identifier
`VarCharField == "null"`,
`VarCharField in ["null", "NULL"]`,
}
// Only valid when the dynamic field exists.
validDynamicOnly := []string{
`$meta["null"] == 1`,
`A == 1`, // sanity: an arbitrary dynamic key still resolves
}
for _, dynamic := range []bool{true, false} {
t.Run(fmt.Sprintf("dynamic=%v", dynamic), func(t *testing.T) {
helper, err := typeutil.CreateSchemaHelper(newTestSchema(dynamic))
assert.NoError(t, err)
for _, exprStr := range rejected {
assertNullLiteralRejected(t, helper, exprStr)
}
for _, exprStr := range validBoth {
assertValidExpr(t, helper, exprStr)
}
if dynamic {
for _, exprStr := range validDynamicOnly {
assertValidExpr(t, helper, exprStr)
}
}
})
}
}
// TestExpr_NullLiteral_LegacyNullField locks the schema-aware side of the
// bare-NULL guard: "null" only became a create-time keyword together with this
// guard, so a legacy collection may own a field literally named "null", and the
// bare identifier is the ONLY syntax that can reference a top-level scalar
// field (quoting like field["null"] reaches JSON sub-keys only). Such a field
// must stay queryable; the strict GetFieldFromName check makes it resolve while
// everything else keeps the reserved-word rejection (see errNullLiteral).
func TestExpr_NullLiteral_LegacyNullField(t *testing.T) {
withNullField := func(dataType schemapb.DataType) *typeutil.SchemaHelper {
schema := newTestSchema(true)
schema.Fields = append(schema.Fields, &schemapb.FieldSchema{
FieldID: 199, Name: "null", Description: "legacy field literally named null", DataType: dataType,
})
helper, err := typeutil.CreateSchemaHelper(schema)
require.NoError(t, err)
return helper
}
t.Run("scalar field named null", func(t *testing.T) {
helper := withNullField(schemapb.DataType_Int64)
// The bare identifier resolves to the declared field, as before the guard.
assertValidExpr(t, helper, `null > 5`)
assertValidExpr(t, helper, `null in [1, 2]`)
assertValidExpr(t, helper, `Int64Field == null`)
// The guard previously rejected these outright; schema-aware turns them
// into valid "is the null-named field NULL?" predicates.
assertValidExpr(t, helper, `null is null`)
assertValidExpr(t, helper, `null is not null`)
// Strict lookup is exact-case: a differently-cased NULL does not match the
// declared field and keeps the reserved-word rejection (pre-guard it would
// have been misparsed as the dynamic JSON key $meta["NULL"]).
assertNullLiteralRejected(t, helper, `NULL > 5`)
assertNullLiteralRejected(t, helper, `Int64Field == Null`)
})
t.Run("json field named null", func(t *testing.T) {
helper := withNullField(schemapb.DataType_JSON)
// The subscript base resolves via the same schema-aware guard in
// getColumnInfoFromJSONIdentifier.
assertValidExpr(t, helper, `null["x"] == 1`)
assertValidExpr(t, helper, `null["x"] is null`)
assertNullLiteralRejected(t, helper, `NULL["x"] == 1`)
})
}
func TestExpr_Call(t *testing.T) {
schema := newTestSchema(true)
helper, err := typeutil.CreateSchemaHelper(schema)
+1 -1
View File
@@ -397,7 +397,7 @@ func validateFieldName(fieldName string) error {
return merr.WrapErrFieldNameInvalid(fieldName, msg)
}
}
if _, ok := common.FieldNameKeywords[fieldName]; ok {
if common.IsFieldNameKeyword(fieldName) {
msg := invalidMsg + fmt.Sprintf("%s is keyword in milvus.", fieldName)
return merr.WrapErrFieldNameInvalid(fieldName, msg)
}
+5
View File
@@ -366,6 +366,11 @@ func TestValidateFieldName(t *testing.T) {
string(longName),
"中文",
"True",
"null",
"Null",
"NULL",
"nUlL",
"NuLL",
"array_contains",
"json_contains_any",
"ARRAY_LENGTH",
@@ -258,7 +258,7 @@ func validateAddedStructFieldName(fieldName string) error {
return merr.WrapErrFieldNameInvalid(fieldName, msg)
}
}
if _, ok := common.FieldNameKeywords[fieldName]; ok {
if common.IsFieldNameKeyword(fieldName) {
msg := invalidMsg + fmt.Sprintf("%s is keyword in milvus.", fieldName)
return merr.WrapErrFieldNameInvalid(fieldName, msg)
}
+15
View File
@@ -1,5 +1,7 @@
package common
import "strings"
var FieldNameKeywords = map[string]struct{}{
"$meta": {},
"like": {},
@@ -36,3 +38,16 @@ var FieldNameKeywords = map[string]struct{}{
"random_sample": {},
"RANDOM_SAMPLE": {},
}
// IsFieldNameKeyword reports whether fieldName is a reserved word that cannot be
// used as a field name. `null` is matched case-insensitively — any casing of NULL
// is rejected — to stay consistent with the expression parser, which rejects a
// bare NULL literal regardless of casing (issue #50882). It is therefore handled
// here rather than enumerated in FieldNameKeywords; the other keywords match the
// exact casings listed there.
func IsFieldNameKeyword(fieldName string) bool {
if _, ok := FieldNameKeywords[fieldName]; ok {
return true
}
return strings.EqualFold(fieldName, "null")
}
+13
View File
@@ -0,0 +1,13 @@
package common
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestIsFieldNameKeywordRejectsNullCaseInsensitive(t *testing.T) {
for _, name := range []string{"null", "Null", "NULL", "nUlL", "NuLL"} {
assert.True(t, IsFieldNameKeyword(name), name)
}
}
@@ -14,12 +14,11 @@ These tests verify correct behavior of:
import pytest
from base.client_v2_base import TestMilvusClientV2Base
from utils.util_log import test_log as log
from common import common_func as cf
from common import common_type as ct
from common.common_type import CaseLabel, CheckTasks
from common.common_type import CaseLabel
from pymilvus import DataType
from utils.util_log import test_log as log
prefix = "three_valued_logic"
default_dim = ct.default_dim
@@ -125,15 +124,11 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
def ids_both_null(self):
"""IDs where both nullable_a AND nullable_b are NULL."""
return self.get_ids_where(
lambda r: r[self.nullable_a_field] is None and r[self.nullable_b_field] is None
)
return self.get_ids_where(lambda r: r[self.nullable_a_field] is None and r[self.nullable_b_field] is None)
def ids_either_null(self):
"""IDs where nullable_a OR nullable_b is NULL."""
return self.get_ids_where(
lambda r: r[self.nullable_a_field] is None or r[self.nullable_b_field] is None
)
return self.get_ids_where(lambda r: r[self.nullable_a_field] is None or r[self.nullable_b_field] is None)
def ids_both_not_null(self):
"""IDs where both nullable_a AND nullable_b are NOT NULL."""
@@ -143,28 +138,24 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
def ids_a_not_null_and_gt(self, value):
"""IDs where nullable_a IS NOT NULL AND nullable_a > value."""
return self.get_ids_where(
lambda r: r[self.nullable_a_field] is not None and r[self.nullable_a_field] > value
)
return self.get_ids_where(lambda r: r[self.nullable_a_field] is not None and r[self.nullable_a_field] > value)
def ids_a_null_or_a_gte(self, value):
"""IDs where nullable_a IS NULL OR nullable_a >= value."""
return self.get_ids_where(
lambda r: r[self.nullable_a_field] is None or r[self.nullable_a_field] >= value
)
return self.get_ids_where(lambda r: r[self.nullable_a_field] is None or r[self.nullable_a_field] >= value)
def ids_a_null_or_a_gt(self, value):
"""IDs where nullable_a IS NULL OR nullable_a > value."""
return self.get_ids_where(
lambda r: r[self.nullable_a_field] is None or r[self.nullable_a_field] > value
)
return self.get_ids_where(lambda r: r[self.nullable_a_field] is None or r[self.nullable_a_field] > value)
def ids_complex_nested(self):
"""IDs for NOT (((A IS NOT NULL) AND (A > 200)) AND (B IS NOT NULL))."""
# Inner: (A NOT NULL AND A > 200) AND B NOT NULL
inner = self.get_ids_where(
lambda r: (r[self.nullable_a_field] is not None and r[self.nullable_a_field] > 200)
and r[self.nullable_b_field] is not None
lambda r: (
(r[self.nullable_a_field] is not None and r[self.nullable_a_field] > 200)
and r[self.nullable_b_field] is not None
)
)
# NOT of inner = all IDs except inner
all_ids = set(row[self.pk_field] for row in self.datas)
@@ -172,9 +163,7 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
def ids_pk_lt_and_a_null(self, pk_limit):
"""IDs where id < pk_limit AND nullable_a IS NULL."""
return self.get_ids_where(
lambda r: r[self.pk_field] < pk_limit and r[self.nullable_a_field] is None
)
return self.get_ids_where(lambda r: r[self.pk_field] < pk_limit and r[self.nullable_a_field] is None)
# ========================================================================
# Basic IS NULL / IS NOT NULL Tests
@@ -188,9 +177,9 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: return rows where nullable_a is NULL
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"{self.nullable_a_field} IS NULL",
output_fields=[self.pk_field])[0]
res = self.query(
client, self.collection_name, filter=f"{self.nullable_a_field} IS NULL", output_fields=[self.pk_field]
)[0]
expected_ids = self.ids_a_is_null()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -204,9 +193,9 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: return rows where nullable_a is NOT NULL
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"{self.nullable_a_field} IS NOT NULL",
output_fields=[self.pk_field])[0]
res = self.query(
client, self.collection_name, filter=f"{self.nullable_a_field} IS NOT NULL", output_fields=[self.pk_field]
)[0]
expected_ids = self.ids_a_is_not_null()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -226,13 +215,16 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
This is the critical bug fixed in PR #47333.
"""
client = self._client()
res1 = self.query(client, self.collection_name,
filter=f"{self.nullable_a_field} IS NULL",
output_fields=[self.pk_field])[0]
res1 = self.query(
client, self.collection_name, filter=f"{self.nullable_a_field} IS NULL", output_fields=[self.pk_field]
)[0]
res2 = self.query(client, self.collection_name,
filter=f"NOT ({self.nullable_a_field} IS NOT NULL)",
output_fields=[self.pk_field])[0]
res2 = self.query(
client,
self.collection_name,
filter=f"NOT ({self.nullable_a_field} IS NOT NULL)",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_a_is_null()
ids1 = sorted([r[self.pk_field] for r in res1])
@@ -247,13 +239,13 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: identical results
"""
client = self._client()
res1 = self.query(client, self.collection_name,
filter=f"{self.nullable_a_field} IS NOT NULL",
output_fields=[self.pk_field])[0]
res1 = self.query(
client, self.collection_name, filter=f"{self.nullable_a_field} IS NOT NULL", output_fields=[self.pk_field]
)[0]
res2 = self.query(client, self.collection_name,
filter=f"NOT ({self.nullable_a_field} IS NULL)",
output_fields=[self.pk_field])[0]
res2 = self.query(
client, self.collection_name, filter=f"NOT ({self.nullable_a_field} IS NULL)", output_fields=[self.pk_field]
)[0]
expected_ids = self.ids_a_is_not_null()
ids1 = sorted([r[self.pk_field] for r in res1])
@@ -272,9 +264,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: same as IS NULL
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"NOT (NOT ({self.nullable_a_field} IS NULL))",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"NOT (NOT ({self.nullable_a_field} IS NULL))",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_a_is_null()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -288,9 +283,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: same as NOT (IS NULL) = IS NOT NULL
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"NOT (NOT (NOT ({self.nullable_a_field} IS NULL)))",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"NOT (NOT (NOT ({self.nullable_a_field} IS NULL)))",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_a_is_not_null()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -308,9 +306,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: rows where both fields are NULL
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"({self.nullable_a_field} IS NULL) AND ({self.nullable_b_field} IS NULL)",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"({self.nullable_a_field} IS NULL) AND ({self.nullable_b_field} IS NULL)",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_both_null()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -324,9 +325,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: rows where at least one field is NULL
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"({self.nullable_a_field} IS NULL) OR ({self.nullable_b_field} IS NULL)",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"({self.nullable_a_field} IS NULL) OR ({self.nullable_b_field} IS NULL)",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_either_null()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -346,9 +350,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
Tests AND short-circuit fix in PR #47333.
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"NOT (({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_b_field} IS NOT NULL))",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"NOT (({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_b_field} IS NOT NULL))",
output_fields=[self.pk_field],
)[0]
# NOT (both NOT NULL) = at least one is NULL
expected_ids = self.ids_either_null()
@@ -363,9 +370,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: equivalent to (A IS NOT NULL) AND (B IS NOT NULL)
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"NOT (({self.nullable_a_field} IS NULL) OR ({self.nullable_b_field} IS NULL))",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"NOT (({self.nullable_a_field} IS NULL) OR ({self.nullable_b_field} IS NULL))",
output_fields=[self.pk_field],
)[0]
# NOT (either NULL) = both NOT NULL
expected_ids = self.ids_both_not_null()
@@ -385,9 +395,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
"""
client = self._client()
threshold = 250
res = self.query(client, self.collection_name,
filter=f"({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_a_field} > {threshold})",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_a_field} > {threshold})",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_a_not_null_and_gt(threshold)
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -402,9 +415,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
"""
client = self._client()
threshold = 250
res = self.query(client, self.collection_name,
filter=f"NOT (({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_a_field} < {threshold}))",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"NOT (({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_a_field} < {threshold}))",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_a_null_or_a_gte(threshold)
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -418,9 +434,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
expected: complement of rows matching inner condition
"""
client = self._client()
res = self.query(client, self.collection_name,
filter=f"NOT ((({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_a_field} > 200)) AND ({self.nullable_b_field} IS NOT NULL))",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"NOT ((({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_a_field} > 200)) AND ({self.nullable_b_field} IS NOT NULL))",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_complex_nested()
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -439,9 +458,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
"""
client = self._client()
pk_limit = 15
res = self.query(client, self.collection_name,
filter=f"({self.pk_field} < {pk_limit}) AND ({self.nullable_a_field} IS NULL)",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"({self.pk_field} < {pk_limit}) AND ({self.nullable_a_field} IS NULL)",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_pk_lt_and_a_null(pk_limit)
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -456,9 +478,12 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
"""
client = self._client()
threshold = 350
res = self.query(client, self.collection_name,
filter=f"({self.nullable_a_field} IS NULL) OR ({self.nullable_a_field} > {threshold})",
output_fields=[self.pk_field])[0]
res = self.query(
client,
self.collection_name,
filter=f"({self.nullable_a_field} IS NULL) OR ({self.nullable_a_field} > {threshold})",
output_fields=[self.pk_field],
)[0]
expected_ids = self.ids_a_null_or_a_gt(threshold)
actual_ids = sorted([r[self.pk_field] for r in res])
@@ -477,11 +502,15 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
"""
client = self._client()
vectors = cf.gen_vectors(1, self.dim)
search_res = self.search(client, self.collection_name, vectors,
filter=f"NOT ({self.nullable_a_field} IS NOT NULL)",
anns_field=self.vector_field,
limit=100,
output_fields=[self.pk_field])[0]
search_res = self.search(
client,
self.collection_name,
vectors,
filter=f"NOT ({self.nullable_a_field} IS NOT NULL)",
anns_field=self.vector_field,
limit=100,
output_fields=[self.pk_field],
)[0]
expected_ids = set(self.ids_a_is_null())
# search_res[0] is the hits for the first query vector
@@ -497,11 +526,15 @@ class TestMilvusClientThreeValuedLogic(TestMilvusClientV2Base):
"""
client = self._client()
vectors = cf.gen_vectors(1, self.dim)
search_res = self.search(client, self.collection_name, vectors,
filter=f"NOT (({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_b_field} IS NOT NULL))",
anns_field=self.vector_field,
limit=100,
output_fields=[self.pk_field])[0]
search_res = self.search(
client,
self.collection_name,
vectors,
filter=f"NOT (({self.nullable_a_field} IS NOT NULL) AND ({self.nullable_b_field} IS NOT NULL))",
anns_field=self.vector_field,
limit=100,
output_fields=[self.pk_field],
)[0]
expected_ids = set(self.ids_either_null())
# search_res[0] is the hits for the first query vector
@@ -558,32 +591,52 @@ class TestMilvusClientThreeValuedLogicIssue46972(TestMilvusClientV2Base):
schema.add_field("c16", DataType.DOUBLE, nullable=True)
schema.add_field("c17", DataType.BOOL, nullable=True)
schema.add_field("meta_json", DataType.JSON, nullable=True)
schema.add_field("tags_array", DataType.ARRAY, element_type=DataType.INT64,
max_capacity=50, nullable=True)
schema.add_field("tags_array", DataType.ARRAY, element_type=DataType.INT64, max_capacity=50, nullable=True)
self.create_collection(client, self.collection_name, schema=schema, force_teardown=False)
log.info(f"Created collection for issue #46972: {self.collection_name}")
# Create index
index_params = self.prepare_index_params(client)[0]
index_params.add_index(self.vector_field, index_type="HNSW", metric_type="L2",
params={"M": 32, "efConstruction": 256})
index_params.add_index(
self.vector_field, index_type="HNSW", metric_type="L2", params={"M": 32, "efConstruction": 256}
)
self.create_index(client, self.collection_name, index_params=index_params)
# Insert the exact test data from the issue
self.datas = [{
'id': 1051,
'vector': [0.1] * self.dim,
'c0': None, 'c1': True, 'c2': 55943, 'c3': 2379.7519128726576, 'c4': None, 'c5': True,
'c6': -57942, 'c7': False, 'c8': 'Vaxr5xzbWPHJazy9loD', 'c9': 1682.7331496087677,
'c10': None, 'c11': -62195, 'c12': False, 'c13': 71210, 'c14': None,
'c15': 2884.9004720171306, 'c16': 4866.809897346821, 'c17': True,
'meta_json': {
'price': 561, 'color': 'Green', 'active': False,
'config': {'version': 7}, 'history': [7, 20, 88], 'random_payload': 12168
},
'tags_array': [73]
}]
self.datas = [
{
"id": 1051,
"vector": [0.1] * self.dim,
"c0": None,
"c1": True,
"c2": 55943,
"c3": 2379.7519128726576,
"c4": None,
"c5": True,
"c6": -57942,
"c7": False,
"c8": "Vaxr5xzbWPHJazy9loD",
"c9": 1682.7331496087677,
"c10": None,
"c11": -62195,
"c12": False,
"c13": 71210,
"c14": None,
"c15": 2884.9004720171306,
"c16": 4866.809897346821,
"c17": True,
"meta_json": {
"price": 561,
"color": "Green",
"active": False,
"config": {"version": 7},
"history": [7, 20, 88],
"random_payload": 12168,
},
"tags_array": [73],
}
]
self.insert(client, self.collection_name, self.datas)
self.flush(client, self.collection_name)
@@ -600,40 +653,46 @@ class TestMilvusClientThreeValuedLogicIssue46972(TestMilvusClientV2Base):
def test_issue_46972_false_or_false_complex_json(self):
"""
target: test that False OR False = False (not True) with complex JSON expressions
method: use the exact expressions from issue #46972
method: use the expressions from issue #46972
expected: left expr is False, right expr is False, combined is False
This is the critical bug fixed in PR #47333: complex expressions with
JSON fields and nullable fields incorrectly evaluated (False OR False) as True.
Note: the original issue #46972 expressions contained bare `null is null`
/ `null is not null` sub-terms, which only ever "worked" by accident a
bare `null` used as a left-value was silently misparsed as a dynamic JSON
key. That misparse is now rejected at parse time (a bare `null` is not a
valid left-value), so those sub-terms are replaced with their exact
semantic equivalents `true` (`null is null`) and `false`
(`null is not null`). This keeps every sub-clause's boolean value — and
thus the False-OR-False regression coverage identical.
"""
client = self._client()
# Block A: Left Expression (should be False for test data)
expr_left = """
(((meta_json["config"]["version"] == 9 or (meta_json["history"][0] > 49 or (meta_json["history"][0] > 47 and (meta_json["price"] > 294 and meta_json["price"] < 379)))) and (((meta_json["config"]["version"] == 7 or meta_json["history"][0] > 27) or (tags_array is not null or null is null)) or c17 == false)) or ((((c14 <= 863.28694295187 or (c10 != "kaKmPWPbAaEFnHzX" and c10 != "ZmBmv")) and ((c4 > 1113.2711377477458 or c13 >= -76839) or (meta_json["config"]["version"] == 3 or (meta_json["price"] > 150 and meta_json["price"] < 237)))) and c7 == false) and ((((c1 is not null and null is null) or (c12 == false and meta_json["config"]["version"] == 4)) or (((meta_json["active"] == true and meta_json["color"] == "Blue") and c12 == true) and (c6 is not null or c13 == 180192))) or (c4 < 105376.953125 or ((c15 <= 3484.876420871185 or c1 == false) or (meta_json["config"]["version"] == 9 and c10 != "OjcbvwxZ5LfV2PZNPgS7"))))))
(((meta_json["config"]["version"] == 9 or (meta_json["history"][0] > 49 or (meta_json["history"][0] > 47 and (meta_json["price"] > 294 and meta_json["price"] < 379)))) and (((meta_json["config"]["version"] == 7 or meta_json["history"][0] > 27) or (tags_array is not null or true)) or c17 == false)) or ((((c14 <= 863.28694295187 or (c10 != "kaKmPWPbAaEFnHzX" and c10 != "ZmBmv")) and ((c4 > 1113.2711377477458 or c13 >= -76839) or (meta_json["config"]["version"] == 3 or (meta_json["price"] > 150 and meta_json["price"] < 237)))) and c7 == false) and ((((c1 is not null and true) or (c12 == false and meta_json["config"]["version"] == 4)) or (((meta_json["active"] == true and meta_json["color"] == "Blue") and c12 == true) and (c6 is not null or c13 == 180192))) or (c4 < 105376.953125 or ((c15 <= 3484.876420871185 or c1 == false) or (meta_json["config"]["version"] == 9 and c10 != "OjcbvwxZ5LfV2PZNPgS7"))))))
"""
# Block B: Right Expression (should be False for test data)
expr_right = """
((((((c17 == false or exists(meta_json["non_exist"])) or c14 <= 5363.7872388701035) or meta_json["history"][0] > 72) or (((c4 is not null and c17 == true) and ((c2 < 60835 or c1 is null) or (c5 == false or meta_json["history"][0] > 64))) and ((c6 >= -56376 or c16 is not null) or ((c4 >= 812.9318963654309 and c16 >= 2907.7772038042867) and (meta_json["price"] > 449 and meta_json["price"] < 624))))) or (((((null is not null and c13 < 180192) or (c13 < -71746 or c17 == true)) and ((meta_json["price"] > 407 and meta_json["price"] < 521) and (c8 like "w%" or c14 > 3021.9496428003613))) and (((c12 == false or (meta_json["price"] > 412 and meta_json["price"] < 571)) or (c9 < 1264.220988053753 or c6 != 2063)) or c5 == false)) and (meta_json["active"] == true and meta_json["color"] == "Red"))) or ((((c13 != 54759 and (c7 is null and (c17 == true or c9 <= 1067.5165787120216))) and (((c11 != 36936 or meta_json["history"][0] > 59) or ((meta_json["price"] > 354 and meta_json["price"] < 528) and c9 >= 135.84002581554114)) and ((c0 > 105381.9375 or exists(meta_json["non_exist"])) or (c12 == true or meta_json is null)))) or ((((c14 >= 2547.2285277180335 and c1 == false) and (meta_json["history"][0] > 37 or json_contains(meta_json["k_11"], "o"))) and ((c9 < 2869.0647504243566 and c4 > 449.2640493406897) or (meta_json["config"]["version"] == 9 and c16 < 3626.0660282789318))) and (((c11 <= -57792 or (meta_json["price"] > 320 and meta_json["price"] < 488)) and (meta_json["active"] == false and c3 >= 105383.3671875)) or ((c13 >= -32496 and c5 == true) and (meta_json["active"] == true and meta_json["color"] == "Blue"))))) and c13 <= 180192))
((((((c17 == false or exists(meta_json["non_exist"])) or c14 <= 5363.7872388701035) or meta_json["history"][0] > 72) or (((c4 is not null and c17 == true) and ((c2 < 60835 or c1 is null) or (c5 == false or meta_json["history"][0] > 64))) and ((c6 >= -56376 or c16 is not null) or ((c4 >= 812.9318963654309 and c16 >= 2907.7772038042867) and (meta_json["price"] > 449 and meta_json["price"] < 624))))) or (((((false and c13 < 180192) or (c13 < -71746 or c17 == true)) and ((meta_json["price"] > 407 and meta_json["price"] < 521) and (c8 like "w%" or c14 > 3021.9496428003613))) and (((c12 == false or (meta_json["price"] > 412 and meta_json["price"] < 571)) or (c9 < 1264.220988053753 or c6 != 2063)) or c5 == false)) and (meta_json["active"] == true and meta_json["color"] == "Red"))) or ((((c13 != 54759 and (c7 is null and (c17 == true or c9 <= 1067.5165787120216))) and (((c11 != 36936 or meta_json["history"][0] > 59) or ((meta_json["price"] > 354 and meta_json["price"] < 528) and c9 >= 135.84002581554114)) and ((c0 > 105381.9375 or exists(meta_json["non_exist"])) or (c12 == true or meta_json is null)))) or ((((c14 >= 2547.2285277180335 and c1 == false) and (meta_json["history"][0] > 37 or json_contains(meta_json["k_11"], "o"))) and ((c9 < 2869.0647504243566 and c4 > 449.2640493406897) or (meta_json["config"]["version"] == 9 and c16 < 3626.0660282789318))) and (((c11 <= -57792 or (meta_json["price"] > 320 and meta_json["price"] < 488)) and (meta_json["active"] == false and c3 >= 105383.3671875)) or ((c13 >= -32496 and c5 == true) and (meta_json["active"] == true and meta_json["color"] == "Blue"))))) and c13 <= 180192))
"""
# Combined expression
expr_combined = f"({expr_left}) OR ({expr_right})"
# Query with left expression
res_left = self.query(client, self.collection_name, filter=expr_left,
output_fields=[self.pk_field])[0]
res_left = self.query(client, self.collection_name, filter=expr_left, output_fields=[self.pk_field])[0]
is_left_true = len(res_left) > 0
# Query with right expression
res_right = self.query(client, self.collection_name, filter=expr_right,
output_fields=[self.pk_field])[0]
res_right = self.query(client, self.collection_name, filter=expr_right, output_fields=[self.pk_field])[0]
is_right_true = len(res_right) > 0
# Query with combined expression
res_combined = self.query(client, self.collection_name, filter=expr_combined,
output_fields=[self.pk_field])[0]
res_combined = self.query(client, self.collection_name, filter=expr_combined, output_fields=[self.pk_field])[0]
is_combined_true = len(res_combined) > 0
log.info(f"Expr Left result: {is_left_true} (hits: {len(res_left)})")
@@ -645,8 +704,9 @@ class TestMilvusClientThreeValuedLogicIssue46972(TestMilvusClientV2Base):
assert not is_right_true, "Right expression should be False for test data"
# The critical check: False OR False should NOT be True
assert not is_combined_true, \
assert not is_combined_true, (
f"BUG: (False OR False) evaluated to True! Combined result has {len(res_combined)} hits"
)
@pytest.mark.tags(CaseLabel.L2)
def test_issue_46972_simplified_false_or_false(self):
@@ -666,12 +726,9 @@ class TestMilvusClientThreeValuedLogicIssue46972(TestMilvusClientV2Base):
expr_right = "c4 > 100"
expr_combined = f"({expr_left}) OR ({expr_right})"
res_left = self.query(client, self.collection_name, filter=expr_left,
output_fields=[self.pk_field])[0]
res_right = self.query(client, self.collection_name, filter=expr_right,
output_fields=[self.pk_field])[0]
res_combined = self.query(client, self.collection_name, filter=expr_combined,
output_fields=[self.pk_field])[0]
res_left = self.query(client, self.collection_name, filter=expr_left, output_fields=[self.pk_field])[0]
res_right = self.query(client, self.collection_name, filter=expr_right, output_fields=[self.pk_field])[0]
res_combined = self.query(client, self.collection_name, filter=expr_combined, output_fields=[self.pk_field])[0]
log.info(f"Simplified test - Left: {len(res_left)}, Right: {len(res_right)}, Combined: {len(res_combined)}")
@@ -679,5 +736,4 @@ class TestMilvusClientThreeValuedLogicIssue46972(TestMilvusClientV2Base):
assert len(res_left) == 0, "NULL > 100 should return 0 results"
assert len(res_right) == 0, "NULL > 100 should return 0 results"
# Combined should also be empty
assert len(res_combined) == 0, \
"Combined (NULL > 100) OR (NULL > 100) should return 0 results"
assert len(res_combined) == 0, "Combined (NULL > 100) OR (NULL > 100) should return 0 results"