zorro-del-caribe/ship-hold-querybuilder

Incorrect wrapping of strings with parentheses

dude333 opened this issue · 5 comments

The following line prevents a string with parentheses to be wrapped correctly:

const isWrappedRegexp = /'(.*)'|"(.*)"|\((.*)\)|^\$/;

For example:

wrap("string with (parentheses)") results in:

'string with (parentheses)' 

instead of:

'"string with (parentheses)"'

I don't know what you have considered to skip this case (avoiding to wrap a function maybe), but my case is that I'm importing data from a file with all sorts of texts, including texts in parentheses.

To fix that I've changed it to:

const isWrappedRegexp = /'(.*)'|"(.*)"|^\$/;

Thanks for the great job on this tool, it's very helpful.

Another change I had to make was on this line:

from:

return wrap(value, `'`);

to:

const x = wrap(value, `$$`);

Using the dollar-quoted string avoids having to escape the single quote '.

Thanks for your report.

Can you please provide a way to reproduce the bugs:

  1. give how you call the builder
  2. What you would expect the generated SQL to be

This would help to create regression tests.

Thanks a lot

Hi @lorenzofox3, I'm reading data from an Excel file, parsing all rows and for each of them I run a code similar to this one (consider the row object below as a row read from that file):

const { shiphold } = require("ship-hold");

const sh = shiphold();

const row = {
  a: "text",
  b: 123,
  c: "with (parentheses)",
  d: "John's",
  e: "single 'quotation mark'",
  f: 'double "quotation mark"',
};

(async function (obj) {
  const { text } = await sh.insert(obj).into("table").build();
  console.log(text);
})(row);

That results into this:

INSERT INTO "table" ( "a", "b", "c", "d", "e", "f" ) VALUES ( 'text', 123, with (parentheses), 'John's', single 'quotation mark', double "quotation mark" )

Note the missing single quotes around some values and the possessive apostrophe is note escaped.

When I changed isWrappedRegexp to /'(.*)'|"(.*)"|^\$/;, it fixed the parentheses case:

INSERT INTO "table" ( "a", "b", "c", "d", "e", "f" ) VALUES ( 'text', 123, 'with (parentheses)', 'John's', single 'quotation mark', double "quotation mark" )

Changing from return wrap(value, `'`); to return wrap(value, `$$`); fixed the possessive apostrophe:

INSERT INTO "table" ( "a", "b", "c", "d", "e", "f" ) VALUES ( $$text$$, 123, $$with (parentheses)$$, $$John's$$, single 'quotation mark', double "quotation mark" )

Then this last case I've discovered during this demonstration, when you have quotation marks inside the string: I changed the isWrappedRegexp to const isWrappedRegexp = /^'(.*)'$|^"(.*)"$|^\$/;, generating the expected SQL:

INSERT INTO "table" ( "a", "b", "c", "d", "e", "f" ) VALUES ( $$text$$, 123, $$with (parentheses)$$, $$John's$$, $$single 'quotation mark'$$, $$double "quotation mark"$$ )

I see thanks !

In that case, I would rather recommend using parametrized query:

sh.insert({a:'$a', b:'$b'}).build({a:`it doesn't matter whether there are quote`, b:`or (parenthesis)`});

The idea of using dollar quotes seems very interesting. I am just a bit concerned it would break current behaviour. Have you tried building and running the tests ?
Second point: If I am not wrong dollar quotes is specific to Postgres. In theory it is not a problem, but if I am ok to have dialect specific to Postgres, I am a bit reluctant to introduce non standard SQL as default behaviour.

I will try your suggestion, but I've found another issue:

sh.select({ value: "to_char(SUM(charges_usd), '999G999G999G990D00')", as: "total (usd)" }).from("mytable")

The total (usd) rises and error when executed as it'll render to this:

SELECT to_char(SUM(charges_usd), '999G999G999G990D00') AS total (usd) FROM "mytable"

The total (usd) is not quoted if I executed without my proposed changes.

The dollar-quoted strings is very handy as it avoids the character escaping nightmare, but unfortunately it's not part of the SQL standard and would be a block for other sql dialects.

Edit: I have ran the tests (npm run test) after my changes and it passed with no errors.