Julien Rouhaud <rjuju...@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