public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/3] bootstrap: convert Typ to a List*
5+ messages / 3 participants
[nested] [flat]
* [PATCH 1/3] bootstrap: convert Typ to a List*
@ 2020-11-20 02:48 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw)
---
src/backend/bootstrap/bootstrap.c | 69 ++++++++++++++-----------------
1 file changed, 31 insertions(+), 38 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6f615e6622..18eb62ca47 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -159,7 +159,7 @@ struct typmap
FormData_pg_type am_typ;
};
-static struct typmap **Typ = NULL;
+static List *Typ = NIL; /* List of struct typmap* */
static struct typmap *Ap = NULL;
static Datum values[MAXATTR]; /* current row's attribute values */
@@ -597,7 +597,7 @@ boot_openrel(char *relname)
* pg_type must be filled before any OPEN command is executed, hence we
* can now populate the Typ array if we haven't yet.
*/
- if (Typ == NULL)
+ if (Typ == NIL)
populate_typ_array();
if (boot_reldesc != NULL)
@@ -688,7 +688,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
typeoid = gettype(type);
- if (Typ != NULL)
+ if (Typ != NIL)
{
attrtypes[attnum]->atttypid = Ap->am_oid;
attrtypes[attnum]->attlen = Ap->am_typ.typlen;
@@ -877,36 +877,25 @@ populate_typ_array(void)
Relation rel;
TableScanDesc scan;
HeapTuple tup;
- int nalloc;
- int i;
-
- Assert(Typ == NULL);
- nalloc = 512;
- Typ = (struct typmap **)
- MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+ Assert(Typ == NIL);
rel = table_open(TypeRelationId, NoLock);
scan = table_beginscan_catalog(rel, 0, NULL);
- i = 0;
while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+ struct typmap *newtyp;
+ MemoryContext old;
- /* make sure there will be room for a trailing NULL pointer */
- if (i >= nalloc - 1)
- {
- nalloc *= 2;
- Typ = (struct typmap **)
- repalloc(Typ, nalloc * sizeof(struct typmap *));
- }
- Typ[i] = (struct typmap *)
- MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
- Typ[i]->am_oid = typForm->oid;
- memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
- i++;
+ old = MemoryContextSwitchTo(TopMemoryContext);
+ newtyp = (struct typmap *) palloc(sizeof(struct typmap));
+ Typ = lappend(Typ, newtyp);
+ MemoryContextSwitchTo(old);
+
+ newtyp->am_oid = typForm->oid;
+ memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ));
}
- Typ[i] = NULL; /* Fill trailing NULL pointer */
table_endscan(scan);
table_close(rel, NoLock);
}
@@ -925,16 +914,17 @@ populate_typ_array(void)
static Oid
gettype(char *type)
{
- if (Typ != NULL)
+ if (Typ != NIL)
{
- struct typmap **app;
+ ListCell *lc;
- for (app = Typ; *app != NULL; app++)
+ foreach (lc, Typ)
{
- if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
+ struct typmap *app = lfirst(lc);
+ if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
{
- Ap = *app;
- return (*app)->am_oid;
+ Ap = app;
+ return app->am_oid;
}
}
}
@@ -980,14 +970,17 @@ boot_get_type_io_data(Oid typid,
if (Typ != NULL)
{
/* We have the boot-time contents of pg_type, so use it */
- struct typmap **app;
- struct typmap *ap;
-
- app = Typ;
- while (*app && (*app)->am_oid != typid)
- ++app;
- ap = *app;
- if (ap == NULL)
+ struct typmap *ap = NULL;
+ ListCell *lc;
+
+ foreach (lc, Typ)
+ {
+ ap = lfirst(lc);
+ if (ap->am_oid == typid)
+ break;
+ }
+
+ if (!ap || ap->am_oid != typid)
elog(ERROR, "type OID %u not found in Typ list", typid);
*typlen = ap->am_typ.typlen;
--
2.26.2
--------------EED78D4A67CFF327F6B880D2
Content-Type: text/x-patch; charset=UTF-8;
name="0002-Allow-composite-types-in-bootstrap-20210122b.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0002-Allow-composite-types-in-bootstrap-20210122b.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* Inconsistent Parsing of Offsets with Seconds
@ 2024-06-22 16:25 David E. Wheeler <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: David E. Wheeler @ 2024-06-22 16:25 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hackers,
The treatment of timestamptz (and timetz) values with offsets that include seconds seems a bit inconsistent. One can create such timestamps through the input function:
david=# select '2024-06-22T12:35:00+02:30:15'::timestamptz;
timestamptz
------------------------
2024-06-22 10:04:45+00
But the offset seconds are dropped (or rounded away?) by to_timestamp()’s `OF` and `TZ` formats[2]:
david=# select to_timestamp('2024-06-03 12:35:00+02:30:15', 'YYYY-MM-DD HH24:MI:SSOF');
to_timestamp
------------------------
2024-06-03 10:05:00+00
david=# select to_timestamp('2024-06-03 12:35:00+02:30:15', 'YYYY-MM-DD HH24:MI:SSTZ');
to_timestamp
------------------------
2024-06-03 02:05:00-08
The corresponding jsonpath methods don’t like offsets with seconds *at all*:
david=# select jsonb_path_query('"2024-06-03 12:35:00+02:30:15"', '$.datetime("YYYY-MM-DD HH24:MI:SSOF")');
ERROR: trailing characters remain in input string after datetime format
david=# select jsonb_path_query('"2024-06-03 12:35:00+02:30:15"', '$.timestamp_tz()');
ERROR: timestamp_tz format is not recognized: "2024-06-03 12:35:00+02:30:15"
I see from the source[1] that offsets between plus or minus 15:59:59 are allowed; should the `OF` and `TZ formats be able to parse them? Or perhaps there should be a `TZS` format to complement `TZH` and `TZM`?
Best,
David
[1] https://github.com/postgres/postgres/blob/70a845c/src/include/datatype/timestamp.h#L136-L142
[2]: https://www.postgresql.org/docs/16/functions-formatting.html
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Inconsistent Parsing of Offsets with Seconds
@ 2024-06-22 17:15 Tom Lane <[email protected]>
parent: David E. Wheeler <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2024-06-22 17:15 UTC (permalink / raw)
To: David E. Wheeler <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
"David E. Wheeler" <[email protected]> writes:
> The treatment of timestamptz (and timetz) values with offsets that include seconds seems a bit inconsistent.
It's hard to get excited about this. Per the IANA TZ data,
nowhere in the world has used fractional-minute UT offsets
since 1972:
# In 1972 Liberia was the last country to switch from a UT offset
# that was not a multiple of 15 or 20 minutes.
and they were twenty years later than the next-to-last place (although
IANA will steadfastly deny reliability for their TZ data before 1970).
So timestamps like this simply don't exist in the wild.
> The corresponding jsonpath methods don’t like offsets with seconds *at all*:
Perhaps that should be fixed, but it's pretty low-priority IMO.
I doubt there is any standard saying that JSON timestamps need
to be able to include that.
> I see from the source[1] that offsets between plus or minus 15:59:59
> are allowed; should the `OF` and `TZ formats be able to parse them?
I'd vote no. to_date/to_char already have enough trouble with format
strings being squishier than one might expect.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Inconsistent Parsing of Offsets with Seconds
@ 2024-06-22 18:10 David E. Wheeler <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: David E. Wheeler @ 2024-06-22 18:10 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Jun 22, 2024, at 13:15, Tom Lane <[email protected]> wrote:
> It's hard to get excited about this.
I freely admit I’m getting into the weeds here. :-)
>> The corresponding jsonpath methods don’t like offsets with seconds *at all*:
>
> Perhaps that should be fixed, but it's pretty low-priority IMO.
> I doubt there is any standard saying that JSON timestamps need
> to be able to include that.
>
>> I see from the source[1] that offsets between plus or minus 15:59:59
>> are allowed; should the `OF` and `TZ formats be able to parse them?
>
> I'd vote no. to_date/to_char already have enough trouble with format
> strings being squishier than one might expect.
I believe the former issue is caused by the latter: The jsonpath implementation uses the formatting strings to parse the timestamps[1], and since there is no formatting to support offsets with seconds, it doesn’t work at all in JSON timestamp parsing.
[1]: https://github.com/postgres/postgres/blob/70a845c/src/backend/utils/adt/jsonpath_exec.c#L2420-L2442
So if we were to fix the parsing of offsets in jsonpath, we’d either have to change the parsing code there or augment the to_timestamp() formats and use them.
Totally agree not a priority; happy to just pretend offsets with seconds don’t exist in any practical sense.
Best,
David
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Inconsistent Parsing of Offsets with Seconds
@ 2024-06-24 12:08 David E. Wheeler <[email protected]>
parent: David E. Wheeler <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: David E. Wheeler @ 2024-06-24 12:08 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Jun 22, 2024, at 14:10, David E. Wheeler <[email protected]> wrote:
> I believe the former issue is caused by the latter: The jsonpath implementation uses the formatting strings to parse the timestamps[1], and since there is no formatting to support offsets with seconds, it doesn’t work at all in JSON timestamp parsing.
>
> [1]: https://github.com/postgres/postgres/blob/70a845c/src/backend/utils/adt/jsonpath_exec.c#L2420-L2442
A side-effect of this implementation of date/time parsing using the to_char templates is that only time zone offsets and abbreviations are supported. I find the behavior a little surprising TBH:
david=# select to_timestamp('2024-06-03 12:35:00America/New_York', 'YYYY-MM-DD HH24:MI:SSTZ');
ERROR: invalid value "America/New_York" for "TZ"
DETAIL: Time zone abbreviation is not recognized.
Unless the SQL standard only supports offsets and abbreviations, I wonder if we’d be better off updating the above parsing code to also try the various date/time input functions, as well as the custom formats that *are* defined by the standard.
Best,
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-06-24 12:08 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 02:48 [PATCH 1/3] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]>
2024-06-22 16:25 Inconsistent Parsing of Offsets with Seconds David E. Wheeler <[email protected]>
2024-06-22 17:15 ` Re: Inconsistent Parsing of Offsets with Seconds Tom Lane <[email protected]>
2024-06-22 18:10 ` Re: Inconsistent Parsing of Offsets with Seconds David E. Wheeler <[email protected]>
2024-06-24 12:08 ` Re: Inconsistent Parsing of Offsets with Seconds David E. Wheeler <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox