slonik-sql-tag-raw icon indicating copy to clipboard operation
slonik-sql-tag-raw copied to clipboard

Named parameter validation throwing for quoted strings

Open mmkal opened this issue 5 years ago • 7 comments

Repro:

let {createPool, sql} = require('slonik')
const {raw} = require('slonik-sql-tag-raw')

let slonik = createPool('postgresql://postgres:postgres@localhost:5432/postgres')

const queryFromFileSystem = `
  select $$
    this is just a string, it should be able to have any value
    but this causes an error --> :foo:
  $$
`

slonik.query(sql`${raw(queryFromFileSystem)}`)

Running this throws an error: InvalidInputError: Named parameter reference does not have a matching value.

Edit: it also reproduces for single-quoted strings:

select '
  this is just a string, it should be able to have any value
  but this causes an error --> :foo:
'

mmkal avatar Oct 14 '20 08:10 mmkal

This is a feature https://github.com/gajus/slonik-sql-tag-raw#named-parameters

What's the issue?

gajus avatar Oct 14 '20 15:10 gajus

Oh I see. I am not even sure what's the solution to this without parsing the SQL.

gajus avatar Oct 14 '20 15:10 gajus

I suppose the problem is that the SQL is being parsed, but using a regex rather than a full-blown parser. So the options for fixing this are: parse the SQL properly, or not at all.

I assumed there was no parsing already. Obviously, it's somewhat dangerous, but that's to be expected from a function called raw. That's the workaround I used, by skipping this library and the sql template tag entirely:

slonik.query({
  token: 'SLONIK_TOKEN_SQL',
  sql: `
    create function foo() returns trigger as
    $$
      ${fs.readFileSync(...).toString()}
    $$
    language "plv8";
  `,
  values: [],
})

But there may be a better/safer way? In my case, I'm actually using $someGeneratedRandomString$ rather than $$ to make sure something in the file doesn't end the quotes prematurely.

mmkal avatar Oct 14 '20 15:10 mmkal

We can evaluate @taozhi8833998's https://github.com/taozhi8833998/node-sql-parser

gajus avatar Oct 14 '20 16:10 gajus

It looks nice! But GPL-licensed, so this project and anything using it would likely need to switch to a similar license.

mmkal avatar Oct 14 '20 20:10 mmkal

Update: this is new and looks like it could be a good alternative: https://github.com/oguimbal/pgsql-ast-parser

It has no license at all!

mmkal avatar Nov 17 '20 03:11 mmkal

@oguimbal would you be available to contribute to this package?

gajus avatar Nov 17 '20 04:11 gajus