Re: SQL/JSON features for v15

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Amit Langote <amitlangote09(at)gmail(dot)com>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>
Subject: Re: SQL/JSON features for v15
Date: 2022-08-31 17:06:32
Message-ID: 13351.1661965592@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> From my POV the only real discussion is whether we'd want to revert this in 15
>> and HEAD or just 15. There's imo a decent point to be made to just revert in
>> 15 and aggressively press forward with the changes posted in this thread.

> I'm not for that. Code that we don't think is ready to ship
> has no business being in the common tree, nor does it make
> review any easier to be looking at one bulky set of
> already-committed patches and another bulky set of deltas.

To enlarge on that a bit: it seems to me that the really fundamental
issue here is how to catch datatype-specific input and conversion
errors without using subtransactions, because those are too expensive
and can mask errors we'd rather not be masking, such as OOM. (Andres
had some additional, more localized concerns, but I think this is the
one with big-picture implications.)

The currently proposed patchset hacks up a relatively small number
of core datatypes to be able to do that. But it's just a hack
and there's no prospect of extension types being able to join
in the fun. I think where we need to start, for v16, is making
an API design that will let any datatype have this functionality.
(I don't say that we'd convert every datatype to do so right away;
in the long run we should, but I'm content to start with just the
same core types touched here.) Beside the JSON stuff, there is
another even more pressing application for such behavior, namely
the often-requested COPY functionality to be able to shunt bad data
off somewhere without losing the entire transfer. In the COPY case
I think we'd want to be able to capture the error message that
would have been issued, which means the current patches are not
at all appropriate as a basis for that API design: they're just
returning a bool without any details.

So that's why I'm in favor of reverting and starting over.
There are probably big chunks of what's been done that can be
re-used, but it all needs to be re-examined with this sort of
design in mind.

As a really quick sketch of what such an API might look like:
we could invent a new node type, say IOCallContext, which is
intended to be passed as FunctionCallInfo.context to type
input functions and perhaps type conversion functions.
Call sites wishing to have no-thrown-error functionality would
initialize one of these to show "no error" and then pass it
to the data type's usual input function. Old-style input
functions would ignore this and just throw errors as usual;
sorry, you don't get the no-error functionality you wanted.
But I/O functions that had been updated would know to store the
report of a relevant error into that node and then return NULL.
(Although I think there may be assumptions somewhere that
I/O functions don't return NULL, so maybe "just return any
dummy value" is a better idea? Although likely it wouldn't
be hard to remove such assumptions from callers using this
functionality.) The caller would detect the presence of an error
by examining the node contents and then do whatever it needs to do.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-08-31 17:09:51 Re: SQL/JSON features for v15
Previous Message Justin Pryzby 2022-08-31 17:05:55 Re: Add tracking of backend memory allocated to pg_stat_activity