Add CREATE DATABASE LOCALE option

On Wed, Jun 5, 2019 at 5:17 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

Cool... would be nice also add some test cases.

Regards,

--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On 2019-06-05 22:31, Fabrízio de Royes Mello wrote:

On Wed, Jun 5, 2019 at 5:17 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE.  This
sets both LC_COLLATE and LC_CTYPE with one option.  Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Jun 6, 2019 at 6:38 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-06-05 22:31, Fabrízio de Royes Mello wrote:

On Wed, Jun 5, 2019 at 5:17 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate

and

lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

Hmm... good question... I thought we already have some regression tests for
{CREATE|DROP} DATABASE but actually we don't... should we add a new one?

Att,

--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On 2019-Jun-06, Fabr�zio de Royes Mello wrote:

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

Hmm... good question... I thought we already have some regression tests for
{CREATE|DROP} DATABASE but actually we don't... should we add a new one?

I think pg_dump/t/002_pg_dump.pl might be a good place. Not the easiest
program in the world to work with, admittedly.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 05/06/2019 23:17, Peter Eisentraut wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

One objection is that the proposed LOCALE option would only affect
LC_COLLATE and LC_CTYPE. What about lc_messages, lc_monetary, lc_numeric
and lc_time? initdb's --locale option sets those, too. Should CREATE
DATABASE LOCALE set those as well?

On the whole, +1 on adding the option. In practice, you always want to
set LC_COLLATE and LC_CTYPE to the same value, so we should make that
easy. But let's consider those other variables too, at least we've got
to document it carefully.

PS. There was some discussion on doing this when the LC_COLLATE and
LC_CTYPE options were added:
/messages/by-id/491862F7.1060501@enterprisedb.com.
My reading of that is that there was no strong consensus, so we just let
it be.

- Heikki

On 2019-06-06 21:52, Alvaro Herrera wrote:

On 2019-Jun-06, Fabrízio de Royes Mello wrote:

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

Hmm... good question... I thought we already have some regression tests for
{CREATE|DROP} DATABASE but actually we don't... should we add a new one?

I think pg_dump/t/002_pg_dump.pl might be a good place. Not the easiest
program in the world to work with, admittedly.

Updated patch with test and expanded documentation.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hello Peter,

I think pg_dump/t/002_pg_dump.pl might be a good place. Not the easiest
program in the world to work with, admittedly.

Updated patch with test and expanded documentation.

Patch v2 applies cleanly, compiles, make check-world ok with tap enabled.
Doc gen ok.

The addition looks reasonable.

The second error message about conflicting option could more explicit than
a terse "conflicting or redundant options"? The user may expect later
options to superseedes earlier options, for instance.

About the pg_dump code, I'm wondering whether it is worth generating
LOCALE as it breaks backward compatibility (eg dumping a new db to load it
into a older version).

If it is to be generated, I'd do merge the two conditions instead of
nesting.

if (strlen(collate) > 0 && strcmp(collate, ctype) == 0)
// generate LOCALE

--
Fabien.

On 2019-07-13 19:20, Fabien COELHO wrote:

The second error message about conflicting option could more explicit than
a terse "conflicting or redundant options"? The user may expect later
options to superseedes earlier options, for instance.

done

About the pg_dump code, I'm wondering whether it is worth generating
LOCALE as it breaks backward compatibility (eg dumping a new db to load it
into a older version).

We don't really care about backward compatibility here. Moving forward,
with ICU and such, we don't want to have to drag around old syntax forever.

If it is to be generated, I'd do merge the two conditions instead of
nesting.

if (strlen(collate) > 0 && strcmp(collate, ctype) == 0)
// generate LOCALE

done

How about this patch?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hello Peter,

About the pg_dump code, I'm wondering whether it is worth generating
LOCALE as it breaks backward compatibility (eg dumping a new db to load it
into a older version).

We don't really care about backward compatibility here. Moving forward,
with ICU and such, we don't want to have to drag around old syntax forever.

We will drag it anyway because LOCALE is just a shortcut for the other two
LC_* when they have the same value.

How about this patch?

It applies cleanly, compiles, global & pg_dump make check ok, doc gen ok.

I'm still unconvinced of the interest of breaking backward compatibility,
but this is no big deal.

I do not like much calling strlen() to check whether a string is empty,
but this is pre-existing.

I switched the patch to READY.

--
Fabien.

On 2019-07-23 00:18, Fabien COELHO wrote:

It applies cleanly, compiles, global & pg_dump make check ok, doc gen ok.

I'm still unconvinced of the interest of breaking backward compatibility,
but this is no big deal.

I do not like much calling strlen() to check whether a string is empty,
but this is pre-existing.

I switched the patch to READY.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services