GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [SQL] Compare MAP_CONTAINS_KEY's key with the map's keys in a common type by gz · Pull Request #7740 · feldera/feldera

Conversation

@gz

@gz gz commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

MAP_CONTAINS_KEY(MAP[ARRAY[1], ARRAY[2]], a) with a INT ARRAY passed an array of nullable INT where the map's keys are arrays of INT NOT NULL, and the Rust code failed to compile with "mismatched types". The key and the map are now cast to a common key type, as ARRAY_CONTAINS does for its element. That map cast moved an array or map element out of its reference; the element converter now clones it, as it already cloned a base-type element. Extends CalciteSqlOperatorTest#testMapContainsKey; the cast, array, map, variant, and struct test classes pass.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@gz
[SQL] Compare MAP_CONTAINS_KEY's key with the map's keys in a common …
a93e09d
…type

MAP_CONTAINS_KEY(MAP[ARRAY[1], ARRAY[2]], a) with a INT ARRAY passed an array of nullable INT where
the map's keys are arrays of INT NOT NULL, and the Rust code failed to compile with "mismatched
types". The key and the map are now cast to a common key type, as ARRAY_CONTAINS does for its
element. That map cast moved an array or map element out of its reference; the element converter
now clones it, as it already cloned a base-type element.

Signed-off-by: Gerd Zellweger <[email protected]>
@gz
gz requested a review from mihaibudiu October 30, 2026 14:31
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread ...er/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/ExpressionCompiler.java
arg0 = arg0.cast(node, new DBSPTypeMap(keyType, map.getValueType(), arg0.type.mayBeNull),
DBSPCastExpression.CastType.SqlUnsafe);
arg1 = arg1.cast(node, keyType.withMayBeNull(arg1.type.mayBeNull),
DBSPCastExpression.CastType.SqlUnsafe);

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug (medium): ROW keys still fail to compile with "mismatched types", but now the failure is in the result type rather than the key. SELECT map_contains_key(MAP[ROW(1, 2), 1], ROW(1, NULLIF(2, 0))) now produces a valid map_map__(...) key cast. The problem is that Calcite infers BOOLEAN (nullable) here, and map_contains_key__ returns bool:

expected `Option<bool>`, found `bool`

ARRAY_CONTAINS already handles this ("Calcite's type inference returns nullable results if arg1 is nullable"): it builds the call with type.withMayBeNull(arg0.mayBeNull) and then casts to type. Doing the same in the return below would fix it. Please also add the ROW case to the test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@mihaibudiu mihaibudiu Oct 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good point to address.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mythical-fred-oss

mythical-fred-oss Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

What I ran: ./build.sh, then mvn test -pl SQL-compiler on CalciteSqlOperatorTest#testMapContainsKey (passes) and an extra QA class of my own:

Case Result
INT ARRAY table column against MAP[ARRAY[1], …] keys (compiled to Rust) ✅
Opposite direction: map keys ARRAY[1, NULL], arg ARRAY[2] ✅
Nested ARRAY[ARRAY[NULLIF(1,0)]] ✅
ARRAY[ROW(1, NULLIF('a','b'))] keys, ARRAY[NULLIF('a','b')] keys ✅
Nullable map values ✅
ROW(1, NULLIF(2,0)) against ROW(1,2) keys ❌ Rust mismatched types (see inline)

missing (medium): The PR's tests only cover INT ARRAY keys built from constants. Please add the ROW-key case once it's fixed, plus a non-constant case (a table column as the key). Suggested additions to testMapContainsKey:

+                select map_contains_key(map[array[array(1)], 1], array[array[nullif(1, 0)]]);
+                 r
+                ---
+                 t
+                (1 row)
+
+                select map_contains_key(map[row(1, 2), 1], row(1, nullif(2, 0)));
+                 r
+                ---
+                 t
+                (1 row)

I found no other gate failures. It's a compiler bug fix, so no docs or breaking-change concerns, and there's nothing flaky.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mihaibudiu
mihaibudiu requested changes Oct 30, 2026
View reviewed changes
@gz
[SQL] Give MAP_CONTAINS_KEY over a ROW key Calcite's result nullability
ca32396
MAP_CONTAINS_KEY(MAP[ROW(1, 2), 1], ROW(1, NULLIF(2, 0))) failed to compile with "mismatched
types": Calcite infers a nullable BOOLEAN when a ROW argument has a nullable field, but
map_contains_key__ returns bool for a non-null map and key. The call now has the nullability of
its arguments and is cast to Calcite's type, as ARRAY_CONTAINS does. testMapContainsKey covers
ROW and nested ARRAY keys, and testMapContainsKeyTable takes the keys from table columns.

Signed-off-by: Gerd Zellweger <[email protected]>
@gz
gz requested a review from mihaibudiu October 30, 2026 23:49
mihaibudiu
mihaibudiu approved these changes Oct 31, 2026
View reviewed changes
@mihaibudiu
mihaibudiu added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit 9075517 Oct 31, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu
mihaibudiu deleted the gz/map-contains-key-common-type branch October 31, 2026 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@gz @mihaibudiu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.