[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

On Tue, Sep 1, 2015 at 6:13 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

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.

I agree with Pavel on this one: creating an extra type here is going
to cause more pain than it removes.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
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, Aug 31, 2015 at 10:08 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

In this attached patch, I have minimized the ereport calls. Kindly check
and let me know.

3. test should be enhanced for optional parameters

Fixed it.

Regards,
Dinesh
manojadinesh.blogspot.com

Regards

2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

you have a true - in this case we can use YYTEXT - so the code can be some
like

if (tok != ';')
{
if (parse_elog_level(yytext, &elog_level)
{
...
}
}

but it means double string comparation, what is not good, or removing elog
levels from keyword list (what is surely out of area of this patch). So
using it in PLpgSQL was not practical idea. I am sorry.

Regards

Pavel

2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

In this attached patch, I have minimized the ereport calls. Kindly check
and let me know.

3. test should be enhanced for optional parameters

Fixed it.

only few points:

1. missing to set errstate - any exception should to have some errcode
value. There can be default like PLpgSQL ERRCODE_RAISE_EXCEPTION for any
where elog_level >= error

2. the explicit setting context is not consistent with our PL - the context
is implicit value only - not possible to set it explicitly. The behave of
this field is not clear - but in this moment, the context is related to
PostgreSQL area - not to application area.

Regards

Pavel

On Fri, Sep 4, 2015 at 1:08 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

you have a true - in this case we can use YYTEXT - so the code can be some
like

if (tok != ';')
{
if (parse_elog_level(yytext, &elog_level)
{
...
}
}

but it means double string comparation, what is not good, or removing elog
levels from keyword list (what is surely out of area of this patch). So
using it in PLpgSQL was not practical idea. I am sorry.

Hi,

No Worries. Thanks a lot for your guidance on this patch.

Regards,
Dinesh
manojadinesh.blogspot.com

On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

In this attached patch, I have minimized the ereport calls. Kindly check
and let me know.

3. test should be enhanced for optional parameters

Fixed it.

only few points:

1. missing to set errstate - any exception should to have some errcode
value. There can be default like PLpgSQL ERRCODE_RAISE_EXCEPTION for any
where elog_level >= error

Fixed It.

2. the explicit setting context is not consistent with our PL - the
context is implicit value only - not possible to set it explicitly. The
behave of this field is not clear - but in this moment, the context is
related to PostgreSQL area - not to application area.

OK. Shall i remove the context field from this function.

Regards,
Dinesh
manojadinesh.blogspot.com

Regards

2015-09-05 8:35 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <pavel.stehule@gmail.com

wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

In this attached patch, I have minimized the ereport calls. Kindly check
and let me know.

3. test should be enhanced for optional parameters

Fixed it.

only few points:

1. missing to set errstate - any exception should to have some errcode
value. There can be default like PLpgSQL ERRCODE_RAISE_EXCEPTION for any
where elog_level >= error

Fixed It.

it should be a optional parameter,

please fix doc, there are not any difference between mandatory and optional
parametere

2. the explicit setting context is not consistent with our PL - the
context is implicit value only - not possible to set it explicitly. The
behave of this field is not clear - but in this moment, the context is
related to PostgreSQL area - not to application area.

OK. Shall i remove the context field from this function.

ok

Hi Pavel,

On Sat, Sep 5, 2015 at 12:36 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-09-05 8:35 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <
pavel.stehule@gmail.com> wrote:

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

Fixed it.

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

Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.

In this attached patch, I have minimized the ereport calls. Kindly
check and let me know.

3. test should be enhanced for optional parameters

Fixed it.

only few points:

1. missing to set errstate - any exception should to have some errcode
value. There can be default like PLpgSQL ERRCODE_RAISE_EXCEPTION for any
where elog_level >= error

Fixed It.

it should be a optional parameter,

please fix doc, there are not any difference between mandatory and
optional parametere

Fixed this.

2. the explicit setting context is not consistent with our PL - the

context is implicit value only - not possible to set it explicitly. The
behave of this field is not clear - but in this moment, the context is
related to PostgreSQL area - not to application area.

OK. Shall i remove the context field from this function.

ok

Fixed this.

Thanks much.

Regards,
Dinesh
manojadinesh.blogspot.com

Regards

Pavel

Regards,
Dinesh
manojadinesh.blogspot.com

Regards

Pavel

Thanks in advance.

Regards,
Dinesh

Regards

Pavel

Regards,
Dinesh
manojadinesh.blogspot.com

--

Regards,
Dinesh
manojadinesh.blogspot.com

Hi

I am sending little bit modified version.

1. sqlstate should be text, not boolean - a boolean is pretty useless
3. fixed formatting and code style

Questions:

I dislike the using empty message when message parameter is null. We have
to show some text or we have to disallow it?

Regards

Pavel

Hi

attached patch with fixed broken error message

Regards

Pavel

On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

attached patch with fixed broken error message

Regards

Pavel

Hi Pavel,

Thanks much for taking care of it. Patch looks great.

--

Regards,
Dinesh
manojadinesh.blogspot.com

On Sun, Sep 6, 2015 at 4:00 PM, dinesh kumar <dineshkumar02@gmail.com>
wrote:

On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

attached patch with fixed broken error message

Regards

Pavel

Hi Pavel,

Thanks much for taking care of it. Patch looks great.

Hi Pavel,

Could you let me know, what status value i need to change in commitfest's
UI.

--

Regards,
Dinesh
manojadinesh.blogspot.com

--

Regards,
Dinesh
manojadinesh.blogspot.com

2015-09-06 13:12 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Sun, Sep 6, 2015 at 4:00 PM, dinesh kumar <dineshkumar02@gmail.com>
wrote:

On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

attached patch with fixed broken error message

Regards

Pavel

Hi Pavel,

Thanks much for taking care of it. Patch looks great.

Hi Pavel,

Could you let me know, what status value i need to change in commitfest's
UI.

if you have not objections, the status can be "ready for commiter"

Regards

Pavel

On Sun, Sep 6, 2015 at 4:46 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-09-06 13:12 GMT+02:00 dinesh kumar <dineshkumar02@gmail.com>:

On Sun, Sep 6, 2015 at 4:00 PM, dinesh kumar <dineshkumar02@gmail.com>
wrote:

On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

attached patch with fixed broken error message

Regards

Pavel

Hi Pavel,

Thanks much for taking care of it. Patch looks great.

Hi Pavel,

Could you let me know, what status value i need to change in commitfest's
UI.

if you have not objections, the status can be "ready for commiter"

I do not have objections. Let's take this to committers for more inputs.

Thanks again.

Regards

Pavel

--

Regards,
Dinesh
manojadinesh.blogspot.com

--

Regards,
Dinesh
manojadinesh.blogspot.com

--

Regards,
Dinesh
manojadinesh.blogspot.com

On Wed, Jul 22, 2015 at 9:56 PM, dinesh kumar <dineshkumar02@gmail.com> wrote:

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.

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.

But, why?

I just took a look at the latest patch and I can't see why it's any
better than just using PL/pgsql's RAISE statement.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

HI Robert,

On Wed, Sep 9, 2015 at 8:30 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Jul 22, 2015 at 9:56 PM, dinesh kumar <dineshkumar02@gmail.com>
wrote:

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.

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.

But, why?

I am admitting here that, I don’t know the real use case behind this

proposal in our TODO list. I thought, having ereport wrapper at SQL level,
gives a default debugging behavior for the end users, and this is the only
real use case I see.

I just took a look at the latest patch and I can't see why it's any
better than just using PL/pgsql's RAISE statement.

Sure, it’s a clear fact that, we can implement this function with RAISE

statements.

Thanks in advance for your guidance.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--

Regards,
Dinesh
manojadinesh.blogspot.com

On Wed, Sep 9, 2015 at 11:37 AM, dinesh kumar <dineshkumar02@gmail.com> wrote:

I am admitting here that, I don’t know the real use case behind this
proposal in our TODO list. I thought, having ereport wrapper at SQL level,
gives a default debugging behavior for the end users, and this is the only
real use case I see.

...

Sure, it’s a clear fact that, we can implement this function with RAISE
statements.

Given that, I suggest we just forget the whole thing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

On 9/9/15 5:27 PM, Robert Haas wrote:

Sure, it’s a clear fact that, we can implement this function with RAISE

statements.

Given that, I suggest we just forget the whole thing.

Except that you can't use a variable to control the log level in a
plpgsql RAISE, so then you end up with a CASE statement. That perhaps
wouldn't be so bad, until you also want to optionally report detail,
hint, and/or errcode. So trying to create a generic wrapper around RAISE
is decidedly non-trivial. Another option is removing those restrictions
from RAISE, but it seems a bit silly to take the hit of firing up a
plpgsql function for this.

So I think there is value in a SQL equivalent to RAISE. I'm not thrilled
by piling another hack onto the horribly inadequate errlevel
infrastructure, but at least Dinesh's "MESSAGE" idea is essentially just
side-stepping that hole instead of digging it deeper.
--
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

On 2015-09-09 18:27:51 -0400, Robert Haas wrote:

On Wed, Sep 9, 2015 at 11:37 AM, dinesh kumar <dineshkumar02@gmail.com> wrote:

Sure, it’s a clear fact that, we can implement this function with RAISE
statements.

Given that, I suggest we just forget the whole thing.

I'm not convinced. Sure, it's easy, but I by now have written the
respective function dozens of times. Why should we force that on
everyone?

Greetings,

Andres Freund

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

On Thu, Sep 10, 2015 at 01:32:10AM +0200, Andres Freund wrote:

On 2015-09-09 18:27:51 -0400, Robert Haas wrote:

On Wed, Sep 9, 2015 at 11:37 AM, dinesh kumar <dineshkumar02@gmail.com> wrote:

Sure, it’s a clear fact that, we can implement this function
with RAISE statements.

Given that, I suggest we just forget the whole thing.

I'm not convinced. Sure, it's easy, but I by now have written the
respective function dozens of times. Why should we force that on
everyone?

+20 or so here, that being the number of times I recall offhand having
written the function.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
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-10 2:47 GMT+02:00 David Fetter <david@fetter.org>:

On Thu, Sep 10, 2015 at 01:32:10AM +0200, Andres Freund wrote:

On 2015-09-09 18:27:51 -0400, Robert Haas wrote:

On Wed, Sep 9, 2015 at 11:37 AM, dinesh kumar <dineshkumar02@gmail.com>

wrote:

Sure, it’s a clear fact that, we can implement this function
with RAISE statements.

Given that, I suggest we just forget the whole thing.

I'm not convinced. Sure, it's easy, but I by now have written the
respective function dozens of times. Why should we force that on
everyone?

+20 or so here, that being the number of times I recall offhand having
written the function.

I have same opinion - it is pretty often used function.

Pavel

David Fetter <david@fetter.org> writes:

On Thu, Sep 10, 2015 at 01:32:10AM +0200, Andres Freund wrote:

I'm not convinced. Sure, it's easy, but I by now have written the
respective function dozens of times. Why should we force that on
everyone?

+20 or so here, that being the number of times I recall offhand having
written the function.

Were all twenty of them exactly alike? Were they identical to Andres'
several dozen attempts?

The problem I've got with this proposal is that by the time you get to
a function that could satisfy every possible use-case, you do not have
something that is easier to use than "write your own function that
addresses just your use-case".

The only complaint I've seen in this thread that seems like a valid
deficiency is that RAISE can't deal with treating the error severity level
as a variable. But surely we should address that as a new RAISE feature,
not by inventing a SQL wrapper that will need to reproduce every existing
RAISE feature before it can think about solving anything new.

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,

Thanks for your inputs on this.

Here, I see a conflict between the doable{RAISE}, and convenience{SQL
function}, and will follow your inputs on this.

Also, I was under impression that, all our TODO
<https://wiki.postgresql.org/wiki/Todo&gt; items are filtered for the real use
cases. Is my impression wrong. If I wanted to work on another TODO item,
where i need to take a look.

Thanks in advance.

--

Regards,
Dinesh
manojadinesh.blogspot.com

dinesh kumar wrote:

Also, I was under impression that, all our TODO
<https://wiki.postgresql.org/wiki/Todo&gt; items are filtered for the real use
cases. Is my impression wrong. If I wanted to work on another TODO item,
where i need to take a look.

Your impression is completely, absolutely, horribly wrong.

The TODO contains some ideas that are good but postponed, other ideas
that are bad but we didn't know at the time they were recorded, other
ideas that we don't know either way. Before doing anything on an item
from the TODO list, you should first read the linked threads (if any),
and keep track when they end with an email saying "what an awful idea".
If this doesn't happen, _search_ for other threads not linked on the
TODO list that also deal with the same topic; note if they end the same
way (if you find such threads, it's useful to add a link to them in the
TODO item).

Even if you can't find overly negative opinions about some item, discuss
it here before doing any actual coding.

I wonder if we need a new page TONOTDO or something like that.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On Thu, Sep 10, 2015 at 8:44 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:

dinesh kumar wrote:

Also, I was under impression that, all our TODO
<https://wiki.postgresql.org/wiki/Todo&gt; items are filtered for the real

use

cases. Is my impression wrong. If I wanted to work on another TODO item,
where i need to take a look.

Your impression is completely, absolutely, horribly wrong.

:-)

The TODO contains some ideas that are good but postponed, other ideas
that are bad but we didn't know at the time they were recorded, other
ideas that we don't know either way. Before doing anything on an item
from the TODO list, you should first read the linked threads (if any),
and keep track when they end with an email saying "what an awful idea".
If this doesn't happen, _search_ for other threads not linked on the
TODO list that also deal with the same topic; note if they end the same
way (if you find such threads, it's useful to add a link to them in the
TODO item).

Even if you can't find overly negative opinions about some item, discuss

it here before doing any actual coding.

Sure.

I wonder if we need a new page TONOTDO or something like that.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--

Regards,
Dinesh
manojadinesh.blogspot.com

On 2015-09-09 23:45:08 -0400, Tom Lane wrote:

Were all twenty of them exactly alike? Were they identical to Andres'
several dozen attempts?

Mine were pretty much alike and trivial - which is why I never even
bothered to standardize on a variant and store it somewhere.

The problem I've got with this proposal is that by the time you get to
a function that could satisfy every possible use-case, you do not have
something that is easier to use than "write your own function that
addresses just your use-case".

That's a valid concern. I think the answer there is that we shouldn't
design something usable for every use-case, but rather for 90% of the
cases. Which is a tradeof we very frequently make.

The only complaint I've seen in this thread that seems like a valid
deficiency is that RAISE can't deal with treating the error severity level
as a variable. But surely we should address that as a new RAISE feature,
not by inventing a SQL wrapper that will need to reproduce every existing
RAISE feature before it can think about solving anything new.

That seems like something independently useful.

Greetings,

Andres Freund

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

On Thu, Sep 10, 2015 at 8:14 AM, Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:

dinesh kumar wrote:

Also, I was under impression that, all our TODO
<https://wiki.postgresql.org/wiki/Todo&gt; items are filtered for the real

use

cases. Is my impression wrong. If I wanted to work on another TODO item,
where i need to take a look.

Your impression is completely, absolutely, horribly wrong.

The TODO contains some ideas that are good but postponed, other ideas
that are bad but we didn't know at the time they were recorded, other
ideas that we don't know either way. Before doing anything on an item
from the TODO list, you should first read the linked threads (if any),
and keep track when they end with an email saying "what an awful idea".
If this doesn't happen, _search_ for other threads not linked on the
TODO list that also deal with the same topic; note if they end the same
way (if you find such threads, it's useful to add a link to them in the
TODO item).

Even if you can't find overly negative opinions about some item, discuss
it here before doing any actual coding.

I wonder if we need a new page TONOTDO or something like that.

The bottom of the TODO list already has "Features We Do Not Want". I think
it would just be a matter of moving them down to that section.

Or maybe a new section of "Features I lost an argument over, but hope to
eventually accumulate enough allies to re-fight that battle".

I've thought in the paste that maybe we should create a "stale" badge and
go through and add it to every open item. Once someone has re-evaluated
that it is still desirable (and that the description even makes sense in
the first place), they can remove the stale badge. Currently, the TODO
seems to be a collection of belly button lint more than anything else.

Cheers,

Jeff

This patch is marked as Ready for Committer in the CommitFest
application. Here is my attempt to summarize the votes upthread:

Tom Lane: plpgsql RAISE is sufficient; we don't need this.
Pavel Stehule: could be replaced by custom function, but not against it.
Robert Haas: plpgsql RAISE is sufficient; we don't need this.
Jim Nasby: A generic wrapper around RAISE is not trivial, so +1.
Andres Freund: I've written this function many times, so let's have it in core.
David Fetter: I've written this function like 20 times, we should have it.

I'm only -0 on this patch, so I won't yell and scream if some other
committer is prepared to step up and get this committed, but I'm not
excited about doing it myself on the strength of a weak consensus in
which I'm not a participant. Any takers? I recommend that we allow
this patch 30 days to attract an interested committer, and, if nobody
volunteers in that time, that we mark it Rejected for lack of
interest.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

On 9/10/15 10:56 AM, Andres Freund wrote:

The only complaint I've seen in this thread that seems like a valid
deficiency is that RAISE can't deal with treating the error severity level
as a variable. But surely we should address that as a new RAISE feature,
not by inventing a SQL wrapper that will need to reproduce every existing
RAISE feature before it can think about solving anything new.

That seems like something independently useful.

If we're up for that the other thing I'd add is having raise ignore
anything supplied by USING that's NULL, instead of treating it as an
error. That would make it very easy to create a wrapper function that
exposes the full capabilities of RAISE.
--
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-10-16 2:47 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 9/10/15 10:56 AM, Andres Freund wrote:

The only complaint I've seen in this thread that seems like a valid

deficiency is that RAISE can't deal with treating the error severity

level

as a variable. But surely we should address that as a new RAISE

feature,

not by inventing a SQL wrapper that will need to reproduce every

existing

RAISE feature before it can think about solving anything new.

That seems like something independently useful.

fa
If we're up for that the other thing I'd add is having raise ignore
anything supplied by USING that's NULL, instead of treating it as an error.
That would make it very easy to create a wrapper function that exposes the
full capabilities of RAISE.

I don't think so ignoring NULL in RAISE statement is good idea (it is not
safe). We can replace NULL by some string (like "NULL") by default. I am
thinking about other possibilities.

1. some RAISE statement flag - but there was strong disagreement when I did
it last time
2. some plpgsql GUC variables like plpgsq.raise_ignore_null
3. accept a function from this patch

Now, I am thinking so @3 is good option. It can be really useful as last
rescue for other PL without possibility to raise rich PostgreSQL exception
- currently PLPythonu, partially PLPerl (where are more issues), probably
in others.

Regards

Pavel

On 14 October 2015 at 04:01, Robert Haas <robertmhaas@gmail.com> wrote:

This patch is marked as Ready for Committer in the CommitFest
application. Here is my attempt to summarize the votes upthread:

Tom Lane: plpgsql RAISE is sufficient; we don't need this.
Pavel Stehule: could be replaced by custom function, but not against it.
Robert Haas: plpgsql RAISE is sufficient; we don't need this.
Jim Nasby: A generic wrapper around RAISE is not trivial, so +1.
Andres Freund: I've written this function many times, so let's have it in core.
David Fetter: I've written this function like 20 times, we should have it.

Not a committer, so I don't really get a vote here, but I think it's
useful and I've written a "RAISE" wrapper in plpgsql enough times to
think it's useful. I've used it for tracing progress of queries, for
aborting queries on error conditions, all sorts of things.

-1 for any attempt to bypass log_min_messages etc. If we were going to
go there we should do it with proper heirachical logging control, not
a special case half measure.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

On 10/15/15 11:51 PM, Pavel Stehule wrote:

I don't think so ignoring NULL in RAISE statement is good idea (it is
not safe). We can replace NULL by some string (like "NULL") by default.
I am thinking about other possibilities.

