Re: [PATCH] SQL function to report log message

Hi,

On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:

> Hi
>
> I am starting to work review of this patch
>
> 2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkumar02(at)gmail(dot)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.

> 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.

> 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.

Adding new patch, with the above fixes.

Thanks in advance.

Regards,
Dinesh

>
> Regards
>
> Pavel
>
>
>>
>> Regards,
>> Dinesh
>> manojadinesh.blogspot.com
>>
>
>