On Sun, Aug 30, 2015 at 4:52 AM, Pavel Stehule <pavel.steh...@gmail.com> wrote:
> Hi > > I am starting to work review of this patch > > Hi Pavel, Thanks for your review. > 2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkuma...@gmail.com>: > >> Hi All, >> >> Greetings for the day. >> >> Would like to discuss on below feature here. >> >> Feature: >> Having an SQL function, to write messages to log destination. >> >> Justification: >> As of now, we don't have an SQL function to write custom/application >> messages to log destination. We have "RAISE" clause which is controlled by >> log_ parameters. If we have an SQL function which works irrespective of >> log settings, that would be a good for many log parsers. What i mean is, in >> DBA point of view, if we route all our native OS stats to log files in a >> proper format, then we can have our log reporting tools to give most >> effective reports. Also, Applications can log their own messages to >> postgres log files, which can be monitored by DBAs too. >> >> Implementation: >> Implemented a new function "pg_report_log" which takes one argument >> as text, and returns void. I took, "LOG" prefix for all the reporting >> messages.I wasn't sure to go with new prefix for this, since these are >> normal LOG messages. Let me know, if i am wrong here. >> >> Here is the attached patch. >> > > This patch is not complex, but the implementation doesn't cover a > "ereport" well. > > Although this functionality should be replaced by custom function in any > PL (now or near future), I am not against to have this function in core. > There are lot of companies with strong resistance against stored procedures > - and sometimes this functionality can help with SQL debugging. > > Issues: > > 1. Support only MESSAGE field in exception - I am expecting to support all > fields: HINT, DETAIL, ... > 2. Missing regress tests > 3. the parsing ereport level should be public function shared with PLpgSQL > and other PL > 4. should be hidestmt mandatory parameter? > 5. the function declaration is strange > > postgres=# \sf pg_report_log (text, anyelement, boolean) > CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, anyelement, > boolean) > RETURNS void > LANGUAGE sql > STABLE STRICT COST 1 > AS $function$SELECT pg_report_log($1::pg_catalog.text, > $2::pg_catalog.text, $3::boolean)$function$ > > Why polymorphic? It is useless on any modern release > > postgres=# \sf pg_report_log (text, text, boolean) > CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text, boolean) > RETURNS void > LANGUAGE internal > IMMUTABLE STRICT > AS $function$pg_report_log$function$ > > Why stable, why immutable? This function should be volatile. > > 6. using elog level enum as errcode is wrong idea - errcodes are defined > in table http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html > > Let me go through each concern and will update you on this. Regards, Dinesh manojadinesh.blogspot.com > Regards > > Pavel > > >> >> Regards, >> Dinesh >> manojadinesh.blogspot.com >> > >
