ECSQL: support IS / IS NOT operator between operands (null-safe comparison) by khanaffan · Pull Request #1476 · iTwin/imodel-native

khanaffan · 2026-06-24T13:24:46Z

This pull request adds support for using the IS and IS NOT operators between two operands in ECSQL, extending their previous usage (which was limited to comparisons with NULL, boolean literals, or type predicates). Now, these operators can be used for null-safe comparisons between properties, including multi-column types like points and navigation properties, with appropriate column-wise expansion. The changes include parser, lexer, and documentation updates, as well as comprehensive tests to verify the new behavior.

ECSQL Syntax and Semantics Enhancements:

Added support for IS and IS NOT between two operands (e.g., prop1 IS [NOT] prop2), with null-safe comparison semantics consistent with SQLite. For multi-column operands, IS joins per-column comparisons with AND, and IS NOT with OR.
Updated the ECSQL version to 2.0.4.0 in documentation, code, and tests to reflect the new syntax support.

- Added support for the `IS` and `IS NOT` operators in ECSQL to handle null-safe comparisons. - Updated the ECSQL parser to recognize new boolean expressions involving `IS` and `IS NOT`. - Modified the ECSQL version to 2.0.4.0 to reflect the addition of new features. - Added unit tests to validate the behavior of the new operators, ensuring correct handling of null values and type compatibility. - Updated existing tests to align with the new ECSQL version.

…arnings

- Updated the ECSQL parser to allow the IS and IS NOT operators to accept any value expression on either side, not just NULL literals or column references. This includes support for string literals, function calls, and parameter bindings. - Modified existing tests to reflect the new behavior, ensuring that comparisons using the IS operator with various value expressions are handled correctly. - Adjusted SQL generation tests to confirm that the new semantics are correctly translated into SQL.

aruniverse

Adversarial review (automated) — 4 inline findings below, ranging from grammar-correctness question to missing conflict-count baseline. No blockers on this side of the split; the corresponding itwinjs-core PR has the BLOCKER (missing column in Comparison.ecsql.md).

…e comment - Remove the now-unused ParseTruthValue wrapper from ECSqlParser.h. It had zero callers repo-wide; the legacy IS (ClassName) path goes directly through ParseValueExp. - Reword the type_list_item grammar comment to accurately describe parse behavior: a single unqualified name in parentheses (e.g. 'X IS (S2)') reduces as a value_exp (null-safe comparison), not a type predicate. The type predicate is reached only for a qualified class name, an ONLY/ALL prefix, or a comma-separated list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

aruniverse