What I was trying to say is that if the argument to a USING option is
NULL then RAISE should skip over it, as if it hadn't been applied at
all. Similar to how the code currently tests for \0.

1. some RAISE statement flag - but there was strong disagreement when I
did it last time
2. some plpgsql GUC variables like plpgsq.raise_ignore_null
3. accept a function from this patch

Now, I am thinking so @3 is good option. It can be really useful as last
rescue for other PL without possibility to raise rich PostgreSQL
exception - currently PLPythonu, partially PLPerl (where are more
issues), probably in others.

I agree, assuming the patch exposes all the stuff you can do with USING
in plpgsql.
--
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-10-17 18:42 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 10/15/15 11:51 PM, Pavel Stehule wrote:

I don't think so ignoring NULL in RAISE statement is good idea (it is
not safe). We can replace NULL by some string (like "NULL") by default.
I am thinking about other possibilities.

What I was trying to say is that if the argument to a USING option is NULL
then RAISE should skip over it, as if it hadn't been applied at all.
Similar to how the code currently tests for \0.

I understand, but I don't prefer this behave. The NULL is strange value and
should be signalized.

On 10/17/15 11:49 AM, Pavel Stehule wrote:

2015-10-17 18:42 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com
<mailto:Jim.Nasby@bluetreble.com>>:

