[PATCH] SQL function to report log message

On Mon, Jul 13, 2015 at 4:54 PM, dinesh kumar <dineshkumar02@gmail.com> wrote:

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.

What's the actual use case for this feature other than it would be
good to have it? A log message is here to give information about the
state of something that happens in backend, but in the case of this
function the event that happens is the content of the function itself.
It also adds a new log level for something that has a unique usage,
which looks like an overkill to me. Btw, you could do something more
advanced with simply an extension if you really want to play with this
area... But I am dubious about what kind of applications would use
that.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 13, 2015 at 1:11 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Mon, Jul 13, 2015 at 4:54 PM, dinesh kumar <dineshkumar02@gmail.com>
wrote:

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.

What's the actual use case for this feature other than it would be
good to have it?

That's a good question Michael.

When i was working as a DBA for a different RDBMS, developers used to write
some serious APP errors, Followed by instructions into some sort of log and
trace files.
Since, DBAs didn't have the permission to check app logs, which was owned
by Ops team.

In my old case, application was having serious OOM issues, which was
crashing frequently after the deployment. It wasn't the consistent behavior
from the app side, hence they used to sent a copy all APP metrics to trace
files, and we were monitoring the DB what was happening during the spike on
app servers.

I didn't mean that, we need to have this feature, since we have it on other
RDBMS. I don't see a reason, why don't we have this in our PG too.

I see the similar item in our development list
</messages/by-id/53A8E96E.9060507@2ndquadrant.com&gt;.

Let me know if i miss anything here.

Best Regards,
Dinesh
manojadinesh.blogspot.com

A log message is here to give information about the

On 7/13/15 12:39 PM, dinesh kumar wrote:

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.

What's the actual use case for this feature other than it would be
good to have it?

That's a good question Michael.

When i was working as a DBA for a different RDBMS, developers used to
write some serious APP errors, Followed by instructions into some sort
of log and trace files.
Since, DBAs didn't have the permission to check app logs, which was
owned by Ops team.

In my old case, application was having serious OOM issues, which was
crashing frequently after the deployment. It wasn't the consistent
behavior from the app side, hence they used to sent a copy all APP
metrics to trace files, and we were monitoring the DB what was happening
during the spike on app servers.

Spewing a bunch of stuff into the postgres log doesn't seem like an
improvement here.

I don't really see the point of what you're describing here. Just do
something like RAISE WARNING which should normally be high enough to
make it into the logs. Or use a pl language that will let you write your
own logfile on the server (ie: plperlu).

I didn't mean that, we need to have this feature, since we have it on
other RDBMS. I don't see a reason, why don't we have this in our PG too.

I see the similar item in our development list
</messages/by-id/53A8E96E.9060507@2ndquadrant.com&gt;.

That's not at all what that item is talking about. It's talking about
exposing ereport as a SQL function, without altering the rest of our
logging behavior.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 13, 2015 at 1:29 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 7/13/15 12:39 PM, dinesh kumar wrote:

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.

What's the actual use case for this feature other than it would be
good to have it?

That's a good question Michael.

When i was working as a DBA for a different RDBMS, developers used to
write some serious APP errors, Followed by instructions into some sort
of log and trace files.
Since, DBAs didn't have the permission to check app logs, which was
owned by Ops team.

In my old case, application was having serious OOM issues, which was
crashing frequently after the deployment. It wasn't the consistent
behavior from the app side, hence they used to sent a copy all APP
metrics to trace files, and we were monitoring the DB what was happening
during the spike on app servers.

Spewing a bunch of stuff into the postgres log doesn't seem like an
improvement here.

Agreed Jim.

I don't really see the point of what you're describing here. Just do
something like RAISE WARNING which should normally be high enough to make
it into the logs. Or use a pl language that will let you write your own
logfile on the server (ie: plperlu).

True. Using plperlu, shall we bypass our log_* settings. If it's true, i

wasn't sure about it.

I didn't mean that, we need to have this feature, since we have it on

other RDBMS. I don't see a reason, why don't we have this in our PG too.

I see the similar item in our development list
</messages/by-id/53A8E96E.9060507@2ndquadrant.com&gt;.

That's not at all what that item is talking about. It's talking about
exposing ereport as a SQL function, without altering the rest of our
logging behavior.

Ah. It's' my bad interpretation. Let me work on it, and will send a new
patch as a wrapper sql function for ereport.

Thanks again.

Regards,
Dinesh
manojadinesh.blogspot.com

--

On 7/13/15 3:39 PM, dinesh kumar wrote:

I don't really see the point of what you're describing here. Just do
something like RAISE WARNING which should normally be high enough to
make it into the logs. Or use a pl language that will let you write
your own logfile on the server (ie: plperlu).

True. Using plperlu, shall we bypass our log_* settings. If it's true, i
wasn't sure about it.

plperlu can do anything the server can do. Including fun things like
appending to any file the server can write to or executing things like
`rm -rf pg_xlog`.

I didn't mean that, we need to have this feature, since we have
it on
other RDBMS. I don't see a reason, why don't we have this in our
PG too.

I see the similar item in our development list
</messages/by-id/53A8E96E.9060507@2ndquadrant.com&gt;.

That's not at all what that item is talking about. It's talking
about exposing ereport as a SQL function, without altering the rest
of our logging behavior.

Ah. It's' my bad interpretation. Let me work on it, and will send a new
patch as a wrapper sql function for ereport.

You might want to present a plan for that; it's not as trivial as it
sounds due to how ereport works. In particular, I'd want to see (at
minimum) the same functionality that plpgsql's RAISE command now
provides (errdetail, errhint, etc).
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 13, 2015 at 1:56 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 7/13/15 3:39 PM, dinesh kumar wrote:

I don't really see the point of what you're describing here. Just do
something like RAISE WARNING which should normally be high enough to
make it into the logs. Or use a pl language that will let you write
your own logfile on the server (ie: plperlu).

True. Using plperlu, shall we bypass our log_* settings. If it's true, i
wasn't sure about it.

plperlu can do anything the server can do. Including fun things like
appending to any file the server can write to or executing things like `rm
-rf pg_xlog`.

Thanks Much Jim.

I didn't mean that, we need to have this feature, since we have

it on
other RDBMS. I don't see a reason, why don't we have this in our
PG too.

I see the similar item in our development list
<
/messages/by-id/53A8E96E.9060507@2ndquadrant.com&gt;.

That's not at all what that item is talking about. It's talking
about exposing ereport as a SQL function, without altering the rest
of our logging behavior.

Ah. It's' my bad interpretation. Let me work on it, and will send a new
patch as a wrapper sql function for ereport.

You might want to present a plan for that; it's not as trivial as it
sounds due to how ereport works. In particular, I'd want to see (at
minimum) the same functionality that plpgsql's RAISE command now provides
(errdetail, errhint, etc).

Sure. Let me prepare a prototype for it, and will share with you before
implementing.

Best Regards,
Dinesh
manojadinesh.blogspot.com

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 7/13/15 3:39 PM, dinesh kumar wrote:

Ah. It's' my bad interpretation. Let me work on it, and will send a new
patch as a wrapper sql function for ereport.

You might want to present a plan for that; it's not as trivial as it
sounds due to how ereport works. In particular, I'd want to see (at
minimum) the same functionality that plpgsql's RAISE command now
provides (errdetail, errhint, etc).

The real question is why the existing functionality in plpgsql isn't
sufficient. Somebody who wants a "log from SQL" function can easily
write a simple plpgsql function that does exactly what they want,
with no more or fewer bells-n-whistles than they need. If we try
to create a SQL function that does all that, it's likely to be a mess
to use, even with named arguments.

I'm not necessarily against the basic idea, but I think inventing
something that actually offers an increment in usability compared
to the existing alternative is going to be harder than it sounds.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi All,

On Mon, Jul 13, 2015 at 2:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 7/13/15 3:39 PM, dinesh kumar wrote:

Ah. It's' my bad interpretation. Let me work on it, and will send a new
patch as a wrapper sql function for ereport.

You might want to present a plan for that; it's not as trivial as it
sounds due to how ereport works. In particular, I'd want to see (at
minimum) the same functionality that plpgsql's RAISE command now
provides (errdetail, errhint, etc).

Jim,

