Re: Allow to_date() and to_timestamp() to accept localized names
Juan José Santamaría Flecha <[email protected]> Mon, 13 Jan 2020 13:04:48 +0100
| Newsgroups | gmane.comp.db.postgresql.devel.general |
|---|---|
| Message-ID | <CAC+AXB1rLC7Gu7-fTQPNwSY9sFC9hkpn8_J3WyGwaKXvNZnoVQ@mail.gmail.com> |
On Sat, Jan 11, 2020 at 5:06 PM Tomas Vondra <[email protected]> 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 >
0001-Allow-localized-month-names-to_date-v5.patch
(application/octet-stream, 16.9 KB)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4b42f12..4f50069 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -6574,7 +6574,7 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
</row>
<row>
<entry><literal>TM</literal> prefix</entry>
- <entry>translation mode (print localized day and month names based on
+ <entry>translation mode (use localized day and month names based on
<xref linkend="guc-lc-time"/>)</entry>
<entry><literal>TMMonth</literal></entry>
</row>
@@ -6606,8 +6606,13 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
<listitem>
<para>
<literal>TM</literal> does not include trailing blanks.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
<function>to_timestamp</function> and <function>to_date</function> ignore
- the <literal>TM</literal> modifier.
+ the case when receiving names as an input.
</para>
</listitem>
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index ca3c48d..cf3435f 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -87,6 +87,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/unicode_norm.h"
#include "mb/pg_wchar.h"
#include "utils/builtins.h"
#include "utils/date.h"
@@ -220,11 +221,11 @@ typedef struct
*/
static const char *const months_full[] = {
"January", "February", "March", "April", "May", "June", "July",
- "August", "September", "October", "November", "December", NULL
+ "August", "September", "October", "November", "December"
};
static const char *const days_short[] = {
- "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", NULL
+ "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
};
/* ----------
@@ -256,8 +257,8 @@ static const char *const days_short[] = {
* matches for BC have an odd index. So the boolean value for BC is given by
* taking the array index of the match, modulo 2.
*/
-static const char *const adbc_strings[] = {ad_STR, bc_STR, AD_STR, BC_STR, NULL};
-static const char *const adbc_strings_long[] = {a_d_STR, b_c_STR, A_D_STR, B_C_STR, NULL};
+static const char *const adbc_strings[] = {ad_STR, bc_STR, AD_STR, BC_STR};
+static const char *const adbc_strings_long[] = {a_d_STR, b_c_STR, A_D_STR, B_C_STR};
/* ----------
* AM / PM
@@ -283,8 +284,8 @@ static const char *const adbc_strings_long[] = {a_d_STR, b_c_STR, A_D_STR, B_C_S
* matches for PM have an odd index. So the boolean value for PM is given by
* taking the array index of the match, modulo 2.
*/
-static const char *const ampm_strings[] = {am_STR, pm_STR, AM_STR, PM_STR, NULL};
-static const char *const ampm_strings_long[] = {a_m_STR, p_m_STR, A_M_STR, P_M_STR, NULL};
+static const char *const ampm_strings[] = {am_STR, pm_STR, AM_STR, PM_STR};
+static const char *const ampm_strings_long[] = {a_m_STR, p_m_STR, A_M_STR, P_M_STR};
/* ----------
* Months in roman-numeral
@@ -293,10 +294,10 @@ static const char *const ampm_strings_long[] = {a_m_STR, p_m_STR, A_M_STR, P_M_S
* ----------
*/
static const char *const rm_months_upper[] =
-{"XII", "XI", "X", "IX", "VIII", "VII", "VI", "V", "IV", "III", "II", "I", NULL};
+{"XII", "XI", "X", "IX", "VIII", "VII", "VI", "V", "IV", "III", "II", "I"};
static const char *const rm_months_lower[] =
-{"xii", "xi", "x", "ix", "viii", "vii", "vi", "v", "iv", "iii", "ii", "i", NULL};
+{"xii", "xi", "x", "ix", "viii", "vii", "vi", "v", "iv", "iii", "ii", "i"};
/* ----------
* Roman numbers
@@ -1068,10 +1069,13 @@ static int from_char_parse_int_len(int *dest, char **src, const int len,
FormatNode *node, bool *have_error);
static int from_char_parse_int(int *dest, char **src, FormatNode *node,
bool *have_error);
-static int seq_search(char *name, const char *const *array, int type, int max, int *len);
-static int from_char_seq_search(int *dest, char **src,
- const char *const *array, int type, int max,
- FormatNode *node, bool *have_error);
+static int seq_search_sqlascii(char *name, const char *const *array, int type,
+ int max, int *len, int nelements);
+static int seq_search_localized(char *name, char **array, int max, int *len,
+ int nelements);
+static int from_char_seq_search(int *dest, char **src, const char *const *array,
+ char **localized_array, int type, int max,
+ FormatNode *node, bool *have_error, int nelements);
static void do_to_timestamp(text *date_txt, text *fmt, bool std,
struct pg_tm *tm, fsec_t *fsec, int *fprec,
uint32 *flags, bool *have_error);
@@ -2454,17 +2458,18 @@ from_char_parse_int(int *dest, char **src, FormatNode *node, bool *have_error)
}
/* ----------
- * Sequential search with to upper/lower conversion
+ * Sequential search with to upper/lower conversion for SQL_ASCII array input
* ----------
*/
static int
-seq_search(char *name, const char *const *array, int type, int max, int *len)
+seq_search_sqlascii(char *name, const char *const *array, int type, int max, int *len, int nelements)
{
const char *p;
const char *const *a;
char *n;
int last,
i;
+ int index;
*len = 0;
@@ -2477,7 +2482,7 @@ seq_search(char *name, const char *const *array, int type, int max, int *len)
else if (type == ALL_LOWER)
*name = pg_tolower((unsigned char) *name);
- for (last = 0, a = array; *a != NULL; a++)
+ for (last = 0, a = array, index = 0; index < nelements; a++, index++)
{
/* compare first chars */
if (*name != **a)
@@ -2489,13 +2494,13 @@ seq_search(char *name, const char *const *array, int type, int max, int *len)
if (max && i == max)
{
*len = i;
- return a - array;
+ return index;
}
/* full size */
if (*p == '\0')
{
*len = i;
- return a - array;
+ return index;
}
/* Not found in array 'a' */
if (*n == '\0')
@@ -2525,6 +2530,84 @@ seq_search(char *name, const char *const *array, int type, int max, int *len)
return -1;
}
+/* ----------
+ * Sequential search with initcap conversion for localized array input
+ * ----------
+ */
+static int
+seq_search_localized(char *name, char **array, int max, int *len, int nelements)
+{
+ char **a;
+ char *initcap_element;
+ char *initcap_name;
+ char *norm_name;
+ int index;
+ int mb_max;
+ int name_len;
+ int encoding;
+ int norm_len;
+ int element_len;
+
+ *len = 0;
+
+ if (!*name)
+ return -1;
+
+ encoding = GetDatabaseEncoding();
+ mb_max = max * pg_encoding_max_length(encoding);
+ name_len = strlen(name);
+ name_len = name_len < mb_max ? name_len : mb_max;
+ norm_name = name;
+ norm_len = name_len;
+
+ /* Normalize and initcap name */
+ if (mb_max > max && encoding == PG_UTF8)
+ {
+ pg_wchar *wchar_name;
+ pg_wchar *norm_wname;
+ size_t name_wlen;
+ size_t norm_wlen;
+
+ wchar_name = (pg_wchar *) palloc((name_len + 1) * sizeof(pg_wchar));
+ name_wlen = pg_mb2wchar_with_len(name, wchar_name, name_len);
+ norm_wname = unicode_normalize_kc(wchar_name);
+ pfree(wchar_name);
+ norm_wlen = pg_wchar_strlen(norm_wname);
+ if (name_wlen > norm_wlen)
+ {
+ norm_name = (char *) palloc((norm_wlen + 1) * sizeof(pg_wchar));
+ norm_len = pg_wchar2mb_with_len(norm_wname, norm_name, norm_wlen);
+ }
+ pfree(norm_wname);
+ }
+ initcap_name = str_initcap(norm_name, norm_len, DEFAULT_COLLATION_OID);
+ if (name_len != norm_len)
+ pfree(norm_name);
+
+ for (a = array, index = 0; index < nelements; a++, index++)
+ {
+ /* Initcap element, assume it is normalized */
+ element_len = strlen(*a);
+ initcap_element = str_initcap(*a, element_len, DEFAULT_COLLATION_OID);
+
+#ifdef DEBUG_TO_FROM_CHAR
+ elog(DEBUG_elog_output, "Name: 0x%x, Normalized: 0x%x, Element: 0x%x",
+ (unsigned char)*name, (unsigned char)*initcap_name, (unsigned char)*initcap_element);
+#endif
+ if (strncmp(initcap_name, initcap_element, element_len) == 0)
+ {
+ *len = element_len + name_len - norm_len;
+ pfree(initcap_element);
+ pfree(initcap_name);
+ return index;
+ }
+ pfree(initcap_element);
+ }
+
+ pfree(initcap_name);
+ return -1;
+}
+
/*
* Perform a sequential search in 'array' for text matching the first 'max'
* characters of the source string.
@@ -2537,16 +2620,20 @@ seq_search(char *name, const char *const *array, int type, int max, int *len)
* otherwise set '*have_error' and return -1.
*/
static int
-from_char_seq_search(int *dest, char **src, const char *const *array, int type,
- int max, FormatNode *node, bool *have_error)
+from_char_seq_search(int *dest, char **src, const char *const *array, char **localized_array,
+ int type, int max, FormatNode *node, bool *have_error, int nelements)
{
int len;
- *dest = seq_search(*src, array, type, max, &len);
+ if (localized_array == NULL)
+ *dest = seq_search_sqlascii(*src, array, type, max, &len, nelements);
+ else
+ *dest = seq_search_localized(*src, localized_array, max, &len, nelements);
if (len <= 0)
{
char copy[DCH_MAX_ITEM_SIZ + 1];
+ /* We use byte length, localized names encoding is ignored */
Assert(max <= DCH_MAX_ITEM_SIZ);
strlcpy(copy, *src, max + 1);
@@ -3174,12 +3261,16 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
int len,
value;
bool fx_mode = std;
+ char **localized_names;
/* number of extra skipped characters (more than given in format string) */
int extra_skip = 0;
+ /* cache localized days and months */
+ cache_locale_time();
for (n = node, s = in; n->type != NODE_TYPE_END && *s != '\0'; n++)
{
+ localized_names = NULL;
/*
* Ignore spaces at the beginning of the string and before fields when
* not in FX (fixed width) mode.
@@ -3278,8 +3369,9 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_P_M:
case DCH_a_m:
case DCH_p_m:
- from_char_seq_search(&value, &s, ampm_strings_long,
- ALL_UPPER, n->key->len, n, have_error);
+ from_char_seq_search(&value, &s, ampm_strings_long, localized_names,
+ ALL_UPPER, n->key->len, n, have_error,
+ lengthof(ampm_strings_long));
CHECK_ERROR;
from_char_set_int(&out->pm, value % 2, n, have_error);
CHECK_ERROR;
@@ -3289,8 +3381,9 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_PM:
case DCH_am:
case DCH_pm:
- from_char_seq_search(&value, &s, ampm_strings,
- ALL_UPPER, n->key->len, n, have_error);
+ from_char_seq_search(&value, &s, ampm_strings, localized_names,
+ ALL_UPPER, n->key->len, n, have_error,
+ lengthof(ampm_strings));
CHECK_ERROR;
from_char_set_int(&out->pm, value % 2, n, have_error);
CHECK_ERROR;
@@ -3402,8 +3495,9 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_B_C:
case DCH_a_d:
case DCH_b_c:
- from_char_seq_search(&value, &s, adbc_strings_long,
- ALL_UPPER, n->key->len, n, have_error);
+ from_char_seq_search(&value, &s, adbc_strings_long, localized_names,
+ ALL_UPPER, n->key->len, n, have_error,
+ lengthof(adbc_strings_long));
CHECK_ERROR;
from_char_set_int(&out->bc, value % 2, n, have_error);
CHECK_ERROR;
@@ -3412,8 +3506,9 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_BC:
case DCH_ad:
case DCH_bc:
- from_char_seq_search(&value, &s, adbc_strings,
- ALL_UPPER, n->key->len, n, have_error);
+ from_char_seq_search(&value, &s, adbc_strings, localized_names,
+ ALL_UPPER, n->key->len, n, have_error,
+ lengthof(adbc_strings));
CHECK_ERROR;
from_char_set_int(&out->bc, value % 2, n, have_error);
CHECK_ERROR;
@@ -3421,8 +3516,11 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_MONTH:
case DCH_Month:
case DCH_month:
- from_char_seq_search(&value, &s, months_full, ONE_UPPER,
- MAX_MONTH_LEN, n, have_error);
+ if (S_TM(n->suffix))
+ localized_names = localized_full_months;
+ from_char_seq_search(&value, &s, months_full, localized_names,
+ ONE_UPPER, MAX_MONTH_LEN, n, have_error,
+ lengthof(months_full));
CHECK_ERROR;
from_char_set_int(&out->mm, value + 1, n, have_error);
CHECK_ERROR;
@@ -3430,8 +3528,11 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_MON:
case DCH_Mon:
case DCH_mon:
- from_char_seq_search(&value, &s, months, ONE_UPPER,
- MAX_MON_LEN, n, have_error);
+ if (S_TM(n->suffix))
+ localized_names = localized_abbrev_months;
+ from_char_seq_search(&value, &s, months, localized_names,
+ ONE_UPPER, MAX_MON_LEN, n, have_error,
+ lengthof(months_full));
CHECK_ERROR;
from_char_set_int(&out->mm, value + 1, n, have_error);
CHECK_ERROR;
@@ -3444,8 +3545,11 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_DAY:
case DCH_Day:
case DCH_day:
- from_char_seq_search(&value, &s, days, ONE_UPPER,
- MAX_DAY_LEN, n, have_error);
+ if (S_TM(n->suffix))
+ localized_names = localized_full_days;
+ from_char_seq_search(&value, &s, days, localized_names,
+ ONE_UPPER, MAX_DAY_LEN, n, have_error,
+ lengthof(days_short));
CHECK_ERROR;
from_char_set_int(&out->d, value, n, have_error);
CHECK_ERROR;
@@ -3454,8 +3558,11 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
case DCH_DY:
case DCH_Dy:
case DCH_dy:
- from_char_seq_search(&value, &s, days, ONE_UPPER,
- MAX_DY_LEN, n, have_error);
+ if (S_TM(n->suffix))
+ localized_names = localized_abbrev_days;
+ from_char_seq_search(&value, &s, days_short, localized_names,
+ ONE_UPPER, MAX_DY_LEN, n, have_error,
+ lengthof(days_short));
CHECK_ERROR;
from_char_set_int(&out->d, value, n, have_error);
CHECK_ERROR;
@@ -3571,16 +3678,18 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out, bool std,
SKIP_THth(s, n->suffix);
break;
case DCH_RM:
- from_char_seq_search(&value, &s, rm_months_upper,
- ALL_UPPER, MAX_RM_LEN, n, have_error);
+ from_char_seq_search(&value, &s, rm_months_upper, localized_names,
+ ALL_UPPER, MAX_RM_LEN, n, have_error,
+ lengthof(rm_months_upper));
CHECK_ERROR;
from_char_set_int(&out->mm, MONTHS_PER_YEAR - value,
n, have_error);
CHECK_ERROR;
break;
case DCH_rm:
- from_char_seq_search(&value, &s, rm_months_lower,
- ALL_LOWER, MAX_RM_LEN, n, have_error);
+ from_char_seq_search(&value, &s, rm_months_lower, localized_names,
+ ALL_LOWER, MAX_RM_LEN, n, have_error,
+ lengthof(rm_months_lower));
CHECK_ERROR;
from_char_set_int(&out->mm, MONTHS_PER_YEAR - value,
n, have_error);
@@ -4363,8 +4472,8 @@ do_to_timestamp(text *date_txt, text *fmt, bool std,
}
#ifdef DEBUG_TO_FROM_CHAR
- /* dump_node(format, fmt_len); */
- /* dump_index(DCH_keywords, DCH_index); */
+ dump_node(format, fmt_len);
+ dump_index(DCH_keywords, DCH_index);
#endif
DCH_from_char(format, date_str, &tmfc, std, have_error);
diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out
index 37c6add..08606be 100644
--- a/src/test/regress/expected/collate.linux.utf8.out
+++ b/src/test/regress/expected/collate.linux.utf8.out
@@ -461,6 +461,24 @@ SELECT to_char(date '2010-04-01', 'DD TMMON YYYY' COLLATE "tr_TR");
01 NİS 2010
(1 row)
+-- to_date
+SET DateStyle='ISO, YMD';
+SET TimeZone='UTC';
+SELECT to_timestamp('01 ŞUB 2010', 'DD TMMON YYYY'); -- normalized \u015E
+ to_timestamp
+------------------------
+ 2010-02-01 00:00:00+00
+(1 row)
+
+SELECT to_timestamp('01 ŞUB 2010', 'DD TMMON YYYY'); -- not normalized \u0053+\u0327
+ to_timestamp
+------------------------
+ 2010-02-01 00:00:00+00
+(1 row)
+
+SELECT to_timestamp('1234567890ab 2010', 'TMMONTH YYYY'); -- fail
+ERROR: invalid value "123456789" for "MONTH"
+DETAIL: The given value did not match any of the allowed values for this field.
-- backwards parsing
CREATE VIEW collview1 AS SELECT * FROM collate_test1 WHERE b COLLATE "C" >= 'bbc';
CREATE VIEW collview2 AS SELECT a, b FROM collate_test1 ORDER BY b COLLATE "C";
diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regress/sql/collate.linux.utf8.sql
index 8c26f16..ebe6ede 100644
--- a/src/test/regress/sql/collate.linux.utf8.sql
+++ b/src/test/regress/sql/collate.linux.utf8.sql
@@ -182,6 +182,14 @@ SELECT to_char(date '2010-02-01', 'DD TMMON YYYY' COLLATE "tr_TR");
SELECT to_char(date '2010-04-01', 'DD TMMON YYYY');
SELECT to_char(date '2010-04-01', 'DD TMMON YYYY' COLLATE "tr_TR");
+-- to_date
+
+SET DateStyle='ISO, YMD';
+SET TimeZone='UTC';
+SELECT to_timestamp('01 ŞUB 2010', 'DD TMMON YYYY'); -- normalized \u015E
+SELECT to_timestamp('01 ŞUB 2010', 'DD TMMON YYYY'); -- not normalized \u0053+\u0327
+SELECT to_timestamp('1234567890ab 2010', 'TMMONTH YYYY'); -- fail
+
-- backwards parsing