On 10/15/15 11:51 PM, Pavel Stehule wrote:

I don't think so ignoring NULL in RAISE statement is good idea
(it is
not safe). We can replace NULL by some string (like "NULL") by
default.
I am thinking about other possibilities.

What I was trying to say is that if the argument to a USING option
is NULL then RAISE should skip over it, as if it hadn't been applied
at all. Similar to how the code currently tests for \0.

I understand, but I don't prefer this behave. The NULL is strange value
and should be signalized.

So instead of raising the message we wanted, we throw a completely
different exception? How does that make sense?

More to the point, if RAISE operated this way then it would be trivial
to create a fully functional plpgsql wrapper around it.
--
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-10-18 21:13 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 10/17/15 11:49 AM, Pavel Stehule wrote:

2015-10-17 18:42 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com
<mailto:Jim.Nasby@bluetreble.com>>:

On 10/15/15 11:51 PM, Pavel Stehule wrote:

I don't think so ignoring NULL in RAISE statement is good idea
(it is
not safe). We can replace NULL by some string (like "NULL") by
default.
I am thinking about other possibilities.

What I was trying to say is that if the argument to a USING option
is NULL then RAISE should skip over it, as if it hadn't been applied
at all. Similar to how the code currently tests for \0.

I understand, but I don't prefer this behave. The NULL is strange value
and should be signalized.