One follow-up from the latest round: the CHANGES.md disambiguation note (and the parallel wording in itwinjs-core's Operators.md and NextVersion.md) still says "a parenthesized (ClassName)" without specifying that the name must be qualified. The grammar comment in sqlbison.y is already accurate — this just needs to match it.

A parenthesized qualified name on the RHS of IS/IS NOT (e.g. 'X IS (alias.prop)' or 'X IS (ts.Enum.Enumerator)') is grammatically identical to the '(ClassName)' type predicate, so it previously failed with "class not found" when the name was actually a property or enum reference. ECSqlParser::TryParseParenthesizedNameAsValueExp now reinterprets it as a property-reference / enum-literal value expression (null-safe comparison) when the name does not resolve to a class. A genuine '(ClassName)' type predicate, an ONLY/ALL prefix, a comma-separated type list, or the 'schema:class' colon form keep their original type-predicate meaning. Updates the sqlbison.y grammar comment and CHANGES.md to distinguish a qualified class name (type predicate) from a property path (value expression), addressing the review note that the disambiguation must say "qualified". Adds prepare, statement-semantics (incl. enum and nested struct), and SQL-generation tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

aruniverse

Two new test-coverage gaps found after reviewing commit 593ff15eb9 (TryParseParenthesizedNameAsValueExp). Both are in the test files. Existing implementation looks correct — these are missing negative/regression tests only.

aruniverse · 2026-06-29T20:52:03Z

+    // comparison), not the '(ClassName)' type predicate, when the name does not resolve to a class.
+    EXPECT_EQ(ECSqlStatus::Success, Prepare("SELECT NULL FROM ecsql.P WHERE I IS (P.I)"));
+    EXPECT_EQ(ECSqlStatus::Success, Prepare("SELECT NULL FROM ecsql.P p WHERE p.I IS (p.I)"));
+    EXPECT_EQ(ECSqlStatus::Success, Prepare("SELECT NULL FROM ecsql.P p WHERE p.I IS NOT (p.I)"));


Missing error test: IS (alias.NonExistentProperty) — what happens when the name is neither a class nor a valid property?

The three lines above confirm that IS (P.I) / IS NOT (p.I) prepare successfully. But there is no test for the case where the parenthesized name passes the class-lookup check (returns null), falls through TryParseParenthesizedNameAsValueExp, and then constructs a PropertyNameExp for a name that does not exist as a property.

For example: I IS (P.NonExistentProp) — P.NonExistentProp is not a class, so TryParseParenthesizedNameAsValueExp returns SUCCESS with a PropertyNameExp. But what does downstream semantic validation do? Without a test, the failure mode (and its error message) is unanchored.

Suggested additions:

// parenthesized name that is neither a class nor a property → prepare error EXPECT_EQ(ECSqlStatus::InvalidECSql, Prepare("SELECT NULL FROM ecsql.P WHERE I IS (P.NonExistentProp)")); // parenthesized enum name where the enumerator does not exist → prepare error (not a silent pass) // EXPECT_EQ(ECSqlStatus::InvalidECSql, Prepare("..."));

Added in e2ba404. Two InvalidECSql cases in ECSqlSelectPrepareTests::WhereBasics, right after the IS (P.I) lines:

EXPECT_EQ(ECSqlStatus::InvalidECSql, Prepare("SELECT NULL FROM ecsql.P WHERE I IS (P.NonExistentProp)")); EXPECT_EQ(ECSqlStatus::InvalidECSql, Prepare("SELECT NULL FROM ecsql.P p WHERE p.I IS NOT (p.NonExistentProp)"));

P.NonExistentProp is not a class, so TryParseParenthesizedNameAsValueExp returns a PropertyNameExp, which then fails property resolution during finalize — i.e. the failure mode is a normal "property not found" prepare error, not a silent pass (same behavior as WHERE Garbage = 'bla').

The ecsql.P schema has no enumeration, so I anchored the enum failure mode you mentioned in ECSqlStatementTests::IsAndIsNotOperatorNullSafeSemantics (which has a Status enum) instead:

EXPECT_NE(ECSqlStatus::Success, badProp.Prepare(m_ecdb, "SELECT 1 FROM ts.Foo WHERE S1 IS (Foo.NonExistentProp)")); EXPECT_NE(ECSqlStatus::Success, badEnum.Prepare(m_ecdb, "SELECT 1 FROM ts.Foo WHERE Status IS (ts.Status.DoesNotExist)"));

Partially addressed in e2ba404e56: the missing-property cases were added in ECSqlPrepareTests.cpp, with matching missing-property and missing-enumerator cases in ECSqlStatementTests.cpp. One part of the original concern remains: these assertions check only InvalidECSql / non-success, so the pre-fix "class not found" path would also pass. Please capture the issue (for example with TestIssueListener) and assert property/enumerator resolution—or at least assert that this is not the old class-resolution failure—before resolving the thread.

aruniverse · 2026-06-29T20:52:03Z

+    // multi-column point operand still expands column-wise through the parenthesized property reference
+    assertWhere("SELECT 1 FROM ts.Foo WHERE P1 IS (Foo.P2)",
+                "WHERE ([Foo].[P1_X] IS [Foo].[P2_X] AND [Foo].[P1_Y] IS [Foo].[P2_Y] AND [Foo].[P1_Z] IS [Foo].[P2_Z])");
+    }


Missing regression tests: IS (ONLY ts.Foo) and IS (ts.Foo, ts.Bar) after grammar refactor

The grammar refactored type_list_item from opt_only table_node into three explicit alternatives (including new SQL_TOKEN_ONLY and SQL_TOKEN_ALL rules). The semantic guard in TryParseParenthesizedNameAsValueExp bails out via opt_only->count() != 0 to keep ONLY/ALL forms as type predicates. This is correct by inspection, but no SQL-generation test verifies the end-to-end output after the refactor.

Suggested additions at the end of this test:

// ONLY-prefixed and comma-separated type-list forms must still produce type-predicate SQL // after the type_list_item grammar refactor — not fall into TryParseParenthesizedNameAsValueExp EXPECT_TRUE(GetHelper().ECSqlToSql("SELECT 1 FROM ts.Foo WHERE ECClassId IS (ONLY ts.Foo)").Contains("ec_cache_ClassHierarchy")); EXPECT_TRUE(GetHelper().ECSqlToSql("SELECT 1 FROM ts.Foo WHERE ECClassId IS (ts.Foo, ts.Bar)").Contains("ec_cache_ClassHierarchy"));

Added in e2ba404 — with one correction to the suggested assertions.

