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?
