8000 feat(postgres): infer greatest and least non-null by their arguments by colinhacks · Pull Request #47 · colinhacks/reylo · GitHub
[go: up one dir, main page]
Skip to content
8000

feat(postgres): infer greatest and least non-null by their arguments - #47

Merged
colinhacks merged 2 commits into
mainfrom
greatest-least
Oct 23, 2026
Merged

feat(postgres): infer greatest and least non-null by their arguments#47
colinhacks merged 2 commits into
mainfrom
greatest-least

Conversation

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

Postgres parses greatest(…) and least(…) as their own syntax (MinMaxExpr), never as a function call, so the rule that types them by their arguments never fired, and both were always typed nullable. Each returns NULL only when every argument is, so either is now non-null when one argument is.

select greatest(id, null) as a,        -- int4, was int4 | null
  greatest(manager_id, null) as c      -- still int4 | null
from member
  • Postgres only: MySQL's GREATEST and LEAST return NULL when any argument is, and the same reader parses MySQL and SQLite text, so they are left as they were. A unit test pins both.
  • Both are syntax, not functions, so a user's function of the same name can't shadow them and no trust check applies.
  • Their arguments are also scanned for aggregates, so (select greatest(count(*), 1) from member) reads as an aggregate and stays one row.
  • The oracle case precision_rules gains greatest_least.sql, and every other expected line is unchanged.
  • The Postgres page's nullability list names greatest and least beside coalesce.

Tests: the engine's unit tests, the oracle with --strict-coverage, the wasm fingerprint, the typecheck, the package build and the docs site. The loader tests fail one SQLite test that fails on main too on Node 26 (tests/representation.test.ts, "SQLite: JSON a statement builds…", node:sqlite with readBigInts false); this branch touches nothing on that path.

8000
Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:42
@vercel
vercel Bot commented Oct 22, 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 3:11am UTC

Request Review

Copilot AI reviewed Oct 22, 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 reached their quota limit.

pullfrog[bot]
pullfrog Bot reviewed Oct 22, 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 issues found — one optional cleanup inline.

Reviewed changes

I reviewed the one commit that makes Postgres type greatest/least by their arguments, plus the oracle case, the docs line and the rebuilt wasm.

  • MinMaxExpr read as Expr::Any: on Postgres alone, greatest/least are non-null when any argument is, matching Postgres's "NULL only if all arguments are NULL". The Dialect::Postgres guard keeps MySQL's NULL-if-any reading.
  • Aggregate scan descends into MinMaxExpr: (select greatest(count(*), 1) from t) now reads as one row. A set-returning call nested in the arguments is still caught, because one_value walks the whole node tree generically.
  • Oracle, docs, wasm: the new greatest_least.sql case covers non-null, nullable, text and aggregate-subquery arguments. tests/fingerprint.test.ts passes for the committed packages/postgres/engine.wasm at 7679b61.

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

Comment thread engine/postgres/src/infer.rs Resolved
@colinhacks
colinhacks force-pushed the greatest-least branch 3 times, most recently from 3c28254 to b8f3c6a Compare October 23, 2026 00:06
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.

Reviewed changes

I reviewed b8f3c6a, which removes the old function-call path for greatest/least now that MinMaxExpr handles them.

  • Removed POSTGRES_ANY: its FuncCall arm and its entry in postgres_function_names() are gone. With no catalog, a quoted "greatest"(…) used to be read as the built-in. Now it falls through as an unknown function and is typed nullable, and one_value treats it conservatively. The trust query also stops asking about names that have no pg_proc entry.
  • Added a quoted-name assertion: greatest_and_least_need_one_non_null_argument_on_postgres_alone now pins "greatest"(NULL, 0) as nullable.
  • Rebuilt engine.wasm: tests/fingerprint.test.ts passes for all three committed artifacts at b8f3c6a.

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

@colinhacks
colinhacks merged commit 4fd61cd into main Oct 23, 2026
6 checks passed
@colinhacks
colinhacks deleted the greatest-least branch October 23, 2026 03:18
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