IS (ONLY ts.Foo) does not produce ec_cache_ClassHierarchy. PrepareTypeListExp has an exact-match optimization: a list containing only non-polymorphic (ONLY) classes is emitted as a direct IN (<classId>) filter and never touches ec_cache_ClassHierarchy (that table is only used when the list has a polymorphic entry, via BaseClassId IN (...)). So checking for ec_cache_ClassHierarchy on the ONLY form would fail.

What I added (a Bar class was added to the schema for the comma-list cases):

// ONLY (non-polymorphic) -> exact IN(<id>) filter, no ec_cache_ClassHierarchy Utf8String onlySql = GetHelper().ECSqlToSql("SELECT 1 FROM ts.Foo WHERE ECClassId IS (ONLY ts.Foo)"); EXPECT_FALSE(onlySql.empty()); // still prepares as a type predicate EXPECT_FALSE(onlySql.Contains("ec_cache_ClassHierarchy")); // ... same for (ONLY ts.Foo, ONLY ts.Bar) // polymorphic comma-separated list -> ec_cache_ClassHierarchy EXPECT_TRUE(GetHelper().ECSqlToSql("SELECT 1 FROM ts.Foo WHERE ECClassId IS (ts.Foo, ts.Bar)").Contains("ec_cache_ClassHierarchy")); EXPECT_TRUE(GetHelper().ECSqlToSql("SELECT 1 FROM ts.Foo WHERE ECClassId IS NOT (ts.Foo, ts.Bar)").Contains("ec_cache_ClassHierarchy"));

