From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Daniel Verite <daniel(at)manitou-mail(dot)org> |
Subject: | Re: ICU for global collation |
Date: | 2022-03-10 10:27:12 |
Message-ID: | 20220310102712.22ba474npqggn57f@jrouhaud |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Mar 10, 2022 at 10:52:41AM +0100, Peter Eisentraut wrote:
> On 05.03.22 09:38, Julien Rouhaud wrote:
> > @@ -168,18 +175,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
> > errmsg("collation \"default\" cannot be copied")));
> > }
> >
> > - if (localeEl)
> > - {
> > - collcollate = defGetString(localeEl);
> > - collctype = defGetString(localeEl);
> > - }
> > [...]
> >
> > I tried to read the function and quickly got confused about whether possible
> > problematic conditions could be reached or not and had protection or not. I
> > think that DefineCollation is becoming more and more unreadable, with a mix of
> > fromEl, !fromEl and either. Maybe it would be a good time to refactor the code
> > a bit and have have something like
> >
> > if (fromEl)
> > {
> > // initialize all specific things
> > }
> > else
> > {
> > // initialize all specific things
> > }
> >
> > // handle defaults and sanity checks
> >
> > What do you think?
>
> How about the attached?
Thanks! That's exactly what I had in mind. I think it's easier to follow and
make sure of what it's doing exactly, so +1 for it.
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2022-03-10 11:02:22 | Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints |
Previous Message | Peter Eisentraut | 2022-03-10 09:52:41 | Re: ICU for global collation |