Re: making the backend's json parser work in frontend code
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Mahendra Singh Thalor <mahi6run@gmail.com>,
Mark Dilger <mark.dilger@enterprisedb.com>,
Andrew Dunstan <andrew.dunstan@2ndquadrant.com>,
David Steele <david@pgmasters.net>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-01-28T20:42:08Z
Lists: pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Jan 28, 2020 at 2:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Well, yeah, that's exactly my point. But in my book, "refuse to do >> anything" should be "elog(ERROR)", not "invoke undefined behavior". >> An actual abort() call might be all right here, in that at least >> we'd know what would happen and we could debug it once we got hold >> of a stack trace. But pg_unreachable() is not that. Basically, if >> there's *any* circumstance, broken code or not, where control could >> reach a pg_unreachable() call, you did it wrong. > I don't really agree. I think such defensive coding is more than > justified when the input is coming from a file on disk or some other > external source where it might have been corrupted. There's certainly an argument to be made that an elog() call is an unjustified expenditure of code space and we should just do an abort() (but still not pg_unreachable(), IMO). However, what I'm really on about here is that CreateDestReceiver is out of step with nigh-universal project practice. If it's not worth having an elog() here, then there are literally hundreds of other elog() calls that we ought to be nuking on the same grounds. I don't really want to run around and have a bunch of debates about exactly which extant elog() calls are effectively unreachable and which are not. That's not always very clear, and even if it is clear today it might not be tomorrow. The minute somebody calls CreateDestReceiver with a non-constant argument, the issue becomes open again. And I'd rather not have to stop and think hard about the tradeoff between elog() and abort() when I write such functions in future. So basically, my problem with this is that I don't think it's a coding style we want to encourage, because it's too fragile. And there's no good argument (like performance) to leave it that way. I quite agree with you that there are places like tuple deforming where we're taking more chances than I'd like --- but there is a noticeable performance cost to being paranoid there. regards, tom lane
Commits
-
Add jsonapi.c to Mkvcbuild.pm's @pgcommonallfiles.
- 006b9dcad419 13.0 landed
-
Move jsonapi.c and jsonapi.h to src/common.
- beb4699091e9 13.0 landed
-
Move some code from jsonapi.c to jsonfuncs.c.
- 73ce2a03f30b 13.0 landed
-
Adjust pg_parse_json() so that it does not directly ereport().
- 1f3a021730be 13.0 landed
-
Remove jsonapi.c's lex_accept().
- 530609aa4263 13.0 landed
-
Split JSON lexer/parser from 'json' data type support.
- 11b5e3e35d39 13.0 landed
-
Rationalize code placement between wchar.c, encnames.c, and mbutils.c.
- 5afaa2e42655 13.0 landed
-
Move wchar.c and encnames.c to src/common/.
- e6afa8918c46 13.0 landed
-
Update header comments for wchar.c and encnames.c.
- 3d4cb5d6c180 13.0 landed
-
Make StringInfo available to frontend code.
- 26aaf97b683d 13.0 cited
-
Use SASLprep to normalize passwords for SCRAM authentication.
- 60f11b87a234 10.0 cited