From f43d27142a76fcbabf49e45b9457f8376744e759 Mon Sep 17 00:00:00 2001 From: Joseph Koshakow Date: Sat, 2 Apr 2022 14:42:18 -0400 Subject: [PATCH 2/2] Fix sql standard style negative semantics --- src/backend/utils/adt/datetime.c | 107 ++++++++++++++----------- src/test/regress/expected/interval.out | 14 ++++ src/test/regress/sql/interval.sql | 5 ++ 3 files changed, 79 insertions(+), 47 deletions(-) diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c index dae90e4a9e..5842d249ab 100644 --- a/src/backend/utils/adt/datetime.c +++ b/src/backend/utils/adt/datetime.c @@ -50,6 +50,8 @@ static int DecodeDate(char *str, int fmask, int *tmask, bool *is2digits, static char *AppendSeconds(char *cp, int sec, fsec_t fsec, int precision, bool fillzeros); static bool int64_multiply_add(int64 val, int64 multiplier, int64 *sum); +static void AdjustForSqlStandardGlobalNegative(int64 *val, double *fval, + bool global_negative); static bool AdjustFractMicroseconds(double frac, int64 scale, struct pg_itm_in *itm_in); static bool AdjustFractDays(double frac, int scale, @@ -527,6 +529,19 @@ int64_multiply_add(int64 val, int64 multiplier, int64 *sum) return true; } +/* + * Adjust values sign if SQL Standard style is being used and there's a + * single leading negative sign. + */ +static void AdjustForSqlStandardGlobalNegative(int64 *val, double *fval, + bool global_negative) +{ + if (*val > 0 && global_negative) { + *val = -*val; + *fval = -*fval; + } +} + /* * Multiply frac by scale (to produce microseconds) and add to itm_in->tm_usec. * Returns true if successful, false if itm_in overflows. @@ -3307,10 +3322,43 @@ DecodeInterval(char **field, int *ftype, int nf, int range, int64 val; double fval; + bool global_negative = false; + *dtype = DTK_DELTA; type = IGNORE_DTF; ClearPgItmIn(itm_in); + /*---------- + * The SQL standard defines the interval literal + * '-1 1:00:00' + * to mean "negative 1 days and negative 1 hours", while Postgres + * traditionally treats this as meaning "negative 1 days and positive + * 1 hours". In SQL_STANDARD intervalstyle, we apply the leading sign + * to all fields if there are no other explicit signs. + * + * We leave the signs alone if there are additional explicit signs. + * This protects us against misinterpreting postgres-style dump output, + * since the postgres-style output code has always put an explicit sign on + * all fields following a negative field. But note that SQL-spec output + * is ambiguous and can be misinterpreted on load! (So it's best practice + * to dump in postgres style, not SQL style.) + *---------- + */ + if (IntervalStyle == INTSTYLE_SQL_STANDARD && *field[0] == '-') + { + /* Check for additional explicit signs */ + bool more_signs = false; + for (i = 1; i < nf; i++) + { + if (*field[i] == '-' || *field[i] == '+') + { + more_signs = true; + break; + } + } + global_negative = !more_signs; + } + /* read through list backwards to pick up units before values */ for (i = nf - 1; i >= 0; i--) { @@ -3447,18 +3495,21 @@ DecodeInterval(char **field, int *ftype, int nf, int range, switch (type) { case DTK_MICROSEC: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustMicroseconds(val, fval, 1, itm_in)) return DTERR_FIELD_OVERFLOW; tmask = DTK_M(MICROSECOND); break; case DTK_MILLISEC: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustMicroseconds(val, fval, 1000, itm_in)) return DTERR_FIELD_OVERFLOW; tmask = DTK_M(MILLISECOND); break; case DTK_SECOND: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustMicroseconds(val, fval, USECS_PER_SEC, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3473,12 +3524,14 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_MINUTE: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustMicroseconds(val, fval, USECS_PER_MINUTE, itm_in)) return DTERR_FIELD_OVERFLOW; tmask = DTK_M(MINUTE); break; case DTK_HOUR: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustMicroseconds(val, fval, USECS_PER_HOUR, itm_in)) return DTERR_FIELD_OVERFLOW; tmask = DTK_M(HOUR); @@ -3486,6 +3539,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_DAY: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustDays(val, 1, itm_in) || !AdjustFractMicroseconds(fval, USECS_PER_DAY, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3493,6 +3547,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_WEEK: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustDays(val, 7, itm_in) || !AdjustFractDays(fval, 7, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3500,6 +3555,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_MONTH: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustMonths(val, itm_in) || !AdjustFractDays(fval, DAYS_PER_MONTH, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3507,6 +3563,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_YEAR: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustYears(val, 1, itm_in) || !AdjustFractYears(fval, 1, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3514,6 +3571,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_DECADE: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustYears(val, 10, itm_in) || !AdjustFractYears(fval, 10, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3521,6 +3579,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_CENTURY: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustYears(val, 100, itm_in) || !AdjustFractYears(fval, 100, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3528,6 +3587,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range, break; case DTK_MILLENNIUM: + AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative); if (!AdjustYears(val, 1000, itm_in) || !AdjustFractYears(fval, 1000, itm_in)) return DTERR_FIELD_OVERFLOW; @@ -3580,53 +3640,6 @@ DecodeInterval(char **field, int *ftype, int nf, int range, if (fmask == 0) return DTERR_BAD_FORMAT; - /*---------- - * The SQL standard defines the interval literal - * '-1 1:00:00' - * to mean "negative 1 days and negative 1 hours", while Postgres - * traditionally treats this as meaning "negative 1 days and positive - * 1 hours". In SQL_STANDARD intervalstyle, we apply the leading sign - * to all fields if there are no other explicit signs. - * - * We leave the signs alone if there are additional explicit signs. - * This protects us against misinterpreting postgres-style dump output, - * since the postgres-style output code has always put an explicit sign on - * all fields following a negative field. But note that SQL-spec output - * is ambiguous and can be misinterpreted on load! (So it's best practice - * to dump in postgres style, not SQL style.) - *---------- - */ - if (IntervalStyle == INTSTYLE_SQL_STANDARD && *field[0] == '-') - { - /* Check for additional explicit signs */ - bool more_signs = false; - - for (i = 1; i < nf; i++) - { - if (*field[i] == '-' || *field[i] == '+') - { - more_signs = true; - break; - } - } - - if (!more_signs) - { - /* - * Rather than re-determining which field was field[0], just force - * 'em all negative. - */ - if (itm_in->tm_usec > 0) - itm_in->tm_usec = -itm_in->tm_usec; - if (itm_in->tm_mday > 0) - itm_in->tm_mday = -itm_in->tm_mday; - if (itm_in->tm_mon > 0) - itm_in->tm_mon = -itm_in->tm_mon; - if (itm_in->tm_year > 0) - itm_in->tm_year = -itm_in->tm_year; - } - } - /* finally, AGO negates everything */ if (is_before) { diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out index 00ffe0e2be..ebcc672d13 100644 --- a/src/test/regress/expected/interval.out +++ b/src/test/regress/expected/interval.out @@ -1690,3 +1690,17 @@ SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us'; P-178956970Y-8M-2147483648DT-2562047788H-54.775808S (1 row) +SET IntervalStyle to postgres; +SELECT INTERVAL '-23 hours 45 min 12.34 sec'; + interval +-------------- + -22:14:47.66 +(1 row) + +SET IntervalStyle to sql_standard; +SELECT INTERVAL '-23 hours 45 min 12.34 sec'; + interval +-------------- + -23:45:12.34 +(1 row) + diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql index fc924d5bde..bf409df6cb 100644 --- a/src/test/regress/sql/interval.sql +++ b/src/test/regress/sql/interval.sql @@ -551,3 +551,8 @@ SET IntervalStyle TO sql_standard; SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us'; SET IntervalStyle to iso_8601; SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us'; + +SET IntervalStyle to postgres; +SELECT INTERVAL '-23 hours 45 min 12.34 sec'; +SET IntervalStyle to sql_standard; +SELECT INTERVAL '-23 hours 45 min 12.34 sec'; -- 2.25.1