Skip to content
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

Handling escaped single quotes within comments. #63

Closed
wants to merge 6 commits into from
Closed

Handling escaped single quotes within comments. #63

wants to merge 6 commits into from

Conversation

grzegorzk
Copy link

If comments were containing escaped single quote then script generated by pgquarrel was broken. Simple fix to use dollar-quotes fixes the problem.

@eulerto
Copy link
Owner

eulerto commented Jun 17, 2019

Good catch! [Looking into your PR...] I'm not against dollar-quoting but I prefer the traditional way. pgquarrel tries to mimic pg_dump output as much as possible. Since comment is used only to print a SQL command, why don't use PQescapeLiteral or quote_literal in getXXX functions?

@eulerto eulerto added the bug label Jun 18, 2019
@grzegorzk
Copy link
Author

grzegorzk commented Jun 18, 2019

I thought about going through whole code base and wherever it would make sense replacing '%s' with $QUOT$%s$QUOT$ but thought it begs for different solution (changes I made fix my immediate need but if I have some time I'll look into getXXX).

@grzegorzk
Copy link
Author

If you are interested I also got your tests running within docker (to make it easier to test without having to install all of the dependencies locally)

@grzegorzk
Copy link
Author

grzegorzk commented Jun 18, 2019

quote_literal was introduced in version 9.4 so not an option since you support 9.0+. Same with PQescapeLiteral.

@grzegorzk
Copy link
Author

grzegorzk commented Jun 18, 2019

I ended up adding new method to your common.c, you might not like the fact the string replacement was copied from stack overflow - it was almost decade since I have done C last time.

@grzegorzk
Copy link
Author

I added escaping for enum labels too.

eulerto pushed a commit that referenced this pull request Jul 21, 2019
This bug was reported in PR #63. Thanks @grzegorzk
@eulerto
Copy link
Owner

eulerto commented Nov 18, 2019

Commit 725406a implements it as I suggested in #63 (comment)

@eulerto eulerto closed this Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants