From: | Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Allow to_date() and to_timestamp() to accept localized names |
Date: | 2020-01-13 12:04:48 |
Message-ID: | CAC+AXB1rLC7Gu7-fTQPNwSY9sFC9hkpn8_J3WyGwaKXvNZnoVQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jan 11, 2020 at 5:06 PM Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:
>
> Thanks. I did a quick review of this patch, and I think it's almost RFC.
>
>
Thanks for reviewing.
> - 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.
>
>
It is intentional. This patch uses the TM modifier to identify the usage of
localized names as input for to_timestamp() and to_date().
> - 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.
>
I was trying to match the style surrounding the usage notes for date/time
formatting [1]. Agreed that it is not worth an example on its own, so
dropped.
>
> - 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).
>
Fixed.
>
> - 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.
>
Agreed, using "nelements" as a better style matchup.
Please, find attached a version addressing the above mentioned.
[1] https://www.postgresql.org/docs/current/functions-formatting.html
Regards,
Juan José Santamaría Flecha
>
Attachment | Content-Type | Size |
---|---|---|
0001-Allow-localized-month-names-to_date-v5.patch | application/octet-stream | 16.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Julien Rouhaud | 2020-01-13 12:09:01 | Re: isTempNamespaceInUse() is incorrect with its handling of MyBackendId |
Previous Message | Peter Eisentraut | 2020-01-13 10:57:53 | Re: our checks for read-only queries are not great |