Skip to content

Add quotes to escaped parameters #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add quotes to escaped parameters #58

wants to merge 1 commit into from

Conversation

naturalethic
Copy link

Fixes #57

@porsager
Copy link
Owner

porsager commented Apr 18, 2020

This breaks other things, and there are also failing tests.

@porsager
Copy link
Owner

So the fix needs to be done in the escape serializer and there add quotes if there are any uppercase characters. I actually thought that's how I had done it, but I see I'm missing a test case for it, so gotta look closer if it's a regression or if I simply remember wrong.

@porsager
Copy link
Owner

porsager commented Apr 18, 2020

A fix would be to add || str !== str.toLowerCase() at the end of line 70 in the escape function

Also add a test like

t('dynamic insert quoting', async() => {
  await sql`create table test ("lowAndUpper" int)`
  const x = { lowAndUpper: 42 }

  return [42, (await sql`insert into test ${ sql(x) } returning *`)[0].lowAndUpper, await sql`drop table test`]
})

Then you can do npm run test afterwards to make sure everything's good ;)

@naturalethic
Copy link
Author

Can you give me a copy of your postgres configs (redacted where necessary) to aid me in setting up the proper ssl environment for your tests, plus any other relevant info I might need?

@porsager
Copy link
Owner

porsager commented Apr 20, 2020

Oh I'm sorry, I completely forgot there were environment specific things missing to run the tests 🤦‍♂️

I'll get it added to the README.md while I'm at it.

I'd want to set up github actions to run at some point too..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quote all columns in generated inserts, updates
2 participants