Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.92) (envelope-from ) id 1j89mx-0006tV-LN for pgsql-hackers@arkaria.postgresql.org; Sat, 29 Feb 2020 21:36:47 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1j89mt-0005vS-Pv for pgsql-hackers@arkaria.postgresql.org; Sat, 29 Feb 2020 21:36:43 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1j89ms-0005vK-K8 for pgsql-hackers@lists.postgresql.org; Sat, 29 Feb 2020 21:36:42 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1j89mk-0000XA-Ew for pgsql-hackers@lists.postgresql.org; Sat, 29 Feb 2020 21:36:40 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id 01TLaPmi016876; Sat, 29 Feb 2020 16:36:25 -0500 From: Tom Lane To: =?UTF-8?Q?Juan_Jos=C3=A9_Santamar=C3=ADa_Flecha?= cc: Peter Eisentraut , Mark Dilger , Alvaro Herrera , Arthur Zakirov , Tomas Vondra , PostgreSQL Hackers Subject: Re: Allow to_date() and to_timestamp() to accept localized names In-reply-to: References: <20200124161814.GA8989@alvherre.pgsql> <32487.1579882979@sss.pgh.pa.us> <2f83150e-a2c0-6318-3125-d9b86e421aa2@2ndquadrant.com> <964.1579884408@sss.pgh.pa.us> <7212.1580177517@sss.pgh.pa.us> <2E3D8D71-3B55-4735-A047-1FECE1506CD1@enterprisedb.com> <6642.1580223087@sss.pgh.pa.us> <4144A0A0-4820-458D-A105-D5EC0E0D133B@enterprisedb.com> Comments: In-reply-to =?UTF-8?Q?Juan_Jos=C3=A9_Santamar=C3=ADa_Flecha?= message dated "Fri, 31 Jan 2020 14:02:51 +0100" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <16844.1583012152.0@sss.pgh.pa.us> Date: Sat, 29 Feb 2020 16:36:25 -0500 Message-ID: <16875.1583012185@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <16844.1583012152.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable =3D?UTF-8?Q?Juan_Jos=3DC3=3DA9_Santamar=3DC3=3DADa_Flecha?=3D writes: > On Tue, Jan 28, 2020 at 9:35 PM Peter Eisentraut < > peter.eisentraut@2ndquadrant.com> wrote: >> So I'm withdrawing my concerns with respect to this patch. As long as >> it can do a roundtrip conversion with to_char(), it's fine. > We can avoid issues with non injective case conversion languages with a > double conversion, so both strings in the comparison end up in the same > state. > I propose an upper/lower conversion as in the attached patch. This seems pretty reasonable to me, with a couple of caveats: * I don't think it's appropriate to hard-wire DEFAULT_COLLATION_OID as the collation to do case-folding with. For to_date/to_timestamp, we have collatable text input so we can rely on the function's input collation instead, at the cost of having to pass down the collation OID through a few layers of subroutines :-(. For parse_datetime, I punted for now and let it use DEFAULT_COLLATION_OID, because that's currently only called by JSONB code that probably hasn't got a usable input collation anyway (since jsonb isn't considered collatable). * I think it's likely worthwhile to do a quick check for an exact match before we go through all these case-folding pushups. If the expected use-case is reversing to_char() output, that will win all the time. We're probably ahead of the game even if it only matches a few percent of the time. Attached v10 incorporates those improvements, plus a bit of polishing of docs and comments. Barring objections, this seems committable to me. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; charset="utf-8"; name="0001-Allow-localized-month-names-to_date-v10.patch" Content-ID: <16844.1583012152.2@sss.pgh.pa.us> Content-Description: 0001-Allow-localized-month-names-to_date-v10.patch Content-Transfer-Encoding: quoted-printable diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 28035f1..8b73e05 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -5968,7 +5968,7 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*= )){1,1}'); TM prefix - translation mode (print localized day and month names base= d on + translation mode (use localized day and month names based = on ) TMMonth @@ -5999,9 +5999,20 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.= *)){1,1}'); = - TM does not include trailing blanks. - to_timestamp and to_date= ignore - the TM modifier. + TM suppresses trailing blanks whether or + not FM is specified. + + + + + + to_timestamp and to_date + ignore letter case in the input; so for + example MON, Mon, + and mon all accept the same strings. When usin= g + the TM modifier, case-folding is done according= to + the rules of the function's input collation (see + ). = diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/fo= rmatting.c index d029468..db99a6b 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -1038,7 +1038,7 @@ static void parse_format(FormatNode *node, const cha= r *str, const KeyWord *kw, static void DCH_to_char(FormatNode *node, bool is_interval, TmToChar *in, char *out, Oid collid); static void DCH_from_char(FormatNode *node, const char *in, TmFromChar *o= ut, - bool std, bool *have_error); + Oid collid, bool std, bool *have_error); = #ifdef DEBUG_TO_FROM_CHAR static void dump_index(const KeyWord *k, const int *index); @@ -1057,11 +1057,14 @@ static int from_char_parse_int_len(int *dest, cons= t char **src, const int len, FormatNode *node, bool *have_error); static int from_char_parse_int(int *dest, const char **src, FormatNode *n= ode, bool *have_error); -static int seq_search(const char *name, const char *const *array, int *le= n); +static int seq_search_ascii(const char *name, const char *const *array, i= nt *len); +static int seq_search_localized(const char *name, char **array, int *len, + Oid collid); static int from_char_seq_search(int *dest, const char **src, const char *const *array, + char **localized_array, Oid collid, FormatNode *node, bool *have_error); -static void do_to_timestamp(text *date_txt, text *fmt, bool std, +static void do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool s= td, struct pg_tm *tm, fsec_t *fsec, int *fprec, uint32 *flags, bool *have_error); static char *fill_str(char *str, int c, int max); @@ -2457,7 +2460,7 @@ from_char_parse_int(int *dest, const char **src, For= matNode *node, bool *have_er * suitable for comparisons to ASCII strings. */ static int -seq_search(const char *name, const char *const *array, int *len) +seq_search_ascii(const char *name, const char *const *array, int *len) { unsigned char firstc; const char *const *a; @@ -2503,8 +2506,89 @@ seq_search(const char *name, const char *const *arr= ay, int *len) } = /* - * Perform a sequential search in 'array' for an entry matching the first - * character(s) of the 'src' string case-insensitively. + * Sequentially search an array of possibly non-English words for + * a case-insensitive match to the initial character(s) of "name". + * + * This has the same API as seq_search_ascii(), but we use a more general + * case-folding transformation to achieve case-insensitivity. Case foldi= ng + * is done per the rules of the collation identified by "collid". + * + * The array is treated as const, but we don't declare it that way becaus= e + * the arrays exported by pg_locale.c aren't const. + */ +static int +seq_search_localized(const char *name, char **array, int *len, Oid collid= ) +{ + char **a; + char *upper_name; + char *lower_name; + + *len =3D 0; + + /* empty string can't match anything */ + if (!*name) + return -1; + + /* + * The case-folding processing done below is fairly expensive, so before + * doing that, make a quick pass to see if there is an exact match. + */ + for (a =3D array; *a !=3D NULL; a++) + { + int element_len =3D strlen(*a); + + if (strncmp(name, *a, element_len) =3D=3D 0) + { + *len =3D element_len; + return a - array; + } + } + + /* + * Fold to upper case, then to lower case, so that we can match reliably + * even in languages in which case conversions are not injective. + */ + upper_name =3D str_toupper(unconstify(char *, name), strlen(name), colli= d); + lower_name =3D str_tolower(upper_name, strlen(upper_name), collid); + pfree(upper_name); + + for (a =3D array; *a !=3D NULL; a++) + { + char *upper_element; + char *lower_element; + int element_len; + + /* Likewise upper/lower-case array element */ + upper_element =3D str_toupper(*a, strlen(*a), collid); + lower_element =3D str_tolower(upper_element, strlen(upper_element), + collid); + pfree(upper_element); + element_len =3D strlen(lower_element); + + /* Match? */ + if (strncmp(lower_name, lower_element, element_len) =3D=3D 0) + { + *len =3D element_len; + pfree(lower_element); + pfree(lower_name); + return a - array; + } + pfree(lower_element); + } + + pfree(lower_name); + return -1; +} + +/* + * Perform a sequential search in 'array' (or 'localized_array', if that'= s + * not NULL) for an entry matching the first character(s) of the 'src' + * string case-insensitively. + * + * The 'array' is presumed to be English words (all-ASCII), but + * if 'localized_array' is supplied, that might be non-English + * so we need a more expensive case-folding transformation + * (which will follow the rules of the collation 'collid'). * * If a match is found, copy the array index of the match into the intege= r * pointed to by 'dest', advance 'src' to the end of the part of the stri= ng @@ -2518,11 +2602,15 @@ seq_search(const char *name, const char *const *ar= ray, int *len) */ static int from_char_seq_search(int *dest, const char **src, const char *const *arra= y, + char **localized_array, Oid collid, FormatNode *node, bool *have_error) { int len; = - *dest =3D seq_search(*src, array, &len); + if (localized_array =3D=3D NULL) + *dest =3D seq_search_ascii(*src, array, &len); + else + *dest =3D seq_search_localized(*src, localized_array, &len, collid); = if (len <=3D 0) { @@ -3147,19 +3235,20 @@ DCH_to_char(FormatNode *node, bool is_interval, Tm= ToChar *in, char *out, Oid col *s =3D '\0'; } = -/* ---------- - * Process a string as denoted by a list of FormatNodes. +/* + * Process the string 'in' as denoted by the array of FormatNodes 'node[]= '. * The TmFromChar struct pointed to by 'out' is populated with the result= s. * + * 'collid' identifies the collation to use, if needed. + * 'std' specifies standard parsing mode. + * If 'have_error' is NULL, then errors are thrown, else '*have_error' is= set. + * * Note: we currently don't have any to_interval() function, so there * is no need here for INVALID_FOR_INTERVAL checks. - * - * If 'have_error' is NULL, then errors are thrown, else '*have_error' is= set. - * ---------- */ static void -DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, bool std= , - bool *have_error) +DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, + Oid collid, bool std, bool *have_error) { FormatNode *n; const char *s; @@ -3170,6 +3259,9 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, /* number of extra skipped characters (more than given in format string)= */ int extra_skip =3D 0; = + /* cache localized days and months */ + cache_locale_time(); + for (n =3D node, s =3D in; n->type !=3D NODE_TYPE_END && *s !=3D '\0'; n= ++) { /* @@ -3271,6 +3363,7 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_a_m: case DCH_p_m: from_char_seq_search(&value, &s, ampm_strings_long, + NULL, InvalidOid, n, have_error); CHECK_ERROR; from_char_set_int(&out->pm, value % 2, n, have_error); @@ -3282,6 +3375,7 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_am: case DCH_pm: from_char_seq_search(&value, &s, ampm_strings, + NULL, InvalidOid, n, have_error); CHECK_ERROR; from_char_set_int(&out->pm, value % 2, n, have_error); @@ -3395,6 +3489,7 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_a_d: case DCH_b_c: from_char_seq_search(&value, &s, adbc_strings_long, + NULL, InvalidOid, n, have_error); CHECK_ERROR; from_char_set_int(&out->bc, value % 2, n, have_error); @@ -3405,6 +3500,7 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_ad: case DCH_bc: from_char_seq_search(&value, &s, adbc_strings, + NULL, InvalidOid, n, have_error); CHECK_ERROR; from_char_set_int(&out->bc, value % 2, n, have_error); @@ -3414,6 +3510,8 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_Month: case DCH_month: from_char_seq_search(&value, &s, months_full, + S_TM(n->suffix) ? localized_full_months : NULL, + collid, n, have_error); CHECK_ERROR; from_char_set_int(&out->mm, value + 1, n, have_error); @@ -3423,6 +3521,8 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_Mon: case DCH_mon: from_char_seq_search(&value, &s, months, + S_TM(n->suffix) ? localized_abbrev_months : NULL, + collid, n, have_error); CHECK_ERROR; from_char_set_int(&out->mm, value + 1, n, have_error); @@ -3437,6 +3537,8 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_Day: case DCH_day: from_char_seq_search(&value, &s, days, + S_TM(n->suffix) ? localized_full_days : NULL, + collid, n, have_error); CHECK_ERROR; from_char_set_int(&out->d, value, n, have_error); @@ -3447,6 +3549,8 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_Dy: case DCH_dy: from_char_seq_search(&value, &s, days_short, + S_TM(n->suffix) ? localized_abbrev_days : NULL, + collid, n, have_error); CHECK_ERROR; from_char_set_int(&out->d, value, n, have_error); @@ -3565,6 +3669,7 @@ DCH_from_char(FormatNode *node, const char *in, TmFr= omChar *out, bool std, case DCH_RM: case DCH_rm: from_char_seq_search(&value, &s, rm_months_lower, + NULL, InvalidOid, n, have_error); CHECK_ERROR; from_char_set_int(&out->mm, MONTHS_PER_YEAR - value, @@ -4031,13 +4136,15 @@ to_timestamp(PG_FUNCTION_ARGS) { text *date_txt =3D PG_GETARG_TEXT_PP(0); text *fmt =3D PG_GETARG_TEXT_PP(1); + Oid collid =3D PG_GET_COLLATION(); Timestamp result; int tz; struct pg_tm tm; fsec_t fsec; int fprec; = - do_to_timestamp(date_txt, fmt, false, &tm, &fsec, &fprec, NULL, NULL); + do_to_timestamp(date_txt, fmt, collid, false, + &tm, &fsec, &fprec, NULL, NULL); = /* Use the specified time zone, if any. */ if (tm.tm_zone) @@ -4072,11 +4179,13 @@ to_date(PG_FUNCTION_ARGS) { text *date_txt =3D PG_GETARG_TEXT_PP(0); text *fmt =3D PG_GETARG_TEXT_PP(1); + Oid collid =3D PG_GET_COLLATION(); DateADT result; struct pg_tm tm; fsec_t fsec; = - do_to_timestamp(date_txt, fmt, false, &tm, &fsec, NULL, NULL, NULL); + do_to_timestamp(date_txt, fmt, collid, false, + &tm, &fsec, NULL, NULL, NULL); = /* Prevent overflow in Julian-day routines */ if (!IS_VALID_JULIAN(tm.tm_year, tm.tm_mon, tm.tm_mday)) @@ -4116,7 +4225,13 @@ parse_datetime(text *date_txt, text *fmt, bool stri= ct, Oid *typid, int fprec; uint32 flags; = - do_to_timestamp(date_txt, fmt, strict, &tm, &fsec, &fprec, &flags, have_= error); + /* + * At some point we might wish to have callers supply the collation to + * use, but right now it's not clear that they'd be able to do better th= an + * DEFAULT_COLLATION_OID anyway. + */ + do_to_timestamp(date_txt, fmt, DEFAULT_COLLATION_OID, strict, + &tm, &fsec, &fprec, &flags, have_error); CHECK_ERROR; = *typmod =3D fprec ? fprec : -1; /* fractional part precision */ @@ -4278,21 +4393,21 @@ on_error: * Parse the 'date_txt' according to 'fmt', return results as a struct pg= _tm, * fractional seconds, and fractional precision. * + * 'collid' identifies the collation to use, if needed. + * 'std' specifies standard parsing mode. + * Bit mask of date/time/zone components found in 'fmt' is returned in 'f= lags', + * if that is not NULL. + * If 'have_error' is NULL, then errors are thrown, else '*have_error' is= set. + * * We parse 'fmt' into a list of FormatNodes, which is then passed to * DCH_from_char to populate a TmFromChar with the parsed contents of * 'date_txt'. * * The TmFromChar is then analysed and converted into the final results i= n - * struct 'tm' and 'fsec'. - * - * Bit mask of date/time/zone components found in 'fmt' is returned in 'f= lags'. - * - * 'std' specifies standard parsing mode. - * - * If 'have_error' is NULL, then errors are thrown, else '*have_error' is= set. + * struct 'tm', 'fsec', and 'fprec'. */ static void -do_to_timestamp(text *date_txt, text *fmt, bool std, +do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, struct pg_tm *tm, fsec_t *fsec, int *fprec, uint32 *flags, bool *have_error) { @@ -4352,7 +4467,7 @@ do_to_timestamp(text *date_txt, text *fmt, bool std, /* dump_index(DCH_keywords, DCH_index); */ #endif = - DCH_from_char(format, date_str, &tmfc, std, have_error); + DCH_from_char(format, date_str, &tmfc, collid, std, have_error); CHECK_ERROR; = pfree(fmt_str); diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_= locale.c index 25fb7e2..64fd3ae 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -96,11 +96,17 @@ char *locale_monetary; char *locale_numeric; char *locale_time; = -/* lc_time localization cache */ -char *localized_abbrev_days[7]; -char *localized_full_days[7]; -char *localized_abbrev_months[12]; -char *localized_full_months[12]; +/* + * lc_time localization cache. + * + * We use only the first 7 or 12 entries of these arrays. The last array + * element is left as NULL for the convenience of outside code that wants + * to sequentially scan these arrays. + */ +char *localized_abbrev_days[7 + 1]; +char *localized_full_days[7 + 1]; +char *localized_abbrev_months[12 + 1]; +char *localized_full_months[12 + 1]; = /* indicates whether locale information cache is valid */ static bool CurrentLocaleConvValid =3D false; @@ -922,6 +928,8 @@ cache_locale_time(void) cache_single_string(&localized_full_days[i], bufptr, encoding); bufptr +=3D MAX_L10N_DATA; } + localized_abbrev_days[7] =3D NULL; + localized_full_days[7] =3D NULL; = /* localized months */ for (i =3D 0; i < 12; i++) @@ -931,6 +939,8 @@ cache_locale_time(void) cache_single_string(&localized_full_months[i], bufptr, encoding); bufptr +=3D MAX_L10N_DATA; } + localized_abbrev_months[12] =3D NULL; + localized_full_months[12] =3D NULL; = CurrentLCTimeValid =3D true; } diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/r= egress/expected/collate.linux.utf8.out index 37c6add..f06ae54 100644 --- a/src/test/regress/expected/collate.linux.utf8.out +++ b/src/test/regress/expected/collate.linux.utf8.out @@ -461,6 +461,22 @@ SELECT to_char(date '2010-04-01', 'DD TMMON YYYY' COL= LATE "tr_TR"); 01 N=C4=B0S 2010 (1 row) = +-- to_date +SELECT to_date('01 =C5=9EUB 2010', 'DD TMMON YYYY'); + to_date = +------------ + 02-01-2010 +(1 row) + +SELECT to_date('01 =C5=9Eub 2010', 'DD TMMON YYYY'); + to_date = +------------ + 02-01-2010 +(1 row) + +SELECT to_date('1234567890ab 2010', 'TMMONTH YYYY'); -- fail +ERROR: invalid value "1234567890ab" 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" = >=3D 'bbc'; CREATE VIEW collview2 AS SELECT a, b FROM collate_test1 ORDER BY b COLLAT= E "C"; diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regres= s/sql/collate.linux.utf8.sql index 8c26f16..cbbd220 100644 --- a/src/test/regress/sql/collate.linux.utf8.sql +++ b/src/test/regress/sql/collate.linux.utf8.sql @@ -182,6 +182,12 @@ SELECT to_char(date '2010-02-01', 'DD TMMON YYYY' COL= LATE "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 + +SELECT to_date('01 =C5=9EUB 2010', 'DD TMMON YYYY'); +SELECT to_date('01 =C5=9Eub 2010', 'DD TMMON YYYY'); +SELECT to_date('1234567890ab 2010', 'TMMONTH YYYY'); -- fail + = -- backwards parsing = ------- =_aaaaaaaaaa0--