For now, I worked on (ERROR Level, ERROR Message, HIDE ERROR Stmt). In our
to do item description, I found this wrapper needs to return "Anyelement".
But, I believe, return "VOID" is enough for this function. Let me know if I
erred here.

In design phase,

1. I took a CustomDataType with the elevel code, elevel text

2. Populated this CDT with all existing pre-processors, except {FATAL,
PANIC}. Since, we don't expose these to client.

3. By matching the user elevel text, processing the report log function.

Find the attached patch with implementation.

The real question is why the existing functionality in plpgsql isn't
sufficient. Somebody who wants a "log from SQL" function can easily
write a simple plpgsql function that does exactly what they want,
with no more or fewer bells-n-whistles than they need. If we try
to create a SQL function that does all that, it's likely to be a mess
to use, even with named arguments.

I'm not necessarily against the basic idea, but I think inventing
something that actually offers an increment in usability compared
to the existing alternative is going to be harder than it sounds.

Tom,

I agree with your inputs. We can build pl/pgsql function as alternative
for this.

My initial proposal, and implementation was, logging messages to log file
irrespectively of our log settings. I was not sure we can do this with some
pl/perlu. And then, I started working on our to do item,
ereport, wrapper callable from SQL, and found it can be useful to have a
direct function call with required log level.

Thanks.

Regards,
Dinesh
manojadinesh.blogspot.com

regards, tom lane

On Thu, Jul 23, 2015 at 10:56 AM, dinesh kumar <dineshkumar02@gmail.com> wrote:

Hi All,

On Mon, Jul 13, 2015 at 2:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 7/13/15 3:39 PM, dinesh kumar wrote:

Ah. It's' my bad interpretation. Let me work on it, and will send a new
patch as a wrapper sql function for ereport.

You might want to present a plan for that; it's not as trivial as it
sounds due to how ereport works. In particular, I'd want to see (at
minimum) the same functionality that plpgsql's RAISE command now
provides (errdetail, errhint, etc).

Jim,

For now, I worked on (ERROR Level, ERROR Message, HIDE ERROR Stmt). In our
to do item description, I found this wrapper needs to return "Anyelement".
But, I believe, return "VOID" is enough for this function. Let me know if I
erred here.

In design phase,

1. I took a CustomDataType with the elevel code, elevel text

2. Populated this CDT with all existing pre-processors, except {FATAL,
PANIC}. Since, we don't expose these to client.

3. By matching the user elevel text, processing the report log function.

Find the attached patch with implementation.

Btw, if you want to get more attention for your patch as well as
reviews, you should consider registering to the next commit fest of
September:
https://commitfest.postgresql.org/6/
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 22, 2015 at 8:56 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Thu, Jul 23, 2015 at 10:56 AM, dinesh kumar <dineshkumar02@gmail.com>
wrote:

Hi All,

On Mon, Jul 13, 2015 at 2:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 7/13/15 3:39 PM, dinesh kumar wrote:

Ah. It's' my bad interpretation. Let me work on it, and will send a

new

patch as a wrapper sql function for ereport.

You might want to present a plan for that; it's not as trivial as it
sounds due to how ereport works. In particular, I'd want to see (at
minimum) the same functionality that plpgsql's RAISE command now
provides (errdetail, errhint, etc).

Jim,

For now, I worked on (ERROR Level, ERROR Message, HIDE ERROR Stmt). In

our

to do item description, I found this wrapper needs to return

"Anyelement".

But, I believe, return "VOID" is enough for this function. Let me know

if I

erred here.

In design phase,

1. I took a CustomDataType with the elevel code, elevel text

2. Populated this CDT with all existing pre-processors, except {FATAL,
PANIC}. Since, we don't expose these to client.

3. By matching the user elevel text, processing the report log function.

Find the attached patch with implementation.

Thanks Michael.

Uploaded my patch there.

Regards,
Dinesh
manojadinesh.blogspot.com

Hi

I am starting to work review of this patch

2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkumar02@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

Regards

Pavel

On Sun, Aug 30, 2015 at 4:52 AM, Pavel Stehule <pavel.stehule@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 <dineshkumar02@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

Hi,

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

Hi

I am starting to work review of this patch

2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkumar02@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.

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

2015-08-31 20:43 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

Hi,

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

Hi

I am starting to work review of this patch

2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkumar02@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

2015-09-01 6:59 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

2015-08-31 20:43 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

Hi,

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

Hi

I am starting to work review of this patch

2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkumar02@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.

the code looks better

my objections:

1. I prefer default values be NULL
2. readability:
* parsing error level should be in alone cycle
* you don't need to use more ereport calls - one is good enough - look on
implementation of stmt_raise in PLpgSQL
3. test should be enhanced for optional parameters

Regards

Pavel

On 8/31/15 11:59 PM, Pavel Stehule wrote:

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.

Wouldn't it be better to create an ENUM of error levels instead of
inventing more parsing code?

Though, I guess ENUMs are case sensitive, but I'd rather solve that by
creating a CI ENUM, which would be useful across the board...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2015-09-01 7:20 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 8/31/15 11:59 PM, Pavel Stehule wrote:

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.

Wouldn't it be better to create an ENUM of error levels instead of
inventing more parsing code?

Do you think SQL ENUM? I little bit afraid about possible problems with
pg_upgrade.

It is not simple question - the ENUM can be interesting from custom space
perspective, but from our internal perspective the parsing function is more
practical - and faster. The error level is our internal value, and users
should not to read it - for this purpouse is error level.

On 9/1/15 12:47 AM, Pavel Stehule wrote:

Wouldn't it be better to create an ENUM of error levels instead of
inventing more parsing code?

Do you think SQL ENUM? I little bit afraid about possible problems with
pg_upgrade.

It is not simple question - the ENUM can be interesting from custom
space perspective, but from our internal perspective the parsing
function is more practical - and faster. The error level is our internal
value, and users should not to read it - for this purpouse is error level.

My thought is that there's a fair amount of places where we do string
comparison for not a great reason. Perhaps a better example is data that
comes back from a trigger; AFTER/BEFORE, INSERT/UPDATE/..., which is
more expensive to setup the variables for (strdup a fixed string, which
means a palloc), and then comparisons are done as text varlena (iirc).

Instead if this information came back as an ENUM the variable would be a
simple int as would the comparison. We'd still have a raw string being
parsed in the function body, but that would happen once during initial
compilation and it would be replaced with an ENUM value.

For RAISE, AFAIK we still end up converting the raw string into a
varlena CONST, which means a palloc. If it was an ENUM it'd be converted
to an int.

If we're worried about the overhead of the enum machinery we could
create a relcache for system enums, but I suspect that even without that
it'd be a win over the string stuff. Especially since I bet most people
run UTF8.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2015-09-02 0:13 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 9/1/15 12:47 AM, Pavel Stehule wrote:

Wouldn't it be better to create an ENUM of error levels instead of
inventing more parsing code?

Do you think SQL ENUM? I little bit afraid about possible problems with
pg_upgrade.

It is not simple question - the ENUM can be interesting from custom
space perspective, but from our internal perspective the parsing
function is more practical - and faster. The error level is our internal
value, and users should not to read it - for this purpouse is error level.

My thought is that there's a fair amount of places where we do string
comparison for not a great reason. Perhaps a better example is data that
comes back from a trigger; AFTER/BEFORE, INSERT/UPDATE/..., which is more
expensive to setup the variables for (strdup a fixed string, which means a
palloc), and then comparisons are done as text varlena (iirc).

Instead if this information came back as an ENUM the variable would be a
simple int as would the comparison. We'd still have a raw string being
parsed in the function body, but that would happen once during initial
compilation and it would be replaced with an ENUM value.

For RAISE, AFAIK we still end up converting the raw string into a varlena
CONST, which means a palloc. If it was an ENUM it'd be converted to an int.

If we're worried about the overhead of the enum machinery we could create
a relcache for system enums, but I suspect that even without that it'd be a
win over the string stuff. Especially since I bet most people run UTF8.

What I know, we currently don't use ENUM in core code. One reason can be
missing infrastructure - second increasing complexity for development - the
using ENUM needs database cleaning or initdb currently. There is lot of
work to get ENUM to state be developer friendly. I am don't think so this
is a area for this patch, this thread. If we use shared parsing of elog
levels, then we don't block future changes in this area.

Regards

Pavel