So instead of raising the message we wanted, we throw a completely
different exception? How does that make sense?

It is partially wrong because we handle all fields same. It has sense for
"message" fields, and has not sense for other fields. In this case the text
"NULL" will be better.

More to the point, if RAISE operated this way then it would be trivial to
create a fully functional plpgsql wrapper around it.

I have a different opinion - better to have propossed function in core.
What I know, the NULL is not use in Postgres as "ignore value", and I am
thinking, it is good idea.

Regards

Pavel

Hi

2015-10-13 22:01 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

This patch is marked as Ready for Committer in the CommitFest
application. Here is my attempt to summarize the votes upthread:

Tom Lane: plpgsql RAISE is sufficient; we don't need this.
Pavel Stehule: could be replaced by custom function, but not against it.
Robert Haas: plpgsql RAISE is sufficient; we don't need this.
Jim Nasby: A generic wrapper around RAISE is not trivial, so +1.
Andres Freund: I've written this function many times, so let's have it in
core.
David Fetter: I've written this function like 20 times, we should have it.

I'm only -0 on this patch, so I won't yell and scream if some other
committer is prepared to step up and get this committed, but I'm not
excited about doing it myself on the strength of a weak consensus in
which I'm not a participant. Any takers? I recommend that we allow
this patch 30 days to attract an interested committer, and, if nobody
volunteers in that time, that we mark it Rejected for lack of
interest.

I am changing opinion little bit - the RAISE statement in plpgsql is really
static for some purposes. The proposed function is much more dynamic due
using VARIADIC params. It isn't possible with RAISE statement without some
possible difficult modifications (difficult to find good consensus). The
customized constructor for SPIError can do this work too, but it is not
effective install and to use plpythonu for one functionality. More -
proposed function is pretty simple without side effects - so maintenance of
this function is not high.

