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

standards_conforming_strings=off confuses duckdb parser #119

Closed
hlinnaka opened this issue Aug 14, 2024 · 2 comments · Fixed by #135
Closed

standards_conforming_strings=off confuses duckdb parser #119

hlinnaka opened this issue Aug 14, 2024 · 2 comments · Fixed by #135

Comments

@hlinnaka
Copy link
Collaborator

postgres=# set standard_conforming_strings =off;
SET
postgres=# select * from foo where t = 'foo\'bar';
WARNING:  nonstandard use of \' in a string literal
LINE 1: select * from foo where t = 'foo\'bar';
                                    ^
HINT:  Use '' to write quotes in strings, or use the escape string syntax (E'...').
WARNING:  (DuckDB) Parser Error: syntax error at or near "bar"
LINE 1: select * from foo where t = 'foo\'bar';
                                          ^
ERROR:  unrecognized node type: 1
@Reminiscent
Copy link
Contributor

Reminiscent commented Aug 17, 2024

@wuputah @hlinnaka What do you think the best way to fix this issue?

  1. support standards_conforming_strings in duckdb for compatible. -> I see tThe purpose of this guc is to be compatible with the behavior of old versions of postgres. Maybe it's not good idea to do this.
  2. handle this logic in the pg_duckdb. Do some pre-process in the pg_duckdb.
  3. forbid to use the duckdb when we find we use this GUC.

A general question is what is the general approach for some GUCs that may change behavior or be incompatible with duckdb, or is it handled on a case-by-case?

@JelteF
Copy link
Collaborator

JelteF commented Aug 19, 2024

This is now fixed by #130, I now added a test for this in #135

JelteF added a commit that referenced this issue Aug 20, 2024
The issue in #119 was already fixed by #130, but this PR adds a test for
it.

Fixes #119
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 a pull request may close this issue.

3 participants