Incorrect wrapping of strings with parentheses
dude333 opened this issue · 5 comments
The following line prevents a string with parentheses to be wrapped correctly:
ship-hold-querybuilder/src/lib/nodes.ts
Line 25 in 6b49680
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:
ship-hold-querybuilder/src/lib/nodes.ts
Line 31 in 6b49680
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:
- give how you call the builder
- 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.