Regards

Pavel

On 10/19/15 1:09 AM, Pavel Stehule wrote:

What I was trying to say is that if the argument to a USING
option
is NULL then RAISE should skip over it, as if it hadn't
been applied
at all. Similar to how the code currently tests for \0.

I understand, but I don't prefer this behave. The NULL is
strange value
and should be signalized.

So instead of raising the message we wanted, we throw a completely
different exception? How does that make sense?

It is partially wrong because we handle all fields same. It has sense
for "message" fields, and has not sense for other fields. In this case
the text "NULL" will be better.

I fail to see how doing

HINT: NULL

is much better than just not raising a HINT at all...

More to the point, if RAISE operated this way then it would be
trivial to create a fully functional plpgsql wrapper around it.

I have a different opinion - better to have propossed function in core.
What I know, the NULL is not use in Postgres as "ignore value", and I am
thinking, it is good idea.

Normally I'd agree, but in this case I think it's inline with what the C
code is already doing by testing for \0.

I suppose if we get the function it's not that bad since at least we get
the functionality, so I'll stop arguing it.
--
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

On Mon, Oct 19, 2015 at 7:59 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

I fail to see how doing

HINT: NULL

is much better than just not raising a HINT at all...

I'm not a huge fan of this patch, as previously noted, but I certainly
agree that if we're going to do it, we should ignore a null argument,
not print out the word "NULL". Who would ever want that behavior?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

2015-10-20 16:50 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Mon, Oct 19, 2015 at 7:59 PM, Jim Nasby <Jim.Nasby@bluetreble.com>
wrote:

I fail to see how doing

HINT: NULL

is much better than just not raising a HINT at all...

I'm not a huge fan of this patch, as previously noted, but I certainly
agree that if we're going to do it, we should ignore a null argument,
not print out the word "NULL". Who would ever want that behavior?

Probably it was my request. I don't like to using NULL as value, that
should be ignored. The "hint" is clean, there NULL can be ignored, but what
about DETAIL or MESSAGE?

I am strong in my opinion about PLpgSQL RAISE statement behave, but on
second hand, proposed function should not be 100% same as RAISE stmt. More
we can simply add a parameter like "ignore_nulls"

Regards

Pavel

On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Probably it was my request. I don't like to using NULL as value, that should
be ignored. The "hint" is clean, there NULL can be ignored, but what about
DETAIL or MESSAGE?

If the field is required - as MESSAGE is - then its absence is an
error. If the field is optional, treat a NULL if the parameter were
not supplied.

I am strong in my opinion about PLpgSQL RAISE statement behave, but on
second hand, proposed function should not be 100% same as RAISE stmt. More
we can simply add a parameter like "ignore_nulls"

I would be willing to bet you a drink that 99.9% of people will want
the behavior Jim is advocating, so I don't think this should be
configurable.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

2015-10-20 17:15 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Probably it was my request. I don't like to using NULL as value, that

should

be ignored. The "hint" is clean, there NULL can be ignored, but what

about

DETAIL or MESSAGE?

If the field is required - as MESSAGE is - then its absence is an
error. If the field is optional, treat a NULL if the parameter were
not supplied.

I understand well, what was proposed. Personally I see small risk, but I am
thinking so can be useful if users can choose between two possibilities
(strict, and NULL tolerant). For some adhoc work it can be useful.

I am strong in my opinion about PLpgSQL RAISE statement behave, but on
second hand, proposed function should not be 100% same as RAISE stmt.

More

we can simply add a parameter like "ignore_nulls"

I would be willing to bet you a drink that 99.9% of people will want
the behavior Jim is advocating, so I don't think this should be
configurable.

99.9% of people can think so it is good idea to the moment, when the
important information will be lost without any hint, why it was lost.

Default behave can be like Jim proposed.

On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2015-10-20 17:15 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Probably it was my request. I don't like to using NULL as value, that
should
be ignored. The "hint" is clean, there NULL can be ignored, but what
about
DETAIL or MESSAGE?

If the field is required - as MESSAGE is - then its absence is an
error. If the field is optional, treat a NULL if the parameter were
not supplied.

I understand well, what was proposed. Personally I see small risk, but I am
thinking so can be useful if users can choose between two possibilities
(strict, and NULL tolerant). For some adhoc work it can be useful.

You haven't made any attempt to explain why that behavior would be
useful to anyone except that saying some information might be lost.
But what field of an error report can sensibly be populated with the
word NULL, and nothing else?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

2015-10-20 20:05 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-10-20 17:15 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <

pavel.stehule@gmail.com>

wrote:

Probably it was my request. I don't like to using NULL as value, that
should
be ignored. The "hint" is clean, there NULL can be ignored, but what
about
DETAIL or MESSAGE?

If the field is required - as MESSAGE is - then its absence is an
error. If the field is optional, treat a NULL if the parameter were
not supplied.

I understand well, what was proposed. Personally I see small risk, but I

am

thinking so can be useful if users can choose between two possibilities
(strict, and NULL tolerant). For some adhoc work it can be useful.

You haven't made any attempt to explain why that behavior would be
useful to anyone except that saying some information might be lost.
But what field of an error report can sensibly be populated with the
word NULL, and nothing else?

My previous idea was wrong (I didn't though well about all details). I am
sorry. The implementation of variadic parameters in Postgres requires some
default value - in this case the only one logical default value is NULL.
And in this case, when the default is used, the NULL shouldn't be
displayed. I propose following behave. The level and the message arguments
are mandatory (has not default value), others are optional. The level is
should not be NULL, the message can be NULL, and the NULL should be
displayed, any others are ignored if holds NULL. A alternative is - only
the level will be mandatory, others will be optional, and then there are
not any exception for message.

Regards

Pavel

Hi All,

On Tue, Oct 20, 2015 at 1:22 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-10-20 20:05 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-10-20 17:15 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <

pavel.stehule@gmail.com>

wrote:

Probably it was my request. I don't like to using NULL as value, that
should
be ignored. The "hint" is clean, there NULL can be ignored, but what
about
DETAIL or MESSAGE?

If the field is required - as MESSAGE is - then its absence is an
error. If the field is optional, treat a NULL if the parameter were
not supplied.

I understand well, what was proposed. Personally I see small risk, but

I am

thinking so can be useful if users can choose between two possibilities
(strict, and NULL tolerant). For some adhoc work it can be useful.

You haven't made any attempt to explain why that behavior would be
useful to anyone except that saying some information might be lost.
But what field of an error report can sensibly be populated with the
word NULL, and nothing else?

My previous idea was wrong (I didn't though well about all details). I am
sorry. The implementation of variadic parameters in Postgres requires some
default value - in this case the only one logical default value is NULL.
And in this case, when the default is used, the NULL shouldn't be
displayed. I propose following behave. The level and the message arguments
are mandatory (has not default value), others are optional. The level is
should not be NULL, the message can be NULL, and the NULL should be
displayed, any others are ignored if holds NULL. A alternative is - only
the level will be mandatory, others will be optional, and then there are
not any exception for message.

Thanks for valuable insight inputs.

I just want to be clear about the things from your side,
and want to take further required development from my side.

Let me summarize the issues as below.

1. We need a patch, as per Jim's suggestion about RAISE's USING
should skip any NULL argument, rather throwing an ERROR.
So, we need a new patch if everyone accept this for the RAISE statement.

2. Using this function, if we provide any "NULL" argument to the function,
we should either skip it or report it. I see this is what the function is
doing.

postgres=# SELECT pg_report_log('INFO', 'NULL', false, NULL, NULL);
INFO: NULL

postgres=# SELECT pg_report_log('INFO', 'NULL', false, 'NULL', 'NULL');
INFO: NULL
DETAIL: NULL *-- Are you suggesting to change this behaviour*
HINT: NULL

Kindly let me know your suggestions. Please find the attached patch, which
is generated on top of latest branch head.

Thanks in advance.

--

Regards,
Dinesh
manojadinesh.blogspot.com

On 10/22/15 2:20 AM, dinesh kumar wrote:

2. Using this function, if we provide any "NULL" argument to the function,
we should either skip it or report it. I see this is what the function
is doing.

postgres=# SELECT pg_report_log('INFO', 'NULL', false, NULL, NULL);
INFO: NULL

postgres=# SELECT pg_report_log('INFO', 'NULL', false, 'NULL', 'NULL');
INFO: NULL
DETAIL: NULL /-- Are you suggesting to change this behaviour/
HINT: NULL

It should operate the same as what was decided for RAISE.

I'd say it should also support the remaining RAISE options as well
(COLUMN, CONSTRAINT, DATATYPE, TABLE, SCHEMA).

I think hide_statement is a better name than ishidestmt. It would be
nice if RAISE supported that too...

I think the function should also allow specifying a condition name
instead of a SQL state, same as RAISE does.

In other words, this function and raise should operate exactly the same
unless there's a really strong reason not to. Otherwise it's just going
to create confusion.
--
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

On Thu, Oct 22, 2015 at 11:15 AM, Jim Nasby <Jim.Nasby@bluetreble.com>
wrote:

On 10/22/15 2:20 AM, dinesh kumar wrote:

2. Using this function, if we provide any "NULL" argument to the function,
we should either skip it or report it. I see this is what the function
is doing.

postgres=# SELECT pg_report_log('INFO', 'NULL', false, NULL, NULL);
INFO: NULL

postgres=# SELECT pg_report_log('INFO', 'NULL', false, 'NULL', 'NULL');
INFO: NULL
DETAIL: NULL /-- Are you suggesting to change this behaviour/
HINT: NULL

It should operate the same as what was decided for RAISE.

I'd say it should also support the remaining RAISE options as well
(COLUMN, CONSTRAINT, DATATYPE, TABLE, SCHEMA).

I think hide_statement is a better name than ishidestmt. It would be nice
if RAISE supported that too...

I think the function should also allow specifying a condition name instead
of a SQL state, same as RAISE does.

In other words, this function and raise should operate exactly the same
unless there's a really strong reason not to. Otherwise it's just going to
create confusion.

Thanks Jim,

That make sense to me. Let me cover these options too, and will update here.

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

--

Regards,
Dinesh
manojadinesh.blogspot.com

On 10/22/15 3:20 AM, dinesh kumar wrote:

postgres=# SELECT pg_report_log('INFO', 'NULL', false, NULL, NULL);
INFO: NULL

postgres=# SELECT pg_report_log('INFO', 'NULL', false, 'NULL', 'NULL');
INFO: NULL
DETAIL: NULL /-- Are you suggesting to change this behaviour/
HINT: NULL

These look wrong to me.

I'd throw an error if a null message is passed.

(Not particularly in favor of this patch, but just saying ...)

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

Hi

2015-10-22 22:03 GMT+02:00 Peter Eisentraut <peter_e@gmx.net>:

On 10/22/15 3:20 AM, dinesh kumar wrote:

postgres=# SELECT pg_report_log('INFO', 'NULL', false, NULL, NULL);
INFO: NULL

postgres=# SELECT pg_report_log('INFO', 'NULL', false, 'NULL', 'NULL');
INFO: NULL
DETAIL: NULL /-- Are you suggesting to change this behaviour/
HINT: NULL

These look wrong to me.

I'd throw an error if a null message is passed.

(Not particularly in favor of this patch, but just saying ...)

We talked about this behave - and in this case, I am thinking the any
fields with same value with default value should be ignored.

the behave of pg_report_log should not be exactly same as RAISE statement
in PLpgSQL. If this function will be exactly same, then it lost a sense and
anybody can use RAISE statement. RAISE statement is strict - in this moment
to strict (can be little bit less), and pg_report_log can be NULL tolerant.
It is limmited by our implementation of keyword parameters that needs some
default value.

Regards

Pavel

On 10/22/15 4:42 PM, Pavel Stehule wrote:

the behave of pg_report_log should not be exactly same as RAISE
statement in PLpgSQL.

That makes no sense to me. Having different behaviors is just going to
lead to confusion.

If this function will be exactly same, then it
lost a sense and anybody can use RAISE statement.

It prevents everyone from reinventing the 'create a function wrapper
around RAISE' wheel that several people on this list alone have admitted
to. I think there's plenty of value in that.
--
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-10-22 20:15 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 10/22/15 2:20 AM, dinesh kumar wrote:

2. Using this function, if we provide any "NULL" argument to the function,
we should either skip it or report it. I see this is what the function
is doing.

postgres=# SELECT pg_report_log('INFO', 'NULL', false, NULL, NULL);
INFO: NULL

postgres=# SELECT pg_report_log('INFO', 'NULL', false, 'NULL', 'NULL');
INFO: NULL
DETAIL: NULL /-- Are you suggesting to change this behaviour/
HINT: NULL

It should operate the same as what was decided for RAISE.

I am sorry, there was more opinions - what was decided for RAISE?

I'd say it should also support the remaining RAISE options as well
(COLUMN, CONSTRAINT, DATATYPE, TABLE, SCHEMA).

I think hide_statement is a better name than ishidestmt. It would be nice
if RAISE supported that too...

I think the function should also allow specifying a condition name instead
of a SQL state, same as RAISE does.

In other words, this function and raise should operate exactly the same
unless there's a really strong reason not to. Otherwise it's just going to
create confusion.

I have different opinion - if RAISE and this function is exactly same,then
the function has not sense. There should not be principal difference, but
in same behave I don't see any sense.

2015-10-22 23:54 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 10/22/15 4:42 PM, Pavel Stehule wrote:

the behave of pg_report_log should not be exactly same as RAISE
statement in PLpgSQL.

That makes no sense to me. Having different behaviors is just going to
lead to confusion.

If this function will be exactly same, then it

lost a sense and anybody can use RAISE statement.

It prevents everyone from reinventing the 'create a function wrapper
around RAISE' wheel that several people on this list alone have admitted
to. I think there's plenty of value in that.

I have different opinion, I am sorry. The RAISE statement is differently
designed with different possibility - the function is limited by using
variadic function, and should not to have same behave as RAISE. And I don't
like a idea to push RAISE to behave of variadic function.

Regards

Pavel

On 10/22/15 4:59 PM, Pavel Stehule wrote:

It prevents everyone from reinventing the 'create a function wrapper
around RAISE' wheel that several people on this list alone have
admitted to. I think there's plenty of value in that.

I have different opinion, I am sorry. The RAISE statement is differently
designed with different possibility - the function is limited by using
variadic function, and should not to have same behave as RAISE. And I
don't like a idea to push RAISE to behave of variadic function.

I thought the only issue here was that RAISE currently pukes on a NULL
input, and I thought you'd changed your mind and agreed that it makes
sense for RAISE to just silently ignore anything that's NULL (except
maybe for message). Am I wrong on one or both counts?

IIRC 3 or 4 people on this list liked the idea of a function roughly
equivalent to RAISE, to avoid the make-work of writing that function.
That's why I disagree with your statement that there's no point to this
function even if it acts the same as RAISE.
--
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-10-23 0:07 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

On 10/22/15 4:59 PM, Pavel Stehule wrote:

It prevents everyone from reinventing the 'create a function wrapper
around RAISE' wheel that several people on this list alone have
admitted to. I think there's plenty of value in that.

I have different opinion, I am sorry. The RAISE statement is differently
designed with different possibility - the function is limited by using
variadic function, and should not to have same behave as RAISE. And I
don't like a idea to push RAISE to behave of variadic function.

I thought the only issue here was that RAISE currently pukes on a NULL
input, and I thought you'd changed your mind and agreed that it makes sense
for RAISE to just silently ignore anything that's NULL (except maybe for
message). Am I wrong on one or both counts?

Maybe I don't use some words exactly - but I newer though so RAISE can
ignore NULLs. Current behave of RAISE is probably too strict - the
exception is too hard, but the NULL value should be displayed. In the
function, the NULL can be ignored, because we cannot to do better (we have
not same possibility like Python has) - and I am able to accept it in this
case.

IIRC 3 or 4 people on this list liked the idea of a function roughly
equivalent to RAISE, to avoid the make-work of writing that function.
That's why I disagree with your statement that there's no point to this
function even if it acts the same as RAISE.

I didn't see any strong agreement to change RAISE statement here. The talk
here was about displayed informations - the function should to display all
possible fields - but it is based more on ErrorData structure, than on
RAISE statement.

On 10/22/15 3:20 AM, dinesh kumar wrote:

+      <row>
+       <entry>
+        <literal><function>pg_report_log(<parameter>loglevel</><type>text</>, <parameter>message</> <type>anyelement</>[, <parameter>ishidestmt</> <type>boolean</> ] [, <parameter>detail</> <type> text</>] [, <parameter>hint</> <type>text</>] [, <parameter>sqlstate</> <type>text</>])</function></literal>
+       </entry>
+       <entry><type>void</type></entry>
+       <entry>
+        Report message or error.
+       </entry>
+      </row>

I haven't seen this discussed before, but I don't find the name
pg_report_log particularly good. Why not jut pg_log?

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

On 16 November 2015 at 09:50, Peter Eisentraut <peter_e@gmx.net> wrote:

I haven't seen this discussed before, but I don't find the name
pg_report_log particularly good. Why not jut pg_log?

Sounds like a better name to me. 'report' is noise that adds nothing useful.

I'd like to have this functionality.

I'd prefer to omit fields if explicitly assigned to NULL. You can always
use coalesce if you want the string 'NULL'; I agree with others here that
the vast majority of users will want the field just omitted.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Hi,

On Mon, Nov 16, 2015 at 2:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 10/22/15 3:20 AM, dinesh kumar wrote:

+      <row>
+       <entry>
+

<literal><function>pg_report_log(<parameter>loglevel</><type>text</>,
<parameter>message</> <type>anyelement</>[, <parameter>ishidestmt</>
<type>boolean</> ] [, <parameter>detail</> <type> text</>] [,
<parameter>hint</> <type>text</>] [, <parameter>sqlstate</>
<type>text</>])</function></literal>

+       </entry>
+       <entry><type>void</type></entry>
+       <entry>
+        Report message or error.
+       </entry>
+      </row>

