Re: Inconsistent "ICU Locale" output on older server versions

2022-04-15 Thread Tom Lane
"Euler Taveira"  writes:
> On Fri, Apr 15, 2022, at 11:58 AM, Christoph Berg wrote:
>> When running psql 15 against PG 14, the output is this:
>> The "ICU Locale" column is now populated, that seems wrong.

> Good catch!

Indeed.

> Since dataiculocale allows NULL, my suggestion is to use NULL instead of an
> empty string. It is consistent with a cluster whose locale provider is libc.

Yeah, I agree.  We should make the pre-v15 output match what you'd see
if looking at a non-ICU v15 database.

regards, tom lane




Re: Inconsistent "ICU Locale" output on older server versions

2022-04-15 Thread Euler Taveira
On Fri, Apr 15, 2022, at 11:58 AM, Christoph Berg wrote:
> When running psql 15 against PG 14, the output is this:
> 
> $ psql -l
> List of databases
>Name│  Owner   │ Encoding │  Collate   │   Ctype│ ICU Locale │ 
> Locale Provider │   Access privileges
> ───┼──┼──┼┼┼┼─┼───
> postgres  │ postgres │ UTF8 │ de_DE.utf8 │ de_DE.utf8 │ de_DE.utf8 │ libc 
>│
> template0 │ postgres │ UTF8 │ de_DE.utf8 │ de_DE.utf8 │ de_DE.utf8 │ libc 
>│ =c/postgres  ↵
>│  │  ││││ 
> │ postgres=CTc/postgres
> template1 │ postgres │ UTF8 │ de_DE.utf8 │ de_DE.utf8 │ de_DE.utf8 │ libc 
>│ =c/postgres  ↵
>│  │  ││││ 
> │ postgres=CTc/postgres
> (3 rows)
> 
> The "ICU Locale" column is now populated, that seems wrong.
Good catch!

> The problem is in the else branch in src/bin/psql/describe.c around
> line 900:
> 
> +   if (pset.sversion >= 15)
> +   appendPQExpBuffer(,
> + "   d.daticulocale as \"%s\",\n"
> + "   CASE d.datlocprovider WHEN 'c' THEN 'libc' 
> WHEN 'i' THEN 'icu' END AS \"%s\",\
> + gettext_noop("ICU Locale"),
> + gettext_noop("Locale Provider"));
> +   else
> +   appendPQExpBuffer(,
> + "   d.datcollate as \"%s\",\n"  <--- there
> + "   'libc' AS \"%s\",\n",
> + gettext_noop("ICU Locale"),
> + gettext_noop("Locale Provider"));
> 
> I'd think this should rather be
> 
> + "   '' as \"%s\",\n"
Since dataiculocale allows NULL, my suggestion is to use NULL instead of an
empty string. It is consistent with a cluster whose locale provider is libc.


--
Euler Taveira
EDB   https://www.enterprisedb.com/


Inconsistent "ICU Locale" output on older server versions

2022-04-15 Thread Christoph Berg
Re: To Peter Eisentraut
> This hardly fits in normal-size terminals:
> 
> =# \l
>  List of databases
>Name│ Owner │ Encoding │  Collate   │   Ctype│ ICU Locale │ Locale 
> Provider │ Access privileges
> ───┼───┼──┼┼┼┼─┼───
>  postgres  │ myon  │ UTF8 │ de_DE.utf8 │ de_DE.utf8 ││ libc   
>  │
>  template0 │ myon  │ UTF8 │ de_DE.utf8 │ de_DE.utf8 ││ libc   
>  │ =c/myon  ↵
>│   │  ││││
>  │ myon=CTc/myon
>  template1 │ myon  │ UTF8 │ de_DE.utf8 │ de_DE.utf8 ││ libc   
>  │ =c/myon  ↵
>│   │  ││││
>  │ myon=CTc/myon
> (3 rows)

Another gripe here: The above is the output when run against a PG15
cluster, created without an ICU locale set.

When running psql 15 against PG 14, the output is this:

$ psql -l
List of databases
   Name│  Owner   │ Encoding │  Collate   │   Ctype│ ICU Locale │ 
Locale Provider │   Access privileges
───┼──┼──┼┼┼┼─┼───
 postgres  │ postgres │ UTF8 │ de_DE.utf8 │ de_DE.utf8 │ de_DE.utf8 │ libc  
  │
 template0 │ postgres │ UTF8 │ de_DE.utf8 │ de_DE.utf8 │ de_DE.utf8 │ libc  
  │ =c/postgres  ↵
   │  │  ││││   
  │ postgres=CTc/postgres
 template1 │ postgres │ UTF8 │ de_DE.utf8 │ de_DE.utf8 │ de_DE.utf8 │ libc  
  │ =c/postgres  ↵
   │  │  ││││   
  │ postgres=CTc/postgres
(3 rows)

The "ICU Locale" column is now populated, that seems wrong.

The problem is in the else branch in src/bin/psql/describe.c around
line 900:

+   if (pset.sversion >= 15)
+   appendPQExpBuffer(,
+ "   d.daticulocale as \"%s\",\n"
+ "   CASE d.datlocprovider WHEN 'c' THEN 'libc' 
WHEN 'i' THEN 'icu' END AS \"%s\",\
+ gettext_noop("ICU Locale"),
+ gettext_noop("Locale Provider"));
+   else
+   appendPQExpBuffer(,
+ "   d.datcollate as \"%s\",\n"  <--- there
+ "   'libc' AS \"%s\",\n",
+ gettext_noop("ICU Locale"),
+ gettext_noop("Locale Provider"));

I'd think this should rather be

+ "   '' as \"%s\",\n"

Christoph