Skip to content

sql: Avoid SQL-type mismatches (and panic) when repr types match#35005

Merged
ggevay merged 2 commits intoMaterializeInc:mainfrom
mgree:repr-types-missed-relation-assertion
Feb 13, 2026
Merged

sql: Avoid SQL-type mismatches (and panic) when repr types match#35005
ggevay merged 2 commits intoMaterializeInc:mainfrom
mgree:repr-types-missed-relation-assertion

Conversation

@mgree
Copy link
Contributor

@mgree mgree commented Feb 13, 2026

Motivation

#34958 removed many possible panics; #35003 sussed out another.

Description

We should only panic when types well and truly don't match---if SQL types don't align but repr types do, there is no possibility of runtime error.

Verification

Nightly, baby! This failure got sussed out by #35003, which pushes repr types into many new locations---it is likely not a reachable failure in the existing code.

@mgree mgree requested a review from a team as a code owner February 13, 2026 16:09
@github-actions
Copy link

github-actions bot commented Feb 13, 2026

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

@mgree mgree changed the title sql: Remove a repr-types-oriented panic from an assertion sql: Avoid SQL-type mismatches (and panic) when repr types match Feb 13, 2026
…e nullability correctly, add regression test
@mgree mgree force-pushed the repr-types-missed-relation-assertion branch from f50dcb6 to 4641711 Compare February 13, 2026 19:18
@mgree mgree requested a review from ggevay February 13, 2026 19:32
@ggevay ggevay merged commit 3bc4f7e into MaterializeInc:main Feb 13, 2026
162 checks passed
ggevay pushed a commit that referenced this pull request Feb 13, 2026
### Motivation

#27239

### Description

Changes `MirRelationExpr::typ()` to actually do repr type inference, not
SQL type. While it still returns a SQL type (to be fixed in a later PR),
it's a _canonical_ SQL type, obtained by mapping repr types back up to
SQL types.

This draft PR is on top of
#35005, which should
merge first.

### Verification

Currently WIP.

Will with nightly that
#34958 successfully
avoids all panics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants