Julien Rouhaud <rjuju123@gmail.com> writes:
> [ v4-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patch ]
This seems about ready to go to me, except for
(1) as an exported API, pg_get_querydef needs a full API spec in its
header comment. "Read the code to figure out what to do" is not OK
in my book.
(2) I don't think this has been thought out too well:
> I also kept the wrapColument and startIdent as they
> can be easily used by callers.
The appropriate values for these are determined by macros that are
local in ruleutils.c, so it's not that "easy" for outside callers
to conform to standard practice. I suppose we could move
WRAP_COLUMN_DEFAULT etc into ruleutils.h, but is there actually a
use-case for messing with those? I don't see any other exported
functions that go beyond offering a "bool pretty" option, so
I think it might be a mistake for this one to be different.
(The pattern that I see is that a ruleutils function could have
"bool pretty", or it could have "int prettyFlags, int startIndent"
which is an expansion of that; but mixing those levels of detail
doesn't seem very wise.)
regards, tom lane
