[repr types] Fallback to repr type checks before failing by mgree · Pull Request #34958 · MaterializeInc/materialize

mgree · 2026-02-09T16:31:53Z

Recent (reverted) changes to ColumnKnowledge and enable_cast_elimination tickled some assertions we have in our code that assume we will have SQL typechecking at the MIR level. This PR relaxes those checks to try for repr typing as a fallback, using tracing::error! to notify when this fallback occurs (so we can track it in Sentry).

There were two places these checks were happening:

In SqlColumnType::union, which was frequently .unwrap()ed.
In an assertion for normalize_lets.

Scanning through 1055 assertions in the expr, repr, sql, and transform crates found no others, nor were the 42 uses of base_eq across the codebase suspect.

Motivation

This PR fixes a recognized bug.
https://github.com/MaterializeInc/database-issues/issues/10045
https://github.com/MaterializeInc/database-issues/issues/10046
https://github.com/MaterializeInc/database-issues/issues/10052

Checklist

This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
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).
If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

ggevay · 2026-02-09T23:06:53Z

For the test fails:

💡 SQL logic tests (4 replicas) 4: You could put your file into tests_without_views_and_replica, so that it's not run with multiple replicas.
🏎️ testdrive 4 replicas: Just a rare flake, I've hit retry.
Orchestratord + upgrade, combine props: common flake

ggevay · 2026-02-09T22:56:01Z

src/expr/src/scalar.rs

                                    && e1
                                        .typ(column_types)
-                                        .union(&e2.typ(column_types))
+                                        .sql_union(&e2.typ(column_types))


Here and the below two changed calls, why not try_union? I think the important thing for these calls (see code comment above) is to match typ's handling of If, which does a proper repr union, not a sql_union.

Good catch, thank you! I found two more instances in there. I thought I got all these... I must have not saved properly or something.

I'll trigger another nightly run including the relevant tests (sqllogictest with and without multiple replicas, testdrives) and then merge.

This PR combines two bugfixes to give the best fix, allowing for multireplica tests. ### Motivation #34966 fixes an SLT test from #34905 that broke in nightly when run with multiple replicas. The fix there is better than the fix I put in #34958, which forces the test to only run in single replica mode. ### Description Takes the `singlereplica_` out of the name, so we'll actually run multi-replica tests. ### Verification Running appropriate 4-replica slt tests from nightly.

) ### 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.

### 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.

…erializeInc#34958)" This reverts commit 201dcae.

…Inc#34958) Recent ([reverted]()) changes to [`ColumnKnowledge`]() and [`enable_cast_elimination`]() tickled some assertions we have in our code that assume we will have SQL typechecking at the MIR level. This PR relaxes those checks to try for repr typing as a fallback, using `tracing::error!` to notify when this fallback occurs (so we can track it in Sentry). There were two places these checks were happening: - In `SqlColumnType::union`, which was frequently `.unwrap()`ed. - In an assertion for `normalize_lets`. Scanning through 1055 assertions in the `expr`, `repr`, `sql`, and `transform` crates found no others, nor were the 42 uses of `base_eq` across the codebase suspect. ### Motivation * This PR fixes a recognized bug. MaterializeInc/database-issues#10045 MaterializeInc/database-issues#10046 MaterializeInc/database-issues#10052

…nc#34991) This PR combines two bugfixes to give the best fix, allowing for multireplica tests. ### Motivation MaterializeInc#34966 fixes an SLT test from MaterializeInc#34905 that broke in nightly when run with multiple replicas. The fix there is better than the fix I put in MaterializeInc#34958, which forces the test to only run in single replica mode. ### Description Takes the `singlereplica_` out of the name, so we'll actually run multi-replica tests. ### Verification Running appropriate 4-replica slt tests from nightly.

…erializeInc#35005) ### Motivation MaterializeInc#34958 removed many possible panics; MaterializeInc#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 MaterializeInc#35003, which pushes repr types into many new locations---it is likely not a reachable failure in the existing code.

…c#35003) ### Motivation MaterializeInc#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 MaterializeInc#35005, which should merge first. ### Verification Currently WIP. Will with nightly that MaterializeInc#34958 successfully avoids all panics.

mgree force-pushed the repr-types-check-with-fallback branch from 889b157 to 49786cd Compare February 9, 2026 19:47

mgree marked this pull request as ready for review February 9, 2026 20:34

mgree requested review from a team as code owners February 9, 2026 20:34

mgree requested a review from ggevay February 9, 2026 20:34

ggevay approved these changes Feb 9, 2026

View reviewed changes

mgree added 8 commits February 10, 2026 14:10

rename SqlColumnType::union to ::sql_union

48fff07

missing docs

255bb50

add repr-type fallback logic to union

6660e35

fallback for assertion in normalize_lets

b12d6b5

add tests with enable_cast_elimination

26477a4

follow @ggevay's advice on avoiding multi-replica testing

397129d

change some sql_unions to try_unions

d05cb33

force singlereplica

3785218

mgree force-pushed the repr-types-check-with-fallback branch from 3235085 to 3785218 Compare February 10, 2026 19:10

mgree enabled auto-merge (squash) February 10, 2026 19:11

mgree merged commit 201dcae into MaterializeInc:main Feb 10, 2026
133 checks passed

This was referenced Feb 12, 2026

slt: Allow multireplica in github-10045-10046-10052.slt #34991

Merged

slt: Fix newly introduced github-10045-10046-10052.slt #34966

Merged

This was referenced Feb 13, 2026

sql: Force MRE::typ() to return canonical repr types #35003

Merged

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

Merged

def- added a commit to def-/materialize that referenced this pull request Feb 17, 2026

Revert "[repr types] Fallback to repr type checks before failing (Mat…

f1b325f

…erializeInc#34958)" This reverts commit 201dcae.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[repr types] Fallback to repr type checks before failing#34958

[repr types] Fallback to repr type checks before failing#34958
mgree merged 8 commits intoMaterializeInc:mainfrom
mgree:repr-types-check-with-fallback

mgree commented Feb 9, 2026 •

edited

Loading

Uh oh!

ggevay commented Feb 9, 2026

Uh oh!

ggevay Feb 9, 2026

Uh oh!

mgree Feb 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mgree commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Checklist

Uh oh!

ggevay commented Feb 9, 2026

Uh oh!

ggevay Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

mgree Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mgree commented Feb 9, 2026 •

edited

Loading