-
Notifications
You must be signed in to change notification settings - Fork 217
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
Add support for locale and encoding, fix #406 #416
base: master
Are you sure you want to change the base?
Conversation
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Tagged as draft, since the tests are missing. |
5ce635f
to
c9e6cbe
Compare
c9e6cbe
to
672501f
Compare
Thanks for this contribution. It makes sense to me to have such options. |
[test] |
What I'm thinking about is whether this shouldn't be set earlier, for the whole DB dir, as https://www.postgresql.org/docs/13/multibyte.html says: "The default character set is selected while initializing your PostgreSQL database cluster using initdb". |
@fila43 what do you think? |
672501f
to
f48a57c
Compare
oups, seems I pushed the wrong branch and removed my own code... |
f48a57c
to
672501f
Compare
I am not sure exactly what to do with that error message |
541e601
to
a2cf268
Compare
So using LANG, one can influence the default encoding and collate. However, this doesn't work that well. I tried to create a database with LANG=C.UTF8, and it set the server encoding correctly, but not LC_COLLATE. So I guess we need initdb + the createdb options. |
a2cf268
to
aedee14
Compare
And also, postgresql crash if I set LC_COLLATE to UTF8, so I suspect that using LANG and similar hacks is out of question, as this seems quite fragile and obscure. |
aedee14
to
2977d3e
Compare
I read this as the container's process ended before the exec was done, which might be what you refer to with "postgresql crash if I set LC_COLLATE". |
So I pushed a fix, but as the CI is manually triggered, if someone could trigger for me, it would help (as I have no idea how to do it locally) |
Thanks. |
87a666b
to
690c02c
Compare
So I fixed my tests (and my code), now it fail on:
It also fail on the same error for the master branch, so I guess that's not my code causing that. |
The issue reported in my last comment is tracked on #421 |
[test-all] |
@mscherer Can you please rebased it against master and also run these two commands:
And commit all changes? |
690c02c
to
42fd2c8
Compare
Done |
b64385d
to
bb68c34
Compare
bb68c34
to
a69654e
Compare
a69654e
to
92b08cb
Compare
[test-all] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I went through your code and did not hit any issue. Let's wait on the tests. Thanks for adding also test for it. Tests are more then welcome.
92b08cb
to
6cd0973
Compare
I rebased (before rediffing the other PR) |
6cd0973
to
766be90
Compare
Any progress on this would be highly appreciated. |
Signed-off-by: Michael Scherer <[email protected]>
mhh seems I got it wrong, I erased the branch instead of updating it :( |
766be90
to
785ab38
Compare
Pull Request validationFailed🔴 Review - Missing review from a member (2 required) Success🟢 CI - All checks have passed |
No description provided.