Infer IS (NOT) NULL as `boolean` instead of `boolean

karlhorky commented 2 months ago

Avoid the | null here, from my comment:

// 💥 Query has incorrect type annotation.
//  Expected: { ... is_mammal: boolean; }
//  Actual: { ... is_mammal: boolean | null; }[]
await sql<{ id: number, first_name: string, is_mammal: boolean }[]>`
  SELECT
    animals.id,
    animals.first_name,
    mammals.id IS NOT NULL AS is_mammal
  FROM
    animals
    LEFT JOIN mammals ON animals.id = mammals.animal_id
`;

The tests I added are passing, but I'm not sure if I am missing out on other cases which should NOT be non-nullable.

By the way, is there something like AST explorer for the SQL AST types? I am fumbling a bit in the dark here, searching through the codebase - would be great to have a visual UI to select the parts of the code I'm interested in and have the node types identified.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 208ce1e1d2b81c5bd1e1f3cae1c2425ec6b68c95

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages

| Name | Type | | ---------------------------------------- | ----- | | @ts-safeql/generate | Patch | | @ts-safeql/eslint-plugin | Patch | | @ts-safeql/shared | Patch | | @ts-safeql/sql-ast | Patch | | @ts-safeql-demos/basic-flat-config | Patch | | @ts-safeql-demos/basic-migrations-raw | Patch | | @ts-safeql-demos/basic-transform-type | Patch | | @ts-safeql-demos/basic | Patch | | @ts-safeql-demos/big-project | Patch | | @ts-safeql-demos/config-file-flat-config | Patch | | @ts-safeql-demos/from-config-file | Patch | | @ts-safeql-demos/multi-connections | Patch | | @ts-safeql-demos/playground | Patch | | @ts-safeql-demos/postgresjs-custom-types | Patch | | @ts-safeql-demos/postgresjs-demo | Patch | | @ts-safeql-demos/vercel-postgres | Patch | | @ts-safeql/test-utils | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

karlhorky commented 2 months ago

One thing this PR doesn't touch is IS NOT NULL and IS NULL inside of a subquery, which yields unknown:

// 💥 Query has incorrect type annotation.
//  Expected: { is_null: boolean }[]
//    Actual: { is_null: unknown }[]
await sql<{ is_null: boolean }>`
  SELECT (SELECT 1 IS NULL) AS is_null
`

Newbie012 commented 2 months ago

Well done! LGTM. Regaring a AST Explorer - sadly there isn't. I usually rely heavily on logs and breakpoints.

karlhorky commented 2 months ago

Nice, thanks for the review and merge!

Regaring a AST Explorer - sadly there isn't. I usually rely heavily on logs and breakpoints.

Interesting, yeah then I'll work like this to start too.

Wonder how easy it is to contribute something like this to AST explorer... 🤔 Would be an interesting thing to have the SQL AST more observable / visually debuggable. Or alternatively, build some minimal custom tooling. I'll keep it in mind.

Newbie012 commented 2 months ago

@karlhorky https://github.com/Newbie012/pg-query-ast-explorer

karlhorky commented 2 months ago

Omg amazing, that looks great!! I will definitely use this for the next PRs!

https://pg-query-ast-explorer.vercel.app/

Screenshot 2024-09-20 at 15 04 31

ts-safeql / safeql

Infer IS (NOT) NULL as `boolean` instead of `boolean | null` #265

🦋 Changeset detected