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

Spaces in string_array #1

Closed
kit-ty-kate opened this issue Jan 14, 2014 · 6 comments
Closed

Spaces in string_array #1

kit-ty-kate opened this issue Jan 14, 2014 · 6 comments

Comments

@kit-ty-kate
Copy link
Contributor

In our application we use string_array a bit and we noticed that if there is spaces in a element of an array of string, then the resulting string is "test test" not just test test (the quotes are superfluous) as postgres sends quotes in this case.

I can't test it now but I think that if there is a comma in the string then it will be interpreted as two elements.

@kit-ty-kate
Copy link
Contributor Author

I'm trying to provide a patch. I noticed also a potential problem with "NULL" passed as string.

@darioteixeira
Copy link
Owner

@jpdeplaix: Issue acknowledged. The problem is that any_array_of_string expects strings to be properly escaped, but string_of_string_array was not escaping them. Also, I'm not sure your patch fixes all potential problems (I think a cleaner solution would be to properly escape strings in string_of_string_array). Anyway, I'll take a better look at this tomorrow.

@darioteixeira
Copy link
Owner

@jpdeplaix: Okay, after a closer look, it seems that a) your patch should suffice for all situations which do not involve NULL values, and b) there's no getting around the fact that array values may be NULL. Because of the latter, correctness dictates that a PostgreSQL 'a array type should be mapped to 'a option array on the OCaml side. This change will break backwards-compatibility, which is why I'm gonna raise the issue on the PGOCaml-list.

Anyway, thanks for the patch!

@darioteixeira
Copy link
Owner

@jpdeplaix: I've committed to master a new patch which should fix all issues with arrays, including escaping strings and handling NULL. However, it also requires that array types be now 'a option array, which breaks backward compatibility. Could you test it, please?

@kit-ty-kate
Copy link
Contributor Author

It seems to work perfectly. I've fixed the representation of the arrays in both my fork of macaque for Cumulus and the pull-request about arrays (ocsigen/macaque#5) as well as in Cumulus.
If you want to test, the beta-instance of Cumulus is available here: http://nagi.mirai.fr/ (we use arrays to get the tags).

Thanks a lot

@kit-ty-kate
Copy link
Contributor Author

I think I can close it now. Thanks !

jrochel pushed a commit to jrochel/pgocaml that referenced this issue May 3, 2018
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.

2 participants