fix: join with inner sql statement #449
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@mmkal sorry for the reformatting of the file. haven't found out yet where is wrong for my setup. for this issue is just fixing the issue with join + inner:
I've added a test to demo this issue. |
|
btw, recommend https://biomejs.dev/ which is super for lint and format. (I'm not in anything associated with them, just. a fan) |
|
I have a custom eslint plugin I use on all my projects, it has a prettier config preset, and a few typescript-eslint rules and others. You should be able to run I might set up autofix.ci which will push a commit automatically though. |
|
@mmkal I ran |
packages/client/src/sql.ts
Outdated
| @@ -1,19 +1,19 @@ | |||
| import pgPromise from 'pg-promise' | |||
| import {QueryError} from './errors' | |||
| import {nameQuery} from './naming' | |||
| import {StandardSchemaV1} from './standard-schema/contract' | |||
| import type {StandardSchemaV1} from './standard-schema/contract' | |||
There was a problem hiding this comment.
being able to identify import 'type' is probably the biggest reason I switched to biome
| const innerArgs = value.templateArgs() as Parameters<SQLTagFunction> | ||
| const innerResult = sqlFnInner({priorValues: values.length + priorValues}, ...innerArgs) | ||
| segments.push(...innerResult.segments()) | ||
| values.push(...innerResult.values) |
There was a problem hiding this comment.
@mmkal here is the fix
|
ah. I think the formatting break the tests. let me work on it |
caoer
force-pushed
the
fix/join-statement-with-inner
branch
from
597b980 to
681e84c
Compare
|
@mmkal it is ready now. I think the CI are broken? looks it is failing on the pgkit branch |
|
Thanks! And yes I need to fix - can merge this once CI is green, will try to do today. |
packages/client/test/bugs.test.ts
Outdated
| test('join param', async () => { | ||
| const parts = [] | ||
| parts.push(sql`name = ${'user name'}`, sql`location = ${'london'}`) | ||
|
|
||
| const query = sql` | ||
| update users | ||
| set ${sql.join(parts, sql`, `)} | ||
| where id = ${1} | ||
| ` | ||
|
|
||
| expect(query).toMatchInlineSnapshot(` | ||
| { | ||
| "name": "update_d57bde2", | ||
| "parse": [Function], | ||
| "segments": [Function], | ||
| "sql": " | ||
| update users | ||
| set name = $1, location = $2 | ||
| where id = $3 | ||
| ", | ||
| "templateArgs": [Function], | ||
| "then": [Function], | ||
| "token": "sql", | ||
| "values": [ | ||
| "user name", | ||
| "london", | ||
| 1, | ||
| ], | ||
| } | ||
| `) | ||
| }) |
There was a problem hiding this comment.
this looks good - but i'd prefer a runnable query to make sure it really works. something like:
| test('join param', async () => { | |
| const parts = [] | |
| parts.push(sql`name = ${'user name'}`, sql`location = ${'london'}`) | |
| const query = sql` | |
| update users | |
| set ${sql.join(parts, sql`, `)} | |
| where id = ${1} | |
| ` | |
| expect(query).toMatchInlineSnapshot(` | |
| { | |
| "name": "update_d57bde2", | |
| "parse": [Function], | |
| "segments": [Function], | |
| "sql": " | |
| update users | |
| set name = $1, location = $2 | |
| where id = $3 | |
| ", | |
| "templateArgs": [Function], | |
| "then": [Function], | |
| "token": "sql", | |
| "values": [ | |
| "user name", | |
| "london", | |
| 1, | |
| ], | |
| } | |
| `) | |
| }) | |
| test('join param', async () => { | |
| const parts = [ | |
| sql`id = ${11}`, | |
| sql`name = ${'eleven'}`, | |
| ] | |
| const query = sql` | |
| update edge_cases_test | |
| set ${sql.join(parts, sql`, `)} | |
| where id = ${1} | |
| ` | |
| expect(query).toMatchInlineSnapshot() | |
| await client.query(query) | |
| const result = await client.one(sql`select * from edge_cases_test where id = ${11}`) | |
| expect(result).toEqual({id: 11, name: 'eleven'}) | |
| }) |
|
@mmkal updated |
|
@caoer thank you - good find! i might add just one more test and will publish asap |
|
Release v0.5.0 addresses this. |
|
Release v0.5.0 addresses this. |
No description provided.