Remove inconsistent quotes from date_part error
The attached patch corrects a very minor typographical inconsistency
when date_part is invoked with invalid units on time/timetz data vs
timestamp/timestamptz/interval data.
(If stuff like this is too minor to bother with, let me know and I'll
hold off in the future... but since this was pointed out to me I can't
unsee it.)
Nikhil
Attachments:
0001-Remove-inconsistent-quotes-from-date_part-error.patchapplication/octet-stream; name=0001-Remove-inconsistent-quotes-from-date_part-error.patchDownload
From e9b7d6abd03e2a08f4de5f069a96a3db83351374 Mon Sep 17 00:00:00 2001
From: Nikhil Benesch <nikhil.benesch@gmail.com>
Date: Sun, 2 Jan 2022 21:59:29 -0500
Subject: [PATCH] Remove inconsistent quotes from date_part error
Calling EXTRACT/date_part on a time or timetz value with invalid units
produces an error message that includes quotation marks around "time" or
"timestamp with time zone". These quotation marks are not present in the
error message when date_part is invoked with a timestamp, timestamptz,
or interval value. So this commit removes the quotes from time and
timetz error messages; this seems more in line with how type names are
generally displayed in error messages.
---
src/backend/utils/adt/date.c | 8 ++++----
src/test/regress/expected/time.out | 6 +++---
src/test/regress/expected/timetz.out | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9e8baeaa9c..188795eae4 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -2203,7 +2203,7 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time\" units \"%s\" not recognized",
+ errmsg("time units \"%s\" not recognized",
lowunits)));
intresult = 0;
}
@@ -2219,7 +2219,7 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time\" units \"%s\" not recognized",
+ errmsg("time units \"%s\" not recognized",
lowunits)));
intresult = 0;
}
@@ -2981,7 +2981,7 @@ timetz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time with time zone\" units \"%s\" not recognized",
+ errmsg("time with time zone units \"%s\" not recognized",
lowunits)));
intresult = 0;
}
@@ -3001,7 +3001,7 @@ timetz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time with time zone\" units \"%s\" not recognized",
+ errmsg("time with time zone units \"%s\" not recognized",
lowunits)));
intresult = 0;
}
diff --git a/src/test/regress/expected/time.out b/src/test/regress/expected/time.out
index 39b409feca..e1161ba761 100644
--- a/src/test/regress/expected/time.out
+++ b/src/test/regress/expected/time.out
@@ -161,11 +161,11 @@ SELECT EXTRACT(HOUR FROM TIME '2020-05-26 13:30:25.575401');
(1 row)
SELECT EXTRACT(DAY FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "day" not recognized
+ERROR: time units "day" not recognized
SELECT EXTRACT(FORTNIGHT FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "fortnight" not recognized
+ERROR: time units "fortnight" not recognized
SELECT EXTRACT(TIMEZONE FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "timezone" not recognized
+ERROR: time units "timezone" not recognized
SELECT EXTRACT(EPOCH FROM TIME '2020-05-26 13:30:25.575401');
extract
--------------
diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expected/timetz.out
index f4960c0166..2d0d55546a 100644
--- a/src/test/regress/expected/timetz.out
+++ b/src/test/regress/expected/timetz.out
@@ -178,9 +178,9 @@ SELECT EXTRACT(HOUR FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-
(1 row)
SELECT EXTRACT(DAY FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); -- error
-ERROR: "time with time zone" units "day" not recognized
+ERROR: time with time zone units "day" not recognized
SELECT EXTRACT(FORTNIGHT FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); -- error
-ERROR: "time with time zone" units "fortnight" not recognized
+ERROR: time with time zone units "fortnight" not recognized
SELECT EXTRACT(TIMEZONE FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04:30');
extract
---------
--
2.25.1
On Sun, Jan 02, 2022 at 11:47:32PM -0500, Nikhil Benesch wrote:
The attached patch corrects a very minor typographical inconsistency
when date_part is invoked with invalid units on time/timetz data vs
timestamp/timestamptz/interval data.
Hmm, you are right that this is inconsistent, but I don't think that
what you are doing is completely correct either. First, from what I
can see from the core code, we don't apply quotes to types in error
messages. So your patch is going in the right direction.
However, there is a specific routine called format_type_be() aimed at
formatting type names for error strings. If you switch to that, my
guess is that this makes the error messages of time/timetz and
timestamp/timestamptz/interval more consistent, while reducing the
effort of translation because we'd finish with the same base error
string, as of "%s units \"%s\" not recognized".
If we rework this part, we could even rework this error message more.
One suggestion I have would be "units of type %s not recognized", for
example.
(If stuff like this is too minor to bother with, let me know and I'll
hold off in the future... but since this was pointed out to me I can't
unsee it.)
This usually comes down to a case-by-case analysis. Now, in this
case, your suggestion looks right to me.
--
Michael
On Mon, Jan 3, 2022 at 3:17 AM Michael Paquier <michael@paquier.xyz> wrote:
However, there is a specific routine called format_type_be() aimed at
formatting type names for error strings. If you switch to that, my
guess is that this makes the error messages of time/timetz and
timestamp/timestamptz/interval more consistent, while reducing the
effort of translation because we'd finish with the same base error
string, as of "%s units \"%s\" not recognized".
I could find only a tiny smattering of examples where format_type_be()
is invoked with a constant OID. In almost all error messages where the
type is statically known, it seems the type name is hardcoded into the
error message rather than generated via format_type_be(). For example,
all of the "TYPE out of range" errors.
I'm happy to rework the patch to use format_type_be(), but wanted to
double check first that it is the preferred approach in this
situation.
Nikhil Benesch <nikhil.benesch@gmail.com> writes:
I could find only a tiny smattering of examples where format_type_be()
is invoked with a constant OID. In almost all error messages where the
type is statically known, it seems the type name is hardcoded into the
error message rather than generated via format_type_be(). For example,
all of the "TYPE out of range" errors.
Yeah, but we've been slowly converting to that method to reduce the number
of distinct translatable strings for error messages. If doing so here
would cut the labor for translators, I'm for it.
regards, tom lane
On Mon, Jan 3, 2022 at 10:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yeah, but we've been slowly converting to that method to reduce the number
of distinct translatable strings for error messages. If doing so here
would cut the labor for translators, I'm for it.
Great! I'll update the patch. Thanks for confirming.
Nikhil Benesch <nikhil.benesch@gmail.com> writes:
- errmsg("\"time with time zone\" units \"%s\" not recognized", + errmsg("time with time zone units \"%s\" not recognized", [ etc ]
BTW, if you want to get rid of the quotes, I think that something
else has to be done to set off the type name from the rest. In
this instance someone might think that we're complaining about a
"time zone unit", whatever that is. I suggest swapping it around to
units \"%s\" not recognized for type %s
Also, personally, I'd write unit not units, but that's
more debatable.
regards, tom lane
On Mon, Jan 3, 2022 at 10:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
BTW, if you want to get rid of the quotes, I think that something
else has to be done to set off the type name from the rest. In
this instance someone might think that we're complaining about a
"time zone unit", whatever that is. I suggest swapping it around tounits \"%s\" not recognized for type %s
Also, personally, I'd write unit not units, but that's
more debatable.
Your suggestion sounds good to me. I'll update the patch with that.
Not that it changes anything, I think, but the wording ambiguity you
mention is present today in the timestamptz error message:
benesch=> select extract('nope' from now());
ERROR: timestamp with time zone units "nope" not recognized
Updated patch attached.
Show quoted text
On Mon, Jan 3, 2022 at 10:26 AM Nikhil Benesch <nikhil.benesch@gmail.com> wrote:
On Mon, Jan 3, 2022 at 10:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
BTW, if you want to get rid of the quotes, I think that something
else has to be done to set off the type name from the rest. In
this instance someone might think that we're complaining about a
"time zone unit", whatever that is. I suggest swapping it around tounits \"%s\" not recognized for type %s
Also, personally, I'd write unit not units, but that's
more debatable.Your suggestion sounds good to me. I'll update the patch with that.
Not that it changes anything, I think, but the wording ambiguity you
mention is present today in the timestamptz error message:benesch=> select extract('nope' from now());
ERROR: timestamp with time zone units "nope" not recognized
Attachments:
v2-0001-Improve-unsupported-units-error-for-extract-date_.patchapplication/octet-stream; name=v2-0001-Improve-unsupported-units-error-for-extract-date_.patchDownload
From b9492499b89d9fe002450703b11039b0ac5aff94 Mon Sep 17 00:00:00 2001
From: Nikhil Benesch <nikhil.benesch@gmail.com>
Date: Sun, 2 Jan 2022 21:59:29 -0500
Subject: [PATCH v2] Improve unsupported units error for extract/date_part
Reword the error message when extract/date_part are supplied with
unsupported units from e.g.:
timestamp with time zone units "foo" not supported
to:
unit "foo" not supported for type timestamp with time zone
The new message avoids a grammatical ambiguity around "time zone units"
and consistently avoids quotes around the type name, per standard error
message style. It also injects the type name via format_type_be to
reduce the number of strings which must be translated.
---
src/backend/utils/adt/date.c | 32 ++++++------
src/backend/utils/adt/timestamp.c | 71 +++++++++++++-------------
src/test/regress/expected/date.out | 18 +++----
src/test/regress/expected/interval.out | 4 +-
src/test/regress/expected/time.out | 6 +--
src/test/regress/expected/timetz.out | 4 +-
6 files changed, 69 insertions(+), 66 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9e8baeaa9c..b6decc631e 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -22,6 +22,7 @@
#include <time.h>
#include "access/xact.h"
+#include "catalog/pg_type.h"
#include "common/hashfn.h"
#include "libpq/pqformat.h"
#include "miscadmin.h"
@@ -1124,8 +1125,8 @@ extract_date(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("date units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
}
}
else if (type == UNITS)
@@ -1207,8 +1208,8 @@ extract_date(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("date units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
intresult = 0;
}
}
@@ -1223,8 +1224,8 @@ extract_date(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("date units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
intresult = 0;
}
}
@@ -1232,7 +1233,8 @@ extract_date(PG_FUNCTION_ARGS)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("date units \"%s\" not recognized", lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
intresult = 0;
}
@@ -2203,8 +2205,8 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time\" units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMEOID))));
intresult = 0;
}
}
@@ -2219,8 +2221,8 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time\" units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMEOID))));
intresult = 0;
}
@@ -2981,8 +2983,8 @@ timetz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time with time zone\" units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMETZOID))));
intresult = 0;
}
}
@@ -3001,8 +3003,8 @@ timetz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time with time zone\" units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMETZOID))));
intresult = 0;
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index cb9faff0bb..9d638fceaf 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -3972,8 +3972,8 @@ timestamp_trunc(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
result = 0;
}
@@ -3986,8 +3986,8 @@ timestamp_trunc(PG_FUNCTION_ARGS)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
result = 0;
}
@@ -4182,8 +4182,8 @@ timestamptz_trunc_internal(text *units, TimestampTz timestamp, pg_tz *tzp)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp with time zone units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
result = 0;
}
@@ -4340,14 +4340,14 @@ interval_trunc(PG_FUNCTION_ARGS)
if (val == DTK_WEEK)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("interval units \"%s\" not supported "
+ errmsg("unit \"%s\" not supported for type %s "
"because months usually have fractional weeks",
- lowunits)));
+ lowunits, format_type_be(INTERVALOID))));
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("interval units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(INTERVALOID))));
}
if (tm2interval(tm, fsec, result) != 0)
@@ -4362,8 +4362,8 @@ interval_trunc(PG_FUNCTION_ARGS)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("interval units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(INTERVALOID))));
}
PG_RETURN_INTERVAL_P(result);
@@ -4563,13 +4563,13 @@ NonFiniteTimestampTzPart(int type, int unit, char *lowunits,
if (isTz)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp with time zone units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
}
switch (unit)
@@ -4609,13 +4609,13 @@ NonFiniteTimestampTzPart(int type, int unit, char *lowunits,
if (isTz)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
return 0.0; /* keep compiler quiet */
}
}
@@ -4814,8 +4814,8 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
intresult = 0;
}
}
@@ -4861,8 +4861,8 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
intresult = 0;
}
@@ -4871,7 +4871,8 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp units \"%s\" not recognized", lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
intresult = 0;
}
@@ -5085,8 +5086,8 @@ timestamptz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
intresult = 0;
}
@@ -5133,8 +5134,8 @@ timestamptz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
intresult = 0;
}
}
@@ -5142,8 +5143,8 @@ timestamptz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp with time zone units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
intresult = 0;
}
@@ -5265,8 +5266,8 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("interval units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(INTERVALOID))));
intresult = 0;
}
}
@@ -5326,8 +5327,8 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("interval units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(INTERVALOID))));
intresult = 0;
}
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c8b0566ff4..252cae71e7 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1129,15 +1129,15 @@ SELECT EXTRACT(DECADE FROM DATE '0012-12-31 BC'); -- -2
-- all possible fields
--
SELECT EXTRACT(MICROSECONDS FROM DATE '2020-08-11');
-ERROR: date units "microseconds" not supported
+ERROR: unit "microseconds" not supported for type date
SELECT EXTRACT(MILLISECONDS FROM DATE '2020-08-11');
-ERROR: date units "milliseconds" not supported
+ERROR: unit "milliseconds" not supported for type date
SELECT EXTRACT(SECOND FROM DATE '2020-08-11');
-ERROR: date units "second" not supported
+ERROR: unit "second" not supported for type date
SELECT EXTRACT(MINUTE FROM DATE '2020-08-11');
-ERROR: date units "minute" not supported
+ERROR: unit "minute" not supported for type date
SELECT EXTRACT(HOUR FROM DATE '2020-08-11');
-ERROR: date units "hour" not supported
+ERROR: unit "hour" not supported for type date
SELECT EXTRACT(DAY FROM DATE '2020-08-11');
extract
---------
@@ -1235,11 +1235,11 @@ SELECT EXTRACT(DOY FROM DATE '2020-08-11');
(1 row)
SELECT EXTRACT(TIMEZONE FROM DATE '2020-08-11');
-ERROR: date units "timezone" not supported
+ERROR: unit "timezone" not supported for type date
SELECT EXTRACT(TIMEZONE_M FROM DATE '2020-08-11');
-ERROR: date units "timezone_m" not supported
+ERROR: unit "timezone_m" not supported for type date
SELECT EXTRACT(TIMEZONE_H FROM DATE '2020-08-11');
-ERROR: date units "timezone_h" not supported
+ERROR: unit "timezone_h" not supported for type date
SELECT EXTRACT(EPOCH FROM DATE '2020-08-11');
extract
------------
@@ -1462,7 +1462,7 @@ SELECT EXTRACT(EPOCH FROM DATE 'infinity'); -- Infinity
-- wrong fields from non-finite date:
--
SELECT EXTRACT(MICROSEC FROM DATE 'infinity'); -- error
-ERROR: date units "microsec" not recognized
+ERROR: unit "microsec" not supported for type date
-- test constructors
select make_date(2013, 7, 15);
make_date
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index e4b1246f45..be797cb219 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -963,9 +963,9 @@ SELECT f1,
(10 rows)
SELECT EXTRACT(FORTNIGHT FROM INTERVAL '2 days'); -- error
-ERROR: interval units "fortnight" not recognized
+ERROR: unit "fortnight" not supported for type interval
SELECT EXTRACT(TIMEZONE FROM INTERVAL '2 days'); -- error
-ERROR: interval units "timezone" not supported
+ERROR: unit "timezone" not supported for type interval
SELECT EXTRACT(DECADE FROM INTERVAL '100 y');
extract
---------
diff --git a/src/test/regress/expected/time.out b/src/test/regress/expected/time.out
index 39b409feca..f55cdd5ac0 100644
--- a/src/test/regress/expected/time.out
+++ b/src/test/regress/expected/time.out
@@ -161,11 +161,11 @@ SELECT EXTRACT(HOUR FROM TIME '2020-05-26 13:30:25.575401');
(1 row)
SELECT EXTRACT(DAY FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "day" not recognized
+ERROR: unit "day" not supported for type time without time zone
SELECT EXTRACT(FORTNIGHT FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "fortnight" not recognized
+ERROR: unit "fortnight" not supported for type time without time zone
SELECT EXTRACT(TIMEZONE FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "timezone" not recognized
+ERROR: unit "timezone" not supported for type time without time zone
SELECT EXTRACT(EPOCH FROM TIME '2020-05-26 13:30:25.575401');
extract
--------------
diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expected/timetz.out
index f4960c0166..3990b4356a 100644
--- a/src/test/regress/expected/timetz.out
+++ b/src/test/regress/expected/timetz.out
@@ -178,9 +178,9 @@ SELECT EXTRACT(HOUR FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-
(1 row)
SELECT EXTRACT(DAY FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); -- error
-ERROR: "time with time zone" units "day" not recognized
+ERROR: unit "day" not supported for type time with time zone
SELECT EXTRACT(FORTNIGHT FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); -- error
-ERROR: "time with time zone" units "fortnight" not recognized
+ERROR: unit "fortnight" not supported for type time with time zone
SELECT EXTRACT(TIMEZONE FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04:30');
extract
---------
--
2.25.1
Nikhil Benesch <nikhil.benesch@gmail.com> writes:
Updated patch attached.
Hmm, I think you went a bit too far here. The existing code intends
to draw a distinction between "not recognized" (i.e., "we don't know
what that word was you used") and "not supported" (i.e., "we know
that word, but it doesn't seem to make sense in context, or we
haven't got round to the case yet"). You've mashed those into the
same error text, which I don't think we should do, especially
since we're using distinct ERRCODE values for them.
Attached v3 restores that distinction, and makes some other small
tweaks. (I found that there were actually a couple of spots in
date.c that got it backwards, so admittedly this is a fine point
that not everybody is on board with. But let's make it consistent
now.)
regards, tom lane
Attachments:
v3-0001-Improve-unsupported-units-errors.patchtext/x-diff; charset=us-ascii; name=v3-0001-Improve-unsupported-units-errors.patchDownload
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9e8baeaa9c..629166632d 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -22,6 +22,7 @@
#include <time.h>
#include "access/xact.h"
+#include "catalog/pg_type.h"
#include "common/hashfn.h"
#include "libpq/pqformat.h"
#include "miscadmin.h"
@@ -1124,8 +1125,8 @@ extract_date(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("date units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
}
}
else if (type == UNITS)
@@ -1207,8 +1208,8 @@ extract_date(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("date units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
intresult = 0;
}
}
@@ -1223,8 +1224,8 @@ extract_date(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("date units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(DATEOID))));
intresult = 0;
}
}
@@ -1232,7 +1233,8 @@ extract_date(PG_FUNCTION_ARGS)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("date units \"%s\" not recognized", lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(DATEOID))));
intresult = 0;
}
@@ -2202,9 +2204,9 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric)
case DTK_ISOYEAR:
default:
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time\" units \"%s\" not recognized",
- lowunits)));
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMEOID))));
intresult = 0;
}
}
@@ -2219,8 +2221,8 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time\" units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(TIMEOID))));
intresult = 0;
}
@@ -2980,9 +2982,9 @@ timetz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
case DTK_MILLENNIUM:
default:
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time with time zone\" units \"%s\" not recognized",
- lowunits)));
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMETZOID))));
intresult = 0;
}
}
@@ -3001,8 +3003,8 @@ timetz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"time with time zone\" units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(TIMETZOID))));
intresult = 0;
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index cb9faff0bb..b66d69db00 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -3972,8 +3972,8 @@ timestamp_trunc(PG_FUNCTION_ARGS)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
result = 0;
}
@@ -3986,8 +3986,8 @@ timestamp_trunc(PG_FUNCTION_ARGS)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
result = 0;
}
@@ -4165,8 +4165,8 @@ timestamptz_trunc_internal(text *units, TimestampTz timestamp, pg_tz *tzp)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not "
- "supported", lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
result = 0;
}
@@ -4182,8 +4182,8 @@ timestamptz_trunc_internal(text *units, TimestampTz timestamp, pg_tz *tzp)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp with time zone units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
result = 0;
}
@@ -4337,17 +4337,11 @@ interval_trunc(PG_FUNCTION_ARGS)
break;
default:
- if (val == DTK_WEEK)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("interval units \"%s\" not supported "
- "because months usually have fractional weeks",
- lowunits)));
- else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("interval units \"%s\" not supported",
- lowunits)));
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(INTERVALOID)),
+ (val == DTK_WEEK) ? errdetail("Months usually have fractional weeks.") : 0));
}
if (tm2interval(tm, fsec, result) != 0)
@@ -4362,8 +4356,8 @@ interval_trunc(PG_FUNCTION_ARGS)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("interval units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(INTERVALOID))));
}
PG_RETURN_INTERVAL_P(result);
@@ -4559,18 +4553,11 @@ NonFiniteTimestampTzPart(int type, int unit, char *lowunits,
bool isNegative, bool isTz)
{
if ((type != UNITS) && (type != RESERV))
- {
- if (isTz)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp with time zone units \"%s\" not recognized",
- lowunits)));
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp units \"%s\" not recognized",
- lowunits)));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits,
+ format_type_be(isTz ? TIMESTAMPTZOID : TIMESTAMPOID))));
switch (unit)
{
@@ -4606,16 +4593,11 @@ NonFiniteTimestampTzPart(int type, int unit, char *lowunits,
return get_float8_infinity();
default:
- if (isTz)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not supported",
- lowunits)));
- else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits,
+ format_type_be(isTz ? TIMESTAMPTZOID : TIMESTAMPOID))));
return 0.0; /* keep compiler quiet */
}
}
@@ -4814,8 +4796,8 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
intresult = 0;
}
}
@@ -4861,8 +4843,8 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
intresult = 0;
}
@@ -4871,7 +4853,8 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp units \"%s\" not recognized", lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(TIMESTAMPOID))));
intresult = 0;
}
@@ -5085,8 +5068,8 @@ timestamptz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
intresult = 0;
}
@@ -5133,8 +5116,8 @@ timestamptz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("timestamp with time zone units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
intresult = 0;
}
}
@@ -5142,8 +5125,8 @@ timestamptz_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("timestamp with time zone units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(TIMESTAMPTZOID))));
intresult = 0;
}
@@ -5265,8 +5248,8 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("interval units \"%s\" not supported",
- lowunits)));
+ errmsg("unit \"%s\" not supported for type %s",
+ lowunits, format_type_be(INTERVALOID))));
intresult = 0;
}
}
@@ -5326,8 +5309,8 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("interval units \"%s\" not recognized",
- lowunits)));
+ errmsg("unit \"%s\" not recognized for type %s",
+ lowunits, format_type_be(INTERVALOID))));
intresult = 0;
}
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c8b0566ff4..75ff659378 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1129,15 +1129,15 @@ SELECT EXTRACT(DECADE FROM DATE '0012-12-31 BC'); -- -2
-- all possible fields
--
SELECT EXTRACT(MICROSECONDS FROM DATE '2020-08-11');
-ERROR: date units "microseconds" not supported
+ERROR: unit "microseconds" not supported for type date
SELECT EXTRACT(MILLISECONDS FROM DATE '2020-08-11');
-ERROR: date units "milliseconds" not supported
+ERROR: unit "milliseconds" not supported for type date
SELECT EXTRACT(SECOND FROM DATE '2020-08-11');
-ERROR: date units "second" not supported
+ERROR: unit "second" not supported for type date
SELECT EXTRACT(MINUTE FROM DATE '2020-08-11');
-ERROR: date units "minute" not supported
+ERROR: unit "minute" not supported for type date
SELECT EXTRACT(HOUR FROM DATE '2020-08-11');
-ERROR: date units "hour" not supported
+ERROR: unit "hour" not supported for type date
SELECT EXTRACT(DAY FROM DATE '2020-08-11');
extract
---------
@@ -1235,11 +1235,11 @@ SELECT EXTRACT(DOY FROM DATE '2020-08-11');
(1 row)
SELECT EXTRACT(TIMEZONE FROM DATE '2020-08-11');
-ERROR: date units "timezone" not supported
+ERROR: unit "timezone" not supported for type date
SELECT EXTRACT(TIMEZONE_M FROM DATE '2020-08-11');
-ERROR: date units "timezone_m" not supported
+ERROR: unit "timezone_m" not supported for type date
SELECT EXTRACT(TIMEZONE_H FROM DATE '2020-08-11');
-ERROR: date units "timezone_h" not supported
+ERROR: unit "timezone_h" not supported for type date
SELECT EXTRACT(EPOCH FROM DATE '2020-08-11');
extract
------------
@@ -1462,7 +1462,7 @@ SELECT EXTRACT(EPOCH FROM DATE 'infinity'); -- Infinity
-- wrong fields from non-finite date:
--
SELECT EXTRACT(MICROSEC FROM DATE 'infinity'); -- error
-ERROR: date units "microsec" not recognized
+ERROR: unit "microsec" not recognized for type date
-- test constructors
select make_date(2013, 7, 15);
make_date
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index e4b1246f45..accd4a7d90 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -963,9 +963,9 @@ SELECT f1,
(10 rows)
SELECT EXTRACT(FORTNIGHT FROM INTERVAL '2 days'); -- error
-ERROR: interval units "fortnight" not recognized
+ERROR: unit "fortnight" not recognized for type interval
SELECT EXTRACT(TIMEZONE FROM INTERVAL '2 days'); -- error
-ERROR: interval units "timezone" not supported
+ERROR: unit "timezone" not supported for type interval
SELECT EXTRACT(DECADE FROM INTERVAL '100 y');
extract
---------
diff --git a/src/test/regress/expected/time.out b/src/test/regress/expected/time.out
index 39b409feca..f3a71503c2 100644
--- a/src/test/regress/expected/time.out
+++ b/src/test/regress/expected/time.out
@@ -161,11 +161,11 @@ SELECT EXTRACT(HOUR FROM TIME '2020-05-26 13:30:25.575401');
(1 row)
SELECT EXTRACT(DAY FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "day" not recognized
+ERROR: unit "day" not supported for type time without time zone
SELECT EXTRACT(FORTNIGHT FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "fortnight" not recognized
+ERROR: unit "fortnight" not recognized for type time without time zone
SELECT EXTRACT(TIMEZONE FROM TIME '2020-05-26 13:30:25.575401'); -- error
-ERROR: "time" units "timezone" not recognized
+ERROR: unit "timezone" not supported for type time without time zone
SELECT EXTRACT(EPOCH FROM TIME '2020-05-26 13:30:25.575401');
extract
--------------
diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expected/timetz.out
index f4960c0166..8942a9b95b 100644
--- a/src/test/regress/expected/timetz.out
+++ b/src/test/regress/expected/timetz.out
@@ -178,9 +178,9 @@ SELECT EXTRACT(HOUR FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-
(1 row)
SELECT EXTRACT(DAY FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); -- error
-ERROR: "time with time zone" units "day" not recognized
+ERROR: unit "day" not supported for type time with time zone
SELECT EXTRACT(FORTNIGHT FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); -- error
-ERROR: "time with time zone" units "fortnight" not recognized
+ERROR: unit "fortnight" not recognized for type time with time zone
SELECT EXTRACT(TIMEZONE FROM TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04:30');
extract
---------
On 2022-Jan-03, Tom Lane wrote:
Attached v3 restores that distinction, and makes some other small
tweaks. (I found that there were actually a couple of spots in
date.c that got it backwards, so admittedly this is a fine point
that not everybody is on board with. But let's make it consistent
now.)
LGTM.
@@ -2202,9 +2204,9 @@ time_part_common(PG_FUNCTION_ARGS, bool retnumeric) case DTK_ISOYEAR: default: ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("\"time\" units \"%s\" not recognized", - lowunits))); + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("unit \"%s\" not supported for type %s", + lowunits, format_type_be(TIMEOID))));
I agree that these changes are an improvement.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On Mon, Jan 3, 2022 at 1:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hmm, I think you went a bit too far here. The existing code intends
to draw a distinction between "not recognized" (i.e., "we don't know
what that word was you used") and "not supported" (i.e., "we know
that word, but it doesn't seem to make sense in context, or we
haven't got round to the case yet"). You've mashed those into the
same error text, which I don't think we should do, especially
since we're using distinct ERRCODE values for them.
Oops. I noticed that "inconsistency" between
ERRCODE_FEATURE_NOT_SUPPORTED and ERRCODE_INVALID_PARAMETER_VALUE and
then promptly blazed past it. Thanks for catching that.
Attached v3 restores that distinction, and makes some other small
tweaks. (I found that there were actually a couple of spots in
date.c that got it backwards, so admittedly this is a fine point
that not everybody is on board with. But let's make it consistent
now.)
LGTM too, for whatever that's worth.
Nikhil Benesch <nikhil.benesch@gmail.com> writes:
On Mon, Jan 3, 2022 at 1:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Attached v3 restores that distinction, and makes some other small
tweaks. (I found that there were actually a couple of spots in
date.c that got it backwards, so admittedly this is a fine point
that not everybody is on board with. But let's make it consistent
now.)
LGTM too, for whatever that's worth.
OK, pushed.
regards, tom lane