On Sat, Jul 8, 2023 at 5:06 PM Gurjeet Singh <gurj...@singh.im> wrote:
> I feel the staleness/deficiencies you mention above are not
> captured in the TODO wiki page. It'd be nice if these were documented,
> so that newcomers to the community can pick up work that they feel is
> an easy lift for them.
I think that's a good idea. I've definitely been confused by this in
previous patches I've submitted.
I've broken up this patch into three logical commits and attached them.
None of the actual code has changed.
Thanks,
Joe Koshakow
From b3fe06934b40489d1b4b157677f1292bc698c7da Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sun, 9 Jul 2023 13:12:16 -0400
Subject: [PATCH 1/3] Remove dead code in DecodeInterval
This commit removes dead code for handling unit type RESERVE. There
used to be a unit called "invalid" that was of type RESERVE. At some
point that unit was removed and there were no more units of type
RESERVE. Therefore, the code for RESERVE unit handling is unreachable.
---
src/backend/utils/adt/datetime.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 5d8d583ddc..2a5dddc43f 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3582,11 +3582,6 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
type = uval;
break;
- case RESERV:
- tmask = (DTK_DATE_M | DTK_TIME_M);
- *dtype = uval;
- break;
-
default:
return DTERR_BAD_FORMAT;
}
--
2.34.1
From 6271c5fcca30de0982b4b6073b49c1cea6c7391b Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sun, 9 Jul 2023 13:17:08 -0400
Subject: [PATCH 2/3] Fix Interval 'ago' parsing
This commit Restrict the unit "ago" to only appear at the end of the
interval. According to the docs [0], this is the only valid place to
put it, but we allowed it multiple times at any point in the input.
[0] https://www.postgresql.org/docs/15/datatype-datetime.html#DATATYPE-INTERVAL-INPUT
---
src/backend/utils/adt/datetime.c | 6 ++++++
src/test/regress/expected/interval.out | 9 +++++++++
src/test/regress/sql/interval.sql | 4 ++++
3 files changed, 19 insertions(+)
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 2a5dddc43f..9d09381328 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3578,6 +3578,12 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
break;
case AGO:
+ /*
+ * 'ago' is only allowed to appear at the end of the
+ * interval.
+ */
+ if (i != nf - 1)
+ return DTERR_BAD_FORMAT;
is_before = true;
type = uval;
break;
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 28b71d9681..42062f947f 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -1787,3 +1787,12 @@ SELECT extract(epoch from interval '1000000000 days');
86400000000000.000000
(1 row)
+-- test that ago can only appear once at the end of the interval.
+SELECT INTERVAL '42 days 2 seconds ago ago';
+ERROR: invalid input syntax for type interval: "42 days 2 seconds ago ago"
+LINE 1: SELECT INTERVAL '42 days 2 seconds ago ago';
+ ^
+SELECT INTERVAL '2 minutes ago 5 days';
+ERROR: invalid input syntax for type interval: "2 minutes ago 5 days"
+LINE 1: SELECT INTERVAL '2 minutes ago 5 days';
+ ^
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 56feda1a3d..8fd2e7f41e 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -582,3 +582,7 @@ SELECT f1,
-- internal overflow test case
SELECT extract(epoch from interval '1000000000 days');
+
+-- test that ago can only appear once at the end of the interval.
+SELECT INTERVAL '42 days 2 seconds ago ago';
+SELECT INTERVAL '2 minutes ago 5 days';
--
2.34.1
From 2ffb81e95031b43955fdba784356fc54659775e2 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sun, 9 Jul 2023 13:21:23 -0400
Subject: [PATCH 3/3] Fix Interval unit parsing
This commit will error when the user has multiple consecutive units or
a unit without an accompanying value.
---
src/backend/utils/adt/datetime.c | 12 ++++++++++++
src/test/regress/expected/interval.out | 9 +++++++++
src/test/regress/sql/interval.sql | 4 ++++
3 files changed, 25 insertions(+)
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 9d09381328..edf22f458e 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3278,6 +3278,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
{
bool force_negative = false;
bool is_before = false;
+ bool parsing_unit_val = false;
char *cp;
int fmask = 0,
tmask,
@@ -3336,6 +3337,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
itm_in->tm_usec > 0)
itm_in->tm_usec = -itm_in->tm_usec;
type = DTK_DAY;
+ parsing_unit_val = false;
break;
case DTK_TZ:
@@ -3373,6 +3375,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
* are reading right to left.
*/
type = DTK_DAY;
+ parsing_unit_val = false;
break;
}
@@ -3562,10 +3565,14 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
default:
return DTERR_BAD_FORMAT;
}
+ parsing_unit_val = false;
break;
case DTK_STRING:
case DTK_SPECIAL:
+ /* reject consecutive unhandled units */
+ if (parsing_unit_val)
+ return DTERR_BAD_FORMAT;
type = DecodeUnits(i, field[i], &uval);
if (type == IGNORE_DTF)
continue;
@@ -3575,6 +3582,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
{
case UNITS:
type = uval;
+ parsing_unit_val = true;
break;
case AGO:
@@ -3606,6 +3614,10 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
if (fmask == 0)
return DTERR_BAD_FORMAT;
+ /* reject if unit appeared and was never handled */
+ if (parsing_unit_val)
+ return DTERR_BAD_FORMAT;
+
/* finally, AGO negates everything */
if (is_before)
{
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 42062f947f..7aba799351 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -1796,3 +1796,12 @@ SELECT INTERVAL '2 minutes ago 5 days';
ERROR: invalid input syntax for type interval: "2 minutes ago 5 days"
LINE 1: SELECT INTERVAL '2 minutes ago 5 days';
^
+-- test that consecutive and dangling units are not allowed.
+SELECT INTERVAL 'hour 5 months';
+ERROR: invalid input syntax for type interval: "hour 5 months"
+LINE 1: SELECT INTERVAL 'hour 5 months';
+ ^
+SELECT INTERVAL '1 year months days 5 hours';
+ERROR: invalid input syntax for type interval: "1 year months days 5 hours"
+LINE 1: SELECT INTERVAL '1 year months days 5 hours';
+ ^
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 8fd2e7f41e..7e1cb92918 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -586,3 +586,7 @@ SELECT extract(epoch from interval '1000000000 days');
-- test that ago can only appear once at the end of the interval.
SELECT INTERVAL '42 days 2 seconds ago ago';
SELECT INTERVAL '2 minutes ago 5 days';
+
+-- test that consecutive and dangling units are not allowed.
+SELECT INTERVAL 'hour 5 months';
+SELECT INTERVAL '1 year months days 5 hours';
--
2.34.1
