8000 fix: refuse null for a parameter the statement takes no null for by colinhacks · Pull Request #75 · colinhacks/reylo · GitHub
[go: up one dir, main page]
Skip to content

fix: refuse null for a parameter the statement takes no null for - #75

Merged
colinhacks merged 4 commits into
mainfrom
runtime-null
Oct 23, 2026
Merged

fix: refuse null for a parameter the statement takes no null for#75
colinhacks merged 4 commits into
mainfrom
runtime-null

Conversation

@colinhacks
@colinhacks colinhacks commented Oct 23, 2026
Copy link
Copy Markdown
Owner

A parameter typed non-null bound NULL whenever the caller passed null or left it out, and the rows then broke their declared types: the engines infer a result's nullability on the promise that such a parameter holds a value.

// select :x::int + 1 as y   →   Params { x: number }, Row { y: number }
await query.run(db, { x: null }); // was [{ y: null }]; now throws before the query runs
await query.run(db, {});          // was [{ y: null }] (a missing key bound NULL); now throws

Any untyped input reaches this: a request body passed to .run(), or a JavaScript caller.

What changes

  • The emitted module records, for each parameter, whether the statement takes null for it (inputs[name].nullable).
  • .run() and .runSync() refuse null, and a missing value, for a parameter whose nullable is false, with Parameter "x" is null, and the statement takes no null for it: pass a value. (or is missing), before anything reaches the driver.
  • A typed JSON parameter takes null only where its schema allows null; a default still fills a missing value.
  • Unchanged: a parameter that takes null (a coalesced one, a bare one written to a nullable column) binds NULL, given or left out; one typed unknown (strict_params: false) takes anything; and a query defined by hand without nullable binds what it is given.
  • The Inference page and the skill say that any other parameter takes no null.

@colinhacks
fix: refuse null for a parameter the statement takes no null for
a0793d3 
A parameter typed non-null bound NULL when the caller passed null or left it out, and the rows then broke the types the analysis gave them on that promise. The module now says which parameters take null, and the run time refuses null and a missing value for the others before the statement runs, a typed JSON parameter's included unless its schema allows null.
@colinhacks
docs: say that a non-null parameter refuses null at run time
d831197
Copilot AI balanced review requested due to automatic review settings October 23, 2026 17:47
@vercel
vercel Bot commented Oct 23, 2026 •
edited
Loading
Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
reylo Ready Ready Preview Oct 23, 2026 5:56pm UTC

Request Review

Copilot AI reviewed Oct 23, 2026
Copilot AI left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

pullfrog[bot]
pullfrog Bot reviewed Oct 23, 2026
View reviewed changes
@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

A JSON parameter with no schema is declared unknown in Params, so its type accepts null. With this change the run time throws on that same null, so code that type-checks now fails at run time.

Reviewed changes

I reviewed the emitted nullable flag, the new null refusal in statement(), and the docs and skill lines that describe it.

  • Emitted nullable: sqlModule writes each parameter's nullable from the analysis into inputs, so every build step's module carries it.
  • Run-time refusal: statement() throws a ParameterError for a parameter with nullable: false that gets null or is left out. A typed JSON parameter is refused only when its schema rejects null, and only after its default has filled a missing value. Parameters typed unknown and hand-written queries without the flag are unchanged.
  • Docs and skill: the Inference page and SKILL.md now say that any parameter without coalesce takes no null.
  • Tests: there is a new test in tests/execute.test.ts, and the module snapshots in analyze and casts now include "nullable":false. node --test tests/execute.test.ts passes locally (48/48).

Pullfrog  ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏

Comment thread src/execute.ts Resolved
Comment thread src/execute.ts Resolved
Comment thread site/content/sections/03-inference.mdx Resolved
pullfrog[bot]
pullfrog Bot reviewed Oct 23, 2026
View reviewed changes
@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found. The new commits fix all three points from the last review.

Reviewed changes

I reviewed the two commits pushed since the last review: they bring the run-time null check in line with the declared Params types and correct the docs.

  • Null check follows the declared type: the new refusesNull lets null through for a parameter declared unknown, which covers a parameter typed unknown and a JSON one with no schema. For a JSON parameter whose type has a schema of its own, it asks that schema whether null is allowed, and it refuses null for every other non-null parameter. This matches renderParamType in src/render.ts.
  • checks imported when needed: sqlModule now imports checks for a non-null JSON parameter whose type has a schema, so the run time can check that schema.
  • Structured parameters keep their own message: the null check now runs after the rows branch, so row(…) and rows(…) still say what to pass.
  • Docs and skill: the Inference page and SKILL.md now say that a bare parameter written to a column that may hold null takes null too.
  • Tests: new cases cover a JSON parameter with no schema, a type schema that allows null, one that does not, and the structured-parameter message. node --test tests/execute.test.ts passes locally (48/48).

Pullfrog  ｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏

@colinhacks
colinhacks merged commit 5948fa3 into main Oct 23, 2026
6 checks passed
@colinhacks
colinhacks deleted the runtime-null branch October 23, 2026 18:29
@colinhacks colinhacks mentioned this pull request Oct 23, 2026
Merged
@colinhacks colinhacks mentioned this pull request Nov 1, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@pullfrog pullfrog[bot] pullfrog[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@colinhacks
0