I haven't seen this discussed before, but I don't find the name
pg_report_log particularly good. Why not jut pg_log?

Thanks for your comments.

Sorry for my too late response here.

I'm sure pg_log is good. But, I don't see it's more easily understandable.
What I mean is, If we see "pg_log" as function name, we can't say that,
what this function is going to do by just reading it's name. For example,
we have "pg_write_file". By reading the function name itself, we can define
this, this is the function is for writing contents into given file.

So, shall we make this pg_report_log TO pg_write_log OR pg_ereport OR <SOME
OTHER GOOD SUGGESTIONS> from you.

--

Regards,
Dinesh
manojadinesh.blogspot.com

On Sunday, November 15, 2015 8:51 PM, Craig Ringer <craig@2ndquadrant.com> wrote:

I'd prefer to omit fields if explicitly assigned to NULL. You can
always use coalesce if you want the string 'NULL'; I agree with
others here that the vast majority of users will want the field
just omitted.

+1

Unfortunately those writing the SQL standard chose to have a single
flag (NULL) to indicate either "unknown" or "not applicable". That
causes problems where it's not clear which way the value should be
interpreted, but in this case it seems pretty clear that someone
passing a NULL parameter for hint to a function like this doesn't
mean "there is likely to be a valid value for hint, but I don't
know what it is" -- they mean there is no available hint, so please
don't show one. Any other behavior seems rather silly.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
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, Nov 16, 2015 at 3:58 PM, Kevin Grittner <kgrittn@ymail.com> wrote:

On Sunday, November 15, 2015 8:51 PM, Craig Ringer <craig@2ndquadrant.com>
wrote:

I'd prefer to omit fields if explicitly assigned to NULL. You can
always use coalesce if you want the string 'NULL'; I agree with
others here that the vast majority of users will want the field
just omitted.

+1

Unfortunately those writing the SQL standard chose to have a single
flag (NULL) to indicate either "unknown" or "not applicable". That
causes problems where it's not clear which way the value should be
interpreted, but in this case it seems pretty clear that someone
passing a NULL parameter for hint to a function like this doesn't
mean "there is likely to be a valid value for hint, but I don't
know what it is" -- they mean there is no available hint, so please
don't show one. Any other behavior seems rather silly.

Thanks Kevin/Craig for your comments.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--

Regards,
Dinesh
manojadinesh.blogspot.com

On 11/15/15 9:50 PM, Craig Ringer wrote:

On 16 November 2015 at 09:50, Peter Eisentraut <peter_e@gmx.net
<mailto:peter_e@gmx.net>> wrote:

I haven't seen this discussed before, but I don't find the name
pg_report_log particularly good. Why not jut pg_log?

Sounds like a better name to me. 'report' is noise that adds nothing useful.

I'd like to have this functionality.

I'd prefer to omit fields if explicitly assigned to NULL. You can always
use coalesce if you want the string 'NULL'; I agree with others here
that the vast majority of users will want the field just omitted.

I think the problem was that you can't omit the primary message.

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

Peter Eisentraut <peter_e@gmx.net> writes:

On 11/15/15 9:50 PM, Craig Ringer wrote:

I'd prefer to omit fields if explicitly assigned to NULL. You can always
use coalesce if you want the string 'NULL'; I agree with others here
that the vast majority of users will want the field just omitted.

I think the problem was that you can't omit the primary message.

If you ask me, that would be a feature not a bug.

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

On 11/16/15 5:10 PM, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On 11/15/15 9:50 PM, Craig Ringer wrote:

I'd prefer to omit fields if explicitly assigned to NULL. You can always
use coalesce if you want the string 'NULL'; I agree with others here
that the vast majority of users will want the field just omitted.

I think the problem was that you can't omit the primary message.

If you ask me, that would be a feature not a bug.

Right.

Frankly, I have lost track of what the problem here is.

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

On 11/15/15 10:56 PM, dinesh kumar wrote:

So, shall we make this pg_report_log TO pg_write_log OR pg_ereport OR
<SOME OTHER GOOD SUGGESTIONS> from you.

Why not pg_raise to mirror plpgsql? (The function does have the same
semantics, right? It's not doing something like only sending to the log
and not the client?)
--
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

On 11/17/15 2:16 AM, Jim Nasby wrote:

On 11/15/15 10:56 PM, dinesh kumar wrote:

So, shall we make this pg_report_log TO pg_write_log OR pg_ereport OR
<SOME OTHER GOOD SUGGESTIONS> from you.

Why not pg_raise to mirror plpgsql? (The function does have the same
semantics, right? It's not doing something like only sending to the log
and not the client?)

I think the "raise" terminology is specific to plpgsql, as it actually
raises an exception in that language.

--
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, Nov 16, 2015 at 5:41 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 11/16/15 5:10 PM, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On 11/15/15 9:50 PM, Craig Ringer wrote:

I'd prefer to omit fields if explicitly assigned to NULL. You can always
use coalesce if you want the string 'NULL'; I agree with others here
that the vast majority of users will want the field just omitted.

I think the problem was that you can't omit the primary message.

If you ask me, that would be a feature not a bug.

Right.

Frankly, I have lost track of what the problem here is.

I am still of the opinion that this patch is a solution in search of a problem.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
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 Tue, Nov 17, 2015 at 12:10 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 11/17/15 2:16 AM, Jim Nasby wrote:

On 11/15/15 10:56 PM, dinesh kumar wrote:

So, shall we make this pg_report_log TO pg_write_log OR pg_ereport OR
<SOME OTHER GOOD SUGGESTIONS> from you.

Why not pg_raise to mirror plpgsql? (The function does have the same
semantics, right? It's not doing something like only sending to the log
and not the client?)

I think the "raise" terminology is specific to plpgsql, as it actually
raises an exception in that language.

Sorry for being too late on this, as I have engaged into some other
personal tasks.

Could someone let me know, what else I need to do to get this patch
completed.

Any further suggestions on function name. If all OK with pg_log or
someother, I would modify the patch,
and will submit new one.

Kindly let me know.

Thanks in advance.

--

Regards,
Dinesh
manojadinesh.blogspot.com