Daniel Erez <derez@redhat.com> writes:
> Many thanks for the prompt response and the suggestions!
> So, regarding the issue of "production quality" you've mentioned,
> we understand there are two remaining matters to address:
> 1. debug_query_string:
> As we can't rely on this flag, is there any alternative we can rely on?
> 2. encapsulation:
> Is there any "utility file" we could extract the logic to?
> Or, any other functions that are used for encapsulation mechanism?
The big picture here is that there are two ways we could go:
* Encapsulate some improved version of the logic I posted into an error
reporting auxiliary function, along the lines of function_errposition(fcinfo)
and then run around and add calls to this function in the ereports
where it seems useful. This would be the right answer if we think
only a few such ereports need the extra info --- but if we want it
to apply to many/most function-level error reports, it sounds pretty
darn tedious. Also it'd require that everyplace that is going to
throw such reports have access to the appropriate fcinfo, which would
require some refactoring in places where SQL functions have been
divided into multiple C routines.
* Add logic in execQual.c to automatically add the information whenever
an error is thrown while executing an identifiable expression node.
This solves the problem for all functions at one stroke. The problem
is that it would add some overhead to the non-error code path.
It's hard to see how to do this without at least half a dozen added
instructions per function or operator node (it'd likely involve
saving, setting, and restoring a global variable ...). I'm not sure
if that would be significant but it would very possibly be measurable
on function-call-heavy workloads. We might think that such overhead
is worth paying for better error reporting; but given how few people
have commented on this thread, I'm not sure many people are excited
about it.
I'd like to hear some more comments before undertaking either approach.
As for getting rid of the use of debug_query_string, it'd take some
work, though it seems do-able. I think ActivePortal->sourceText is
the right thing to reference in the "normal" case, but it may not
be the right thing when executing queries from SQL-language functions
for instance. Also, the reason I didn't use that in the quick-hack
patch was that the test cases I wanted to post involved calls that
will get executed during the planner's constant-simplification pass
(since varchar() is marked IMMUTABLE). It turns out there isn't
an ActivePortal at that point, so we'd need some thought about how
to make the right query string accessible during planning.
regards, tom lane