EXPECT_FALSE(empty()) confirms the ONLY/comma-list forms still route through the type-predicate path (they'd yield an empty string if they had misparsed and failed), and the poly-list cases pin the ec_cache_ClassHierarchy output — so the type_list_item refactor and the TryParseParenthesizedNameAsValueExp opt_only/comma-list guards are both exercised end-to-end.

The original requested regressions are addressed in e2ba404e56: IS (ONLY ts.Foo) and an all-ONLY list are verified as exact matches without ec_cache_ClassHierarchy, while polymorphic comma lists for IS and IS NOT are verified to use the hierarchy table. The correction to the suggested ONLY assertion is valid. I posted one separate follow-up because the new test comment claims ALL coverage, but the distinct SQL_TOKEN_ALL grammar branch is still untested.

- ECSqlPrepareTests: add InvalidECSql cases for 'X IS (alias.NonExistentProp)' (parenthesized qualified name that is neither a class nor an existing property falls through TryParseParenthesizedNameAsValueExp to a PropertyNameExp that fails resolution, rather than being silently accepted). - ECSqlStatementTests: add matching negative cases for a missing property and a missing enum enumerator '(ts.Status.DoesNotExist)'. - ECSqlToSqlGenerationTests: add a Bar class and regression tests confirming the type_list_item grammar refactor keeps the ONLY/ALL and comma-separated type-list forms routing through the type-predicate path (ONLY -> exact 'IN (<id>)' with no ec_cache_ClassHierarchy; polymorphic comma-list -> ec_cache). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

soham-bentley · 2026-07-13T13:46:29Z

+BentleyStatus ECSqlParser::TryParseParenthesizedNameAsValueExp(std::unique_ptr<ValueExp>& valueExp, OSQLParseNode const* typePredicateNode) const
+    {
+    valueExp = nullptr;
+    if (!SQL_ISRULE(typePredicateNode, type_predicate))


This method has no code path which returns in ERROR, but I think here if the typePredicateNode is not a type_predicate, we should error out.

soham-bentley · 2026-07-13T13:48:24Z

+        return SUCCESS; // an ONLY/ALL prefix is always a genuine type predicate
+
+    OSQLParseNode const* tableNode = type_list_item->getChild(1/*table_node*/);
+    if (!SQL_ISRULE(tableNode, table_node))


If the tableNode is not of type table_node, shouldn't we error out?

soham-bentley · 2026-07-13T13:53:17Z

+        {
+        Utf8StringCR tableSpaceName = nameNode->getChild(0)->getTokenValue();
+        OSQLParseNode const* qualifiedNameNode = nameNode->getChild(2/*qualified_class_name*/);
+        if (!SQL_ISRULE(qualifiedNameNode, qualified_class_name))


Here also same, if qualifiedNameNode is not a qualified_class_name shouldn't we error out or atleast continue with the execution as is but not branch out on SUCCESS? This method just branches out SUCCESS when we are sure that the node is a className somehow, right?

aruniverse · 2026-07-23T19:45:27Z

+    assertWhere("SELECT 1 FROM ts.Foo WHERE P1 IS (Foo.P2)",
+                "WHERE ([Foo].[P1_X] IS [Foo].[P2_X] AND [Foo].[P1_Y] IS [Foo].[P2_Y] AND [Foo].[P1_Z] IS [Foo].[P2_Z])");
+
+    // regression: the type_list_item grammar refactor (opt_only is now inlined into three explicit


Missing ALL regression despite this comment claiming ONLY / ALL coverage. sqlbison.y has a distinct SQL_TOKEN_ALL table_node alternative, but this block exercises only ONLY and unprefixed comma lists; there is no IS (ALL ...) case repo-wide. Please add at least IS (ALL ts.Foo) (and preferably IS NOT (ALL ts.Foo)) and assert it remains on the polymorphic type-predicate path, e.g. via ec_cache_ClassHierarchy.

aruniverse · 2026-07-23T19:45:28Z

+## ## `06/23/2026`: Added `IS` / `IS NOT` operator between operands
+* ECSql version change `2.0.3.2` -> `2.0.4.0`.
+* The `IS` and `IS NOT` operators can now be used between two operands, e.g. `prop1 IS [NOT] prop2`. Each operand may be any value expression — a property, the `NULL` literal, a constant, a parameter, a function call, an arithmetic expression, etc. These map to SQLite's null-safe comparison operators (`NULL IS NULL` is true, `1 IS NULL` is false).
+* Previously `IS` / `IS NOT` only supported the right-hand operands `NULL`, the boolean literals `TRUE`/`FALSE`/`UNKNOWN`, and the class type predicate `IS (ClassName)`. Those forms are unchanged: a right-hand operand that is exactly `NULL`/`TRUE`/`FALSE`/`UNKNOWN`, or a parenthesized name that resolves to a class — a qualified `(schema.Class)`, an `ONLY`/`ALL`-prefixed form, or a comma-separated list — keeps its original type-predicate meaning. A parenthesized name that does **not** resolve to a class, e.g. a property reference `(alias.prop)`, is parsed as an ordinary value expression (null-safe comparison), exactly as it would be without the parentheses. When a parenthesized qualified name is both a class and a valid property path, the class (type-predicate) reading wins.


Please narrow the fallback wording to dot-qualified names. TryParseParenthesizedNameAsValueExp explicitly excludes the schema:class and tablespace.schema:class colon forms; an unresolved colon-form name remains a type predicate and fails class resolution instead of becoming a value expression. As written, "a parenthesized name that does not resolve to a class" overstates the behavior. Please say dot-qualified (for example alias.prop) and mirror the constraint in the linked core docs.

khanaffan added 2 commits June 23, 2026 11:34

Refactor SQL parser build scripts to suppress known bison conflicts w…

8244641

…arnings

khanaffan mentioned this pull request Jun 24, 2026

ECSQL: support IS / IS NOT operator between operands (null-safe comparison) iTwin/itwinjs-core#9440

Open

khanaffan marked this pull request as ready for review June 24, 2026 16:45

khanaffan requested review from RohitPtnkr1996, calebmshafer, pmconne and rschili as code owners June 24, 2026 16:45

Merge branch 'main' into affank/support-is-op

0c1b839

aruniverse reviewed Jun 25, 2026

View reviewed changes

Comment thread iModelCore/ECDb/ECDb/ECSql/Parser/sqlbison.y Outdated

Comment thread iModelCore/ECDb/ECDb/ECSql/ECSqlParser.cpp

Comment thread iModelCore/ECDb/Scripts/BuildECSqlParser.sh

Comment thread iModelCore/ECDb/Tests/NonPublished/ECSqlPrepareTests.cpp

Copilot started work on behalf of khanaffan June 25, 2026 18:51 View session

Add explanatory comments to UPDATE/DELETE IS operator test blocks

9b17160

Copilot finished work on behalf of khanaffan June 25, 2026 18:53

aruniverse reviewed Jun 26, 2026

View reviewed changes

Comment thread iModelCore/ECDb/CHANGES.md Outdated

Merge branch 'main' into affank/support-is-op

5441fee

aruniverse added this to the iTwin.js 5.12 milestone Jun 29, 2026

aruniverse reviewed Jun 29, 2026

View reviewed changes

khanaffan and others added 2 commits July 7, 2026 13:22

Merge branch 'main' into affank/support-is-op

e1fcba9

soham-bentley reviewed Jul 13, 2026

View reviewed changes

Merge branch 'main' into affank/support-is-op

3d6aed2

aruniverse reviewed Jul 23, 2026

View reviewed changes

aruniverse modified the milestones: iTwin.js 5.12, iTwin.js 5.13 Jul 27, 2026

Uh oh!

Conversation

khanaffan commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aruniverse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aruniverse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aruniverse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

khanaffan commented Jun 24, 2026 •

edited

Loading