Skip to content

fix: from_text sets relation type earlier#6066

Open
kgutwin wants to merge 2 commits into
PRQL:mainfrom
bioteam:kg/5988/zero-row-join-fix
Open

fix: from_text sets relation type earlier#6066
kgutwin wants to merge 2 commits into
PRQL:mainfrom
bioteam:kg/5988/zero-row-join-fix

Conversation

@kgutwin

@kgutwin kgutwin commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Resolves #5988 by populating the type of the Expr returned by from_text earlier in the resolver flow, and adds a covering test.

While I was looking at it, I attempted to improve some of the error handling of infer_type_of_special_func (where the initial crash was located). There are many .unwrap() calls in the various branches of the match statement; I converted some of them to the ? try operator and/or replaced .unwrap() with .ok_or(...)?. The main challenge here is that many of these error conditions are "never supposed to happen" because the type system is supposed to catch bad user input. I don't know if there's a better/recommended strategy for handling this sort of situation -- crashing on unwrapping a None or Err is not good, but hand-writing an individual error message to replace each unwrap call seems excessive. Suggestions are welcome...

@prql-bot prql-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The core fix looks right — I checked out the branch, ran the exact #5988 repro (zero-row from_text format:json joined side:left), and it now compiles to the expected SQL; the full from_text suite plus cargo test -p prqlc and clippy all pass. Setting ty on the result so the empty array resolves to Array(Some(Tuple(..))) instead of Array(None) is the clean place to fix it.

On your question about the "never supposed to happen" branches — this repo already has a convention for exactly that. Error::new_assert(details) renders as internal compiler error; <details>, and Error::new_bug(issue_no) adds a tracking-issue link (see the uses in prqlc/prqlc/src/semantic/lowering.rs and functions.rs:286). Using new_assert for the pure type-kind invariants (the into_tuple/into_function/into_array wrong-kind cases) frames them as compiler bugs rather than user errors, and keeps the {value:#?} debug dump behind that framing. The genuinely user-reachable cases (e.g. a join with that really has no columns) are fine as new_simple.

One thing worth reconsidering is the blanket impl From<TyKind> for Error (flagged inline) — scoping the conversion locally per call site would answer both this and the message-style question in one move.

Comment thread prqlc/prqlc-parser/src/parser/pr/types.rs Outdated
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.

Compiler crash when joining a zero-row relation

2 participants