2015-08-31 20:43 GMT+02:00 dinesh kumar <dineshkuma...@gmail.com>: > Hi, > > On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <pavel.steh...@gmail.com> > wrote: > >> Hi >> >> I am starting to work review of this patch >> >> 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, ... >> > > Added these functionalities too. > > >> 2. Missing regress tests >> > > Adding here. > > >> 3. the parsing ereport level should be public function shared with >> PLpgSQL and other PL >> > > Sorry Pavel. I am not getting your point here. Would you give me an > example. >
The transformation: text -> error level is common task - and PLpgSQL it does in pl_gram.y. My idea is to add new function to error utils named "parse_error_level" and use it from PLpgSQL and from your code. > > >> 4. should be hidestmt mandatory parameter? >> > > I changed this argument's default value as "true". > > >> 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 >> >> > I took quote_ident(anyelement) as referral code, for implementing this. > Could you guide me if I am doing wrong here. > I was wrong - this is ok - it is emulation of force casting to text > > >> 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. >> >> Fixed these to 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 >> > > You mean, if the elevel is 'ERROR', then we need to allow errcode. Let me > know your inputs. > I was blind, but the code was not good. Yes, error and higher needs error code. From ANSI/SQL anything can has error code "00 is ok", "01 .. warnings" ... There is more possibilities - look to PLpgSQL implementation - it can be optional parameter - it default can use ERRCODE_RAISE_EXCEPTION > > Adding new patch, with the above fixes. > > Thanks in advance. > > Regards, > Dinesh > >> >> Regards >> >> Pavel >> >> >>> >>> Regards, >>> Dinesh >>> manojadinesh.blogspot.com >>> >> >> >
