Обсуждение: Rationalizing code-sharing among src/bin/ directories
I'm not terribly happy with the hack I used to get pgbench to be able to borrow psql's psqlscan.l lexer. It's a mess for the MSVC build scripts, and I have seen it causing two concurrent builds of psqlscan.o in parallel builds, which is likely to cause a build failure some of the time. Another unresolved issue is that we can't apply FLEX_NO_BACKUP testing to both of psql's lexers, because that causes each of those make steps to want to write lex.backup in the current directory. I have a modest proposal for improving this: let's move all the code that's currently shared by two or more src/bin/ subdirectories into a new directory, say src/feutils, and build it into a ".a" library in the same way that src/common/ is handled. This would remove the following klugy cross-directory links: src/bin/pgbench/psqlscan.o -> ../../../src/bin/psql/psqlscan.o src/bin/psql/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c src/bin/psql/keywords.c -> ../../../src/bin/pg_dump/keywords.c src/bin/scripts/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c src/bin/scripts/keywords.c -> ../../../src/bin/pg_dump/keywords.c src/bin/scripts/mbprint.c -> ../../../src/bin/psql/mbprint.c src/bin/scripts/print.c -> ../../../src/bin/psql/print.c Note: the reason for a new subdirectory, rather than putting this stuff into src/common/, is that src/common/ is meant for code that's shared between frontend and backend, which this stuff is not. Also, many of these files depend on libpq which seems like an inappropriate dependency for src/common. Having said that, I also notice these dependencies: src/bin/pg_dump/kwlookup.c -> ../../../src/backend/parser/kwlookup.c src/bin/psql/kwlookup.c -> ../../../src/backend/parser/kwlookup.c src/bin/scripts/kwlookup.c -> ../../../src/backend/parser/kwlookup.c src/interfaces/ecpg/preproc/kwlookup.c -> ../../../../src/backend/parser/kwlookup.c src/bin/initdb/encnames.c -> ../../../src/backend/utils/mb/encnames.c src/interfaces/libpq/encnames.c -> ../../../src/backend/utils/mb/encnames.c which seem like they'd be better handled by moving those files into src/common/. Thoughts? regards, tom lane
On Wed, Mar 23, 2016 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm not terribly happy with the hack I used to get pgbench to be able > to borrow psql's psqlscan.l lexer. It's a mess for the MSVC build > scripts, and I have seen it causing two concurrent builds of psqlscan.o > in parallel builds, which is likely to cause a build failure some of > the time. Another unresolved issue is that we can't apply FLEX_NO_BACKUP > testing to both of psql's lexers, because that causes each of those > make steps to want to write lex.backup in the current directory. > > I have a modest proposal for improving this: let's move all the code > that's currently shared by two or more src/bin/ subdirectories into a > new directory, say src/feutils, and build it into a ".a" library in > the same way that src/common/ is handled. This would remove the > following klugy cross-directory links: > > src/bin/pgbench/psqlscan.o -> ../../../src/bin/psql/psqlscan.o > src/bin/psql/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c > src/bin/psql/keywords.c -> ../../../src/bin/pg_dump/keywords.c > src/bin/scripts/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c > src/bin/scripts/keywords.c -> ../../../src/bin/pg_dump/keywords.c > src/bin/scripts/mbprint.c -> ../../../src/bin/psql/mbprint.c > src/bin/scripts/print.c -> ../../../src/bin/psql/print.c > > Note: the reason for a new subdirectory, rather than putting this > stuff into src/common/, is that src/common/ is meant for code that's > shared between frontend and backend, which this stuff is not. Also, > many of these files depend on libpq which seems like an inappropriate > dependency for src/common. > > Having said that, I also notice these dependencies: > > src/bin/pg_dump/kwlookup.c -> ../../../src/backend/parser/kwlookup.c > src/bin/psql/kwlookup.c -> ../../../src/backend/parser/kwlookup.c > src/bin/scripts/kwlookup.c -> ../../../src/backend/parser/kwlookup.c > src/interfaces/ecpg/preproc/kwlookup.c -> ../../../../src/backend/parser/kwlookup.c > src/bin/initdb/encnames.c -> ../../../src/backend/utils/mb/encnames.c > src/interfaces/libpq/encnames.c -> ../../../src/backend/utils/mb/encnames.c > > which seem like they'd be better handled by moving those files into > src/common/. > > Thoughts? No objection. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 23/03/16 17:55, Tom Lane wrote: > I'm not terribly happy with the hack I used to get pgbench to be able > to borrow psql's psqlscan.l lexer. It's a mess for the MSVC build > scripts, and I have seen it causing two concurrent builds of psqlscan.o > in parallel builds, which is likely to cause a build failure some of > the time. Another unresolved issue is that we can't apply FLEX_NO_BACKUP > testing to both of psql's lexers, because that causes each of those > make steps to want to write lex.backup in the current directory. > > I have a modest proposal for improving this: let's move all the code > that's currently shared by two or more src/bin/ subdirectories into a > new directory, say src/feutils, and build it into a ".a" library in > the same way that src/common/ is handled. This would remove the > following klugy cross-directory links: > > src/bin/pgbench/psqlscan.o -> ../../../src/bin/psql/psqlscan.o > src/bin/psql/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c > src/bin/psql/keywords.c -> ../../../src/bin/pg_dump/keywords.c > src/bin/scripts/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c > src/bin/scripts/keywords.c -> ../../../src/bin/pg_dump/keywords.c > src/bin/scripts/mbprint.c -> ../../../src/bin/psql/mbprint.c > src/bin/scripts/print.c -> ../../../src/bin/psql/print.c > > Note: the reason for a new subdirectory, rather than putting this > stuff into src/common/, is that src/common/ is meant for code that's > shared between frontend and backend, which this stuff is not. Also, > many of these files depend on libpq which seems like an inappropriate > dependency for src/common. > > Having said that, I also notice these dependencies: > > src/bin/pg_dump/kwlookup.c -> ../../../src/backend/parser/kwlookup.c > src/bin/psql/kwlookup.c -> ../../../src/backend/parser/kwlookup.c > src/bin/scripts/kwlookup.c -> ../../../src/backend/parser/kwlookup.c > src/interfaces/ecpg/preproc/kwlookup.c -> ../../../../src/backend/parser/kwlookup.c > src/bin/initdb/encnames.c -> ../../../src/backend/utils/mb/encnames.c > src/interfaces/libpq/encnames.c -> ../../../src/backend/utils/mb/encnames.c > > which seem like they'd be better handled by moving those files into > src/common/. > > Thoughts? > Yes please! -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Mar 23, 2016 at 5:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not terribly happy with the hack I used to get pgbench to be able
to borrow psql's psqlscan.l lexer. It's a mess for the MSVC build
scripts, and I have seen it causing two concurrent builds of psqlscan.o
in parallel builds, which is likely to cause a build failure some of
the time. Another unresolved issue is that we can't apply FLEX_NO_BACKUP
testing to both of psql's lexers, because that causes each of those
make steps to want to write lex.backup in the current directory.
I have a modest proposal for improving this: let's move all the code
that's currently shared by two or more src/bin/ subdirectories into a
new directory, say src/feutils, and build it into a ".a" library in
the same way that src/common/ is handled. This would remove the
following klugy cross-directory links:
src/bin/pgbench/psqlscan.o -> ../../../src/bin/psql/psqlscan.o
src/bin/psql/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c
src/bin/psql/keywords.c -> ../../../src/bin/pg_dump/keywords.c
src/bin/scripts/dumputils.c -> ../../../src/bin/pg_dump/dumputils.c
src/bin/scripts/keywords.c -> ../../../src/bin/pg_dump/keywords.c
src/bin/scripts/mbprint.c -> ../../../src/bin/psql/mbprint.c
src/bin/scripts/print.c -> ../../../src/bin/psql/print.c
Note: the reason for a new subdirectory, rather than putting this
stuff into src/common/, is that src/common/ is meant for code that's
shared between frontend and backend, which this stuff is not. Also,
many of these files depend on libpq which seems like an inappropriate
dependency for src/common.
Having said that, I also notice these dependencies:
src/bin/pg_dump/kwlookup.c -> ../../../src/backend/parser/kwlookup.c
src/bin/psql/kwlookup.c -> ../../../src/backend/parser/kwlookup.c
src/bin/scripts/kwlookup.c -> ../../../src/backend/parser/kwlookup.c
src/interfaces/ecpg/preproc/kwlookup.c -> ../../../../src/backend/parser/kwlookup.c
src/bin/initdb/encnames.c -> ../../../src/backend/utils/mb/encnames.c
src/interfaces/libpq/encnames.c -> ../../../src/backend/utils/mb/encnames.c
which seem like they'd be better handled by moving those files into
src/common/.
Thoughts?
+1 on both. I've certainly been annoyed by that kwlookup thing many times :)
Tom Lane wrote: > Note: the reason for a new subdirectory, rather than putting this > stuff into src/common/, is that src/common/ is meant for code that's > shared between frontend and backend, which this stuff is not. Also, > many of these files depend on libpq which seems like an inappropriate > dependency for src/common. Actually you could just list them in OBJS_FRONTEND in src/common. That way they're not built for the server at all; no need for a new subdir. The libpq dependency argument AFAICS only applies to the ones in src/bin/psql. Perhaps we could have feutils with those only, and move the other files to src/common. I'm unclear on the #ifndef PGSCRIPTS thingy in mbprint.c. Perhaps it's no longer needed? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Tom Lane wrote: >> Note: the reason for a new subdirectory, rather than putting this >> stuff into src/common/, is that src/common/ is meant for code that's >> shared between frontend and backend, which this stuff is not. Also, >> many of these files depend on libpq which seems like an inappropriate >> dependency for src/common. > Actually you could just list them in OBJS_FRONTEND in src/common. That > way they're not built for the server at all; no need for a new subdir. Meh. I think stuff in OBJS_FRONTEND has to be pretty weird special cases, such as frontend emulations of server-environment code. Which is what fe_memutils is, so that's OK, but I kinda question whether restricted_token.c belongs in src/common/ at all. > The libpq dependency argument AFAICS only applies to the ones in > src/bin/psql. Perhaps we could have feutils with those only, and move > the other files to src/common. On looking closer, the only one that doesn't depend on libpq is keywords.c, which seems like it belongs in the same place as kwlookup.c. So yeah, let's move both of those to src/common. Anybody want to bikeshed the directory name src/feutils? Maybe fe-utils would be more readable. And where to put the corresponding header files? src/include/fe-utils? regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > Tom Lane wrote: > > Actually you could just list them in OBJS_FRONTEND in src/common. That > > way they're not built for the server at all; no need for a new subdir. > > Meh. I think stuff in OBJS_FRONTEND has to be pretty weird special > cases, such as frontend emulations of server-environment code. Which > is what fe_memutils is, so that's OK, but I kinda question whether > restricted_token.c belongs in src/common/ at all. OK, that makes sense. Feel free to move restricted_token.c if you feel like it. > > The libpq dependency argument AFAICS only applies to the ones in > > src/bin/psql. Perhaps we could have feutils with those only, and move > > the other files to src/common. > > On looking closer, the only one that doesn't depend on libpq is > keywords.c, which seems like it belongs in the same place as kwlookup.c. > So yeah, let's move both of those to src/common. I guess those dependencies are somewhat hidden. No objections to that move. > Anybody want to bikeshed the directory name src/feutils? Maybe fe-utils > would be more readable. Yes, +1 for either a - or _ in there. > And where to put the corresponding header files? > src/include/fe-utils? Sounds fair but would that be installed in PREFIX/include/server? That'd be a bit odd, but I'm not -1 on that. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Tom Lane wrote: >> And where to put the corresponding header files? >> src/include/fe-utils? > Sounds fair but would that be installed in PREFIX/include/server? > That'd be a bit odd, but I'm not -1 on that. The only other plan I can think of is to put the .h files describing src/fe-utils files into src/fe-utils itself. Which would sort of match the existing precedent of src/bin/ files looking into sibling directories for relevant .h files, but it seems a bit odd too. regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > Tom Lane wrote: > >> And where to put the corresponding header files? > >> src/include/fe-utils? > > > Sounds fair but would that be installed in PREFIX/include/server? > > That'd be a bit odd, but I'm not -1 on that. > > The only other plan I can think of is to put the .h files describing > src/fe-utils files into src/fe-utils itself. Which would sort of match > the existing precedent of src/bin/ files looking into sibling directories > for relevant .h files, but it seems a bit odd too. I think it's better that they end up installed in PREFIX/include/server/fe_utils rather than not installed at all, which is what would happen, unless you were to create a tailored command in make install. For instance we have the ones for src/common in PREFIX/include/server/common, which I suppose is useful for extensions (we do install libpgcommon.a, which would be messy to use otherwise.) -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
I wrote: > Having said that, I also notice these dependencies: > ... > src/bin/initdb/encnames.c -> ../../../src/backend/utils/mb/encnames.c > src/interfaces/libpq/encnames.c -> ../../../src/backend/utils/mb/encnames.c > ... > which seem like they'd be better handled by moving those files into > src/common/. After poking around a bit, it seems like it ought to be a win to move both encnames.c and wchar.c into src/common/, as both of those modules are meant to be built for both backend and frontend environments. The stumbling block is pg_wchar.h, which is a total fail modularity-wise, as it does not bother to distinguish stuff which should be visible to the general backend from stuff which should be visible to frontend programs from stuff which is solely of interest to encoding conversion logic (and in many cases only to *particular* conversions). I think we should not do this move without figuring out which parts of that file sanely belong in a src/include/common/ header file and which don't. That's not something that I feel like tackling right now, as it's been in that same messy state for awhile and recent changes haven't made it worse. But it ought to be revisited at some point. regards, tom lane
On Wed, Mar 23, 2016 at 4:00 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >> Anybody want to bikeshed the directory name src/feutils? Maybe fe-utils >> would be more readable. > > Yes, +1 for either a - or _ in there. I vote for an underscore, since that's what we mostly do. [rhaas pgsql]$ find . -type d | awk -F/ '{print $NF}' | grep - | wc -l 6 [rhaas pgsql]$ find . -type d | awk -F/ '{print $NF}' | grep _ | wc -l 74 -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
I wrote: > I have a modest proposal for improving this: let's move all the code > that's currently shared by two or more src/bin/ subdirectories into a > new directory, say src/feutils, and build it into a ".a" library in > the same way that src/common/ is handled. Moving along on this project: I'm looking at pg_dump's dumputils.c/.h, which contains a fair amount of generally-useful stuff but also some stuff that seems to have no earthly use outside pg_dump/pg_dumpall. In particular I do not see the point of moving these things to a shared directory: PGDUMP_STRFTIME_FMT macro extern bool buildACLCommands(const char *name, const char *subname, const char *type, const char *acls, constchar *owner, const char *prefix, int remoteVersion, PQExpBuffer sql); extern bool buildDefaultACLCommands(const char *type, const char *nspname, const char *acls, constchar *owner, int remoteVersion, PQExpBuffer sql); extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name, uint32 objectId, PQExpBuffersql); extern void emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer, const char *target, const char*objname); What I propose doing is leaving the above-listed items in pg_dump/dumputils.h/.c, and moving the rest of what's in those files to new files src/include/fe_utils/string_utils.h and src/fe_utils/string_utils.c. This name is a bit arbitrary, but most of what's there is string processing of some flavor or other, with some list processing thrown in for good measure. If anyone's got a different color to paint this bikeshed, please speak up. regards, tom lane
Robert Haas <robertmhaas@gmail.com> writes: > On Wed, Mar 23, 2016 at 4:00 PM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: >> Yes, +1 for either a - or _ in there. > I vote for an underscore, since that's what we mostly do. Yup, I had just counted and come to the same conclusion. fe_utils/ it shall be. regards, tom lane
Tom Lane wrote: > What I propose doing is leaving the above-listed items in > pg_dump/dumputils.h/.c, and moving the rest of what's in those files > to new files src/include/fe_utils/string_utils.h and > src/fe_utils/string_utils.c. Seems reasonable. > This name is a bit arbitrary, but most of what's there is string > processing of some flavor or other, with some list processing thrown > in for good measure. If anyone's got a different color to paint this > bikeshed, please speak up. I wondered about the list stuff while messing about in pg_dump awhile ago. It seems moderately okay, but not terribly generic; maybe we should get rid of all that stuff and make ilist.c available to frontend. Not sure how easy is that, given that AFAIR ilist uses elog. Anyway maybe we can discuss that in the future, to avoid blocking your patch. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Tom Lane wrote: >> This name is a bit arbitrary, but most of what's there is string >> processing of some flavor or other, with some list processing thrown >> in for good measure. If anyone's got a different color to paint this >> bikeshed, please speak up. > I wondered about the list stuff while messing about in pg_dump awhile > ago. It seems moderately okay, but not terribly generic; maybe we > should get rid of all that stuff and make ilist.c available to frontend. > Not sure how easy is that, given that AFAIR ilist uses elog. Anyway > maybe we can discuss that in the future, to avoid blocking your patch. It wouldn't be terribly hard to split the file into, say, string_utils and simple_list files. On reflection that seems like a good idea as long as we're going for cleanup rather than a one-to-one rename. regards, tom lane
Hi there, > On 23 Mar 2016, at 22:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Anybody want to bikeshed the directory name src/feutils? Maybe fe-utils > would be more readable. And where to put the corresponding header files? > src/include/fe-utils? For me “utils" sounds like something of auxiliary nature. if some pretty basic stuff is going to be added there, then I believe that fe_common or perhaps fe_shared would be a bit more suitable. Regards, Aleksey
Aleksey Demakov <a.demakov@postgrespro.ru> writes: > Hi there, >> On 23 Mar 2016, at 22:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Anybody want to bikeshed the directory name src/feutils? Maybe fe-utils >> would be more readable. And where to put the corresponding header files? >> src/include/fe-utils? > For me “utils" sounds like something of auxiliary nature. if some pretty > basic stuff is going to be added there, then I believe that fe_common or > perhaps fe_shared would be a bit more suitable. Meh... The stuff that is in-scope for it at the moment doesn't seem all that basic. It's just assorted widgets that turned out to be useful outside their original home. Really basic stuff is going into src/common/ these days. regards, tom lane
On Fri, Mar 25, 2016 at 1:11 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Tom Lane wrote: > >> What I propose doing is leaving the above-listed items in >> pg_dump/dumputils.h/.c, and moving the rest of what's in those files >> to new files src/include/fe_utils/string_utils.h and >> src/fe_utils/string_utils.c. > > Seems reasonable. > >> This name is a bit arbitrary, but most of what's there is string >> processing of some flavor or other, with some list processing thrown >> in for good measure. If anyone's got a different color to paint this >> bikeshed, please speak up. > > I wondered about the list stuff while messing about in pg_dump awhile > ago. It seems moderately okay, but not terribly generic; maybe we > should get rid of all that stuff and make ilist.c available to frontend. > Not sure how easy is that, given that AFAIR ilist uses elog. Anyway > maybe we can discuss that in the future, to avoid blocking your patch. Definitely worth it, list mimics of what is in pg_dump are located in pg_basebackup, and since a 9.6 patch in psql as well. That's as much code duplication. Preventing the use of elog in the frontend is something that has been addressed multiple times with FRONTEND, so that's not likely going to be an issue I think. Andres has mentioned as well having some elog stuff available in frontend.. -- Michael
Michael Paquier <michael.paquier@gmail.com> writes: > On Fri, Mar 25, 2016 at 1:11 AM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: >> I wondered about the list stuff while messing about in pg_dump awhile >> ago. It seems moderately okay, but not terribly generic; maybe we >> should get rid of all that stuff and make ilist.c available to frontend. >> Not sure how easy is that, given that AFAIR ilist uses elog. Anyway >> maybe we can discuss that in the future, to avoid blocking your patch. > Definitely worth it, list mimics of what is in pg_dump are located in > pg_basebackup, and since a 9.6 patch in psql as well. That's as much > code duplication. Well, it's not a *lot* of code duplication. Agreed, it's a bit ugly that we've got three or four copied-and-pasted versions of simple_foo_list_append, but I'm not convinced that it'd be worth the trouble to do anything about that. If FE code starts needing more list functionality than that, maybe importing ilist or similar would be worthwhile, but right now I think it would be overkill. > Preventing the use of elog in the frontend is something that has been > addressed multiple times with FRONTEND, so that's not likely going to > be an issue I think. Andres has mentioned as well having some elog > stuff available in frontend.. I'm on board with doing something about that one, though. There are quite a lot of places that could be cleaned up. regards, tom lane
On Fri, Mar 25, 2016 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paquier@gmail.com> writes: >> Preventing the use of elog in the frontend is something that has been >> addressed multiple times with FRONTEND, so that's not likely going to >> be an issue I think. Andres has mentioned as well having some elog >> stuff available in frontend.. > > I'm on board with doing something about that one, though. There are > quite a lot of places that could be cleaned up. Not sure if Andres is working on that for now or not, the main discussion that I am foreseeing here is how we are going to map elevel for the frontend (should FATAL, PANIC exit immediately, etc). I think as well that some sort of callback system would be needed to allow frontend clients to perform filtering of the log entries. Say pg_rewind has a --debug mode, so this would map with DEBUG1 entries, we'd need a way to control if those are issued or not depending on the frontend call arguments... Surely that's a cleanup patch 9.7 though, so there are a couple of months ahead of us... And that's not the highest priority now. Stability of 9.6 is first and just ahead. -- Michael
Michael Paquier <michael.paquier@gmail.com> writes: > Not sure if Andres is working on that for now or not, the main > discussion that I am foreseeing here is how we are going to map elevel > for the frontend (should FATAL, PANIC exit immediately, etc). Doesn't seem that complicated to me: elevel >= ERROR results in exit(1), otherwise just print to stderr and return. We'd be eyeballing each case that we remove "#ifndef FRONTEND" from anyway; if it's expecting behavior noticeably more complicated than that, we could leave it as-is. > Stability of 9.6 is first and just ahead. Agreed, this is code cleanup not a high priority. regards, tom lane
On Fri, Mar 25, 2016 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paquier@gmail.com> writes: >> Not sure if Andres is working on that for now or not, the main >> discussion that I am foreseeing here is how we are going to map elevel >> for the frontend (should FATAL, PANIC exit immediately, etc). > > Doesn't seem that complicated to me: elevel >= ERROR results in exit(1), > otherwise just print to stderr and return. We'd be eyeballing each case > that we remove "#ifndef FRONTEND" from anyway; if it's expecting behavior > noticeably more complicated than that, we could leave it as-is. Something that I see as mandatory as well is a way to bypass some of the elevels depending on the way a frontend tool is called, so we'd need something like that in the common elog facility: void elog(blah) { #ifdef FRONTEND if (!callback_routine_defined_in_frontend(elevel)) return; #endif blah_blah_move_on. } This would be useful to avoid code patterns of this type in a frontend tool where for example a debug flag is linked with elevel. For example this pattern: if (debug) elog(DEBUG1, "Debug blah"); could be reduced to as the callback routine would allow bypassing elog() directly: elog(DEBUG1, "Debug blah"); That's just food for thought at this stage, I get back to reviewing... -- Michael