Re: WIN32 pg_import_system_collations

Поиск
Список
Период
Сортировка
От Juan José Santamaría Flecha
Тема Re: WIN32 pg_import_system_collations
Дата
Msg-id CAC+AXB078_wJumwG9nOP--m3cTWKhAUeC22S73rUMVhXknstUg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: WIN32 pg_import_system_collations  (Peter Eisentraut <peter.eisentraut@enterprisedb.com>)
Ответы Re: WIN32 pg_import_system_collations
Список pgsql-hackers

On Mon, Oct 31, 2022 at 3:09 PM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

Thanks for taking a look into this patch.

Consider this function you are introducing:

+/*
+ * Create a collation if the input locale is valid for so.
+ * Also keeps track of the number of valid locales and collations created.
+ */
+static int
+CollationFromLocale(char *isolocale, char *localebuf, int *nvalid,
+                                       int *ncreated, int nspid)

This declaration is incomprehensible without studying all the callers
and the surrounding code.

Start with the name: What does "collation from locale" mean?  Does it
make a collation?  Does it convert one?  Does it find one?  There should
be a verb in there.

(I think in the context of this file, a lower case name would be more
appropriate for a static function.)

Then the arguments.  The input arguments should be "const".  All the
arguments should be documented.  What is "isolocale", what is
"localebuf", how are they different?  What is being counted by "valid"
(collatons?, locales?), and what makes a thing valid and invalid?  What
is being "created"?  What is nspid?  What is the return value?

Please make another pass over this.

Ok, I can definitely improve the comments for that function.
 
Also consider describing in the commit message what you are doing in
more detail, including some of the things that have been discussed in
this thread.

Going through the thread for the commit message, I think that maybe the collation naming remarks were not properly addressed. In the current version the collations retain their native name, but an alias is created for those with a shape that we can assume a POSIX equivalent exists.

Please find attached a new version.

Regards,

Juan José Santamaría Flecha
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Jeff Davis
Дата:
Сообщение: Re: Refactor to introduce pg_strcoll().
Следующее
От: Jan Wieck
Дата:
Сообщение: Re: PL/pgSQL cursors should get generated portal names by default