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

Two bugfixes #240

Merged
merged 2 commits into from May 26, 2015
Merged

Two bugfixes #240

merged 2 commits into from May 26, 2015

Conversation

alexbaretta
Copy link
Contributor

  • Fix identifier case in FOREIGN KEY constraints
  • Fix string delimiter syntax in COMMENT statements

@dimitri
Copy link
Owner

dimitri commented May 26, 2015

Hi, thanks for the fix!

I don' t understand what you are achieving in the 5b3fb6f patch tho, can you expand on that? All I can see is weakening of the poor man's dollar quoting protection I put into place here...

@alexbaretta
Copy link
Contributor Author

Hey Dmitri,

Thanks for taking time to look into this. The rationale for 5b3fb6f is that dashes are not supported in $-delimiter strings.

main=# select $ABC-DEF$Hello, Dimitri!$ABC-DEF$;
ERROR:  syntax error at or near "$"
LINE 1: select $ABC-DEF$Hello, Dimitri!$ABC-DEF$;
               ^

vs.

main=# select $ABCDEF$Hello, Dimitri!$ABCDEF$;
    ?column?    
----------------
 Hello, Dimitri!
(1 row)

@dimitri
Copy link
Owner

dimitri commented May 26, 2015

I am confused, because I (of course) tested that feature. Now it could be that the test case was wrong and didn't use the code path :( Anyways, can you amend your patch here to use "_" rather than "-", please?

@alexbaretta
Copy link
Contributor Author

main=# select $ABC_DEF$Hello, Dmitri!$ABC_DEF$;

?column?

Hello, Dmitri!
(1 row)

I guess I can...

dimitri added a commit that referenced this pull request May 26, 2015
@dimitri dimitri merged commit ceaad92 into dimitri:master May 26, 2015
@dimitri
Copy link
Owner

dimitri commented May 26, 2015

Thanks!

@alexbaretta
Copy link
Contributor Author

My pleasure!

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.

None yet

2 participants