From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Allow to_date() and to_timestamp() to accept localized names
Date: Sat, 11 Jan 2020 17:06:02 +0100
Message-ID: <20200111160602.4wwvyl7lo3oxoewf@development> (raw)
In-Reply-To: <CAC+AXB12A7Fr9djoq7f1-WYesnZTRKpGpHM12MRG+M00mewN9g@mail.gmail.com>
References: <CAC+AXB2JOe4Dw1teZP-N-isNyDdY3F96oGFVYFxhmWtsbgi=ig@mail.gmail.com>
<20190925195745.GA21894@alvherre.pgsql>
<CAC+AXB12A7Fr9djoq7f1-WYesnZTRKpGpHM12MRG+M00mewN9g@mail.gmail.com>
On Thu, Sep 26, 2019 at 08:36:25PM +0200, Juan José Santamaría Flecha wrote:
>On Wed, Sep 25, 2019 at 9:57 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>>
>> This patch no longer applies. Can you please rebase?
>
>Thank you for the notification.
>
>The patch rot after commit 1a950f3, a rebased version is attached.
>
Thanks. I did a quick review of this patch, and I think it's almost RFC.
I only found a couple of minor issue:
- In func.sgml, it seems we've lost this bit:
<para>
<literal>TM</literal> does not include trailing blanks.
<function>to_timestamp</function> and <function>to_date</function> ignore
the <literal>TM</literal> modifier.
</para>
Does that mean the function no longer ignore the TM modifier? That
would be somewhat problematic (i.e. it might break some user code).
But looking at the code I don't see why the patch would have this
effect, so I suppose it's merely a doc bug.
- I don't think we need to include examples how to_timestmap ignores
case, I'd say just stating the fact is clear enough. But if we want to
have examples, I think we should not inline in the para but use the
established pattern:
<para>
Some examples:
<programlisting>
...
</programlisting>
</para>
which is used elsewhere in the func.sgml file.
- In formatting.c the "common/unicode_norm.h" should be right after
includes from "catalog/" to follow the alphabetic order (unless
there's a reason why that does not work).
- I rather dislike the "dim" parameter name, because I immediately think
"dimension" which makes little sense. I suggest renaming to "nitems"
or "nelements" or something like that.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, juanjo.santamaria@gmail.com, alvherre@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Allow to_date() and to_timestamp() to accept localized names
In-Reply-To: <20200111160602.4wwvyl7lo3oxoewf@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox