Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-23 Thread dinesh kumar
Hi All,

On Tue, Nov 17, 2015 at 12:10 PM, Peter Eisentraut  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
> >>  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-18 Thread Robert Haas
On Mon, Nov 16, 2015 at 5:41 PM, Peter Eisentraut  wrote:
> On 11/16/15 5:10 PM, Tom Lane wrote:
>> Peter Eisentraut  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-17 Thread Peter Eisentraut
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
>>  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-16 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-16 Thread Peter Eisentraut
On 11/16/15 5:10 PM, Tom Lane wrote:
> Peter Eisentraut  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-16 Thread Tom Lane
Peter Eisentraut  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-16 Thread Peter Eisentraut
On 11/15/15 9:50 PM, Craig Ringer wrote:
> On 16 November 2015 at 09:50, Peter Eisentraut  > 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-16 Thread dinesh kumar
On Mon, Nov 16, 2015 at 3:58 PM, Kevin Grittner  wrote:

> On Sunday, November 15, 2015 8:51 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.
>
> +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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-16 Thread Kevin Grittner
On Sunday, November 15, 2015 8:51 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.

+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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-15 Thread dinesh kumar
Hi,

On Mon, Nov 16, 2015 at 2:50 AM, Peter Eisentraut  wrote:

> On 10/22/15 3:20 AM, dinesh kumar wrote:
> > +  
> > +   
> > +
> pg_report_log(logleveltext,
> message anyelement[, ishidestmt
> boolean ] [, detail  text] [,
> hint text] [, sqlstate
> text])
> > +   
> > +   void
> > +   
> > +Report message or error.
> > +   
> > +  
>
> 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  from you.


-- 

Regards,
Dinesh
manojadinesh.blogspot.com


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-15 Thread Craig Ringer
On 16 November 2015 at 09:50, Peter Eisentraut  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-11-15 Thread Peter Eisentraut
On 10/22/15 3:20 AM, dinesh kumar wrote:
> +  
> +   
> +
> pg_report_log(logleveltext, 
> message anyelement[, ishidestmt 
> boolean ] [, detail  text] [, 
> hint text] [, sqlstate 
> text])
> +   
> +   void
> +   
> +Report message or error.
> +   
> +  

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Pavel Stehule
2015-10-23 0:07 GMT+02:00 Jim Nasby :

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



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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Pavel Stehule
2015-10-22 23:54 GMT+02:00 Jim Nasby :

> 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




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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Pavel Stehule
2015-10-22 20:15 GMT+02:00 Jim Nasby :

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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Pavel Stehule
Hi

2015-10-22 22:03 GMT+02:00 Peter Eisentraut :

> 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Peter Eisentraut
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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread dinesh kumar
On Thu, Oct 22, 2015 at 11:15 AM, Jim Nasby 
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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-22 Thread dinesh kumar
Hi All,

On Tue, Oct 20, 2015 at 1:22 PM, Pavel Stehule 
wrote:

>
> 2015-10-20 20:05 GMT+02:00 Robert Haas :
>
>> On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule 
>> wrote:
>> > 2015-10-20 17:15 GMT+02:00 Robert Haas :
>> >> On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <
>> pavel.steh...@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
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 2946122..8deb679 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17966,6 +17966,15 @@ postgres=# SELECT * FROM 
pg_xlogfile_name_offset(pg_stop_backup());
 Return information about a file.

   
+  
+   
+pg_report_log(logleveltext, 
message anyelement[, ishidestmt 
boolean ] [, detail  text] [, 
hint text] [, sqlstate 
text])
+   
+   void
+   
+Report message or error.
+   
+  
  
 

@@ -18034,6 +18043,32 @@ SELECT (pg_stat_file('filename')).modification;
 

 
+   
+pg_report_log
+   
+   
+pg_report_log is useful to write custom messages
+or raise exception. This function don't support the PANIC, FATAL
+log levels due to their unique internal DB usage, which may cause
+the database instability. Using ishidestmt which default 
values
+is true, function can write or ignore the current SQL statement
+into log destination. Also, we can have DETAIL, HINT log messages
+by provding detail, hint as function
+arguments, which are NULL by default. The parameter sqlstate
+allows to set a SQLSTATE of raised exception. Default value of this
+parameter is 'P0001' for ERROR only level.
+
+Typical usages include:
+
+postgres=# SELECT pg_report_log('NOTICE', 'Custom Message', true);
+NOTICE:  Custom Message
+ pg_report_log 
+---
+ 
+(1 row)
+
+   
+
   
 
   
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index ccc030f..7e551f2 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -940,3 +940,22 @@ RETURNS jsonb
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'jsonb_set';
+
+CREATE OR REPLACE FUNCTION pg_report_log(loglevel text, message text,
+ ishidestmt boolean DEFAULT true, 
detail text DEFAULT NULL,
+ hint text DEFAULT NULL, sqlstate text 
DEFAULT 'P0001')
+RETURNS VOID
+LANGUAGE INTERNAL
+VOLATILE
+AS 'pg_report_log';

Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-20 Thread Pavel Stehule
2015-10-20 20:05 GMT+02:00 Robert Haas :

> On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule 
> wrote:
> > 2015-10-20 17:15 GMT+02:00 Robert Haas :
> >> On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule <
> pavel.steh...@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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-20 Thread Robert Haas
On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule  wrote:
> 2015-10-20 17:15 GMT+02:00 Robert Haas :
>> On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule 
>> 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-20 Thread Pavel Stehule
2015-10-20 17:15 GMT+02:00 Robert Haas :

> On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule 
> 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.




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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-20 Thread Robert Haas
On Tue, Oct 20, 2015 at 11:09 AM, Pavel Stehule  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-20 Thread Pavel Stehule
2015-10-20 16:50 GMT+02:00 Robert Haas :

> On Mon, Oct 19, 2015 at 7:59 PM, Jim Nasby 
> 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



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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-20 Thread Robert Haas
On Mon, Oct 19, 2015 at 7:59 PM, Jim Nasby  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-19 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-19 Thread Pavel Stehule
Hi

2015-10-13 22:01 GMT+02:00 Robert Haas :

> 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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-18 Thread Pavel Stehule
2015-10-18 21:13 GMT+02:00 Jim Nasby :

> On 10/17/15 11:49 AM, Pavel Stehule wrote:
>
>> 2015-10-17 18:42 GMT+02:00 Jim Nasby > >:
>>
>> 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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-18 Thread Jim Nasby

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

2015-10-17 18:42 GMT+02:00 Jim Nasby mailto:jim.na...@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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-17 Thread Pavel Stehule
2015-10-17 18:42 GMT+02:00 Jim Nasby :

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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-17 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-15 Thread Craig Ringer
On 14 October 2015 at 04:01, Robert Haas  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-15 Thread Pavel Stehule
2015-10-16 2:47 GMT+02:00 Jim Nasby :

> 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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-15 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-10-13 Thread Robert Haas
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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-10 Thread Jeff Janes
On Thu, Sep 10, 2015 at 8:14 AM, Alvaro Herrera 
wrote:

> dinesh kumar wrote:
>
> > Also, I was under impression that, all our TODO
> >  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-10 Thread Andres Freund
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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-10 Thread dinesh kumar
On Thu, Sep 10, 2015 at 8:44 PM, Alvaro Herrera 
wrote:

> dinesh kumar wrote:
>
> > Also, I was under impression that, all our TODO
> >  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 Herrerahttp://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>



-- 

Regards,
Dinesh
manojadinesh.blogspot.com


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-10 Thread Alvaro Herrera
dinesh kumar wrote:

> Also, I was under impression that, all our TODO
>  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 Herrerahttp://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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-10 Thread dinesh kumar
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
 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread Tom Lane
David Fetter  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread Pavel Stehule
2015-09-10 2:47 GMT+02:00 David Fetter :

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


>
> Cheers,
> David.
> --
> David Fetter  http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter  XMPP: david.fet...@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
>


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread David Fetter
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  
> > 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  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread Andres Freund
On 2015-09-09 18:27:51 -0400, Robert Haas wrote:
> On Wed, Sep 9, 2015 at 11:37 AM, dinesh kumar  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread Robert Haas
On Wed, Sep 9, 2015 at 11:37 AM, dinesh kumar  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread dinesh kumar
HI Robert,

On Wed, Sep 9, 2015 at 8:30 PM, Robert Haas  wrote:

> On Wed, Jul 22, 2015 at 9:56 PM, dinesh kumar 
> 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-09 Thread Robert Haas
On Wed, Jul 22, 2015 at 9:56 PM, dinesh kumar  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-06 Thread dinesh kumar
On Sun, Sep 6, 2015 at 4:46 PM, Pavel Stehule 
wrote:

>
>
> 2015-09-06 13:12 GMT+02:00 dinesh kumar :
>
>>
>> On Sun, Sep 6, 2015 at 4:00 PM, dinesh kumar 
>> wrote:
>>
>>> On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule 
>>> 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-06 Thread Pavel Stehule
2015-09-06 13:12 GMT+02:00 dinesh kumar :

>
> On Sun, Sep 6, 2015 at 4:00 PM, dinesh kumar 
> wrote:
>
>> On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule 
>> 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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-06 Thread dinesh kumar
On Sun, Sep 6, 2015 at 4:00 PM, dinesh kumar 
wrote:

> On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule 
> 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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-06 Thread dinesh kumar
On Sun, Sep 6, 2015 at 3:39 PM, Pavel Stehule 
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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-06 Thread Pavel Stehule
Hi

attached patch with fixed broken error message

Regards

Pavel
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index b3b78d2..b7a2cc2
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** postgres=# SELECT * FROM pg_xlogfile_nam
*** 17925,17930 
--- 17925,17939 
  Return information about a file.
 

+   
+
+ pg_report_log(logleveltext, 
message anyelement[, ishidestmt 
boolean ] [, detail  text] [, 
hint text] [, sqlstate 
text])
+
+void
+
+ Report message or error.
+
+   
   
  
 
*** SELECT (pg_stat_file('filename')).modifi
*** 17993,17998 
--- 18002,18033 
  
 
  
+
+ pg_report_log
+
+
+ pg_report_log is useful to write custom messages
+ or raise exception. This function don't support the PANIC, FATAL
+ log levels due to their unique internal DB usage, which may cause
+ the database instability. Using ishidestmt which default 
values
+ is true, function can write or ignore the current SQL statement
+ into log destination. Also, we can have DETAIL, HINT log messages
+ by provding detail, hint as function
+ arguments, which are NULL by default. The parameter sqlstate
+ allows to set a SQLSTATE of raised exception. Default value of this
+ parameter is 'P0001' for ERROR only level.
+ 
+ Typical usages include:
+ 
+ postgres=# SELECT pg_report_log('NOTICE', 'Custom Message', true);
+ NOTICE:  Custom Message
+  pg_report_log 
+ ---
+  
+ (1 row)
+ 
+
+ 

  

diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
new file mode 100644
index ccc030f..7e551f2
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*** RETURNS jsonb
*** 940,942 
--- 940,961 
  LANGUAGE INTERNAL
  STRICT IMMUTABLE
  AS 'jsonb_set';
+ 
+ CREATE OR REPLACE FUNCTION pg_report_log(loglevel text, message text,
+  ishidestmt boolean DEFAULT true, 
detail text DEFAULT NULL,
+  hint text DEFAULT NULL, sqlstate 
text DEFAULT 'P0001')
+ RETURNS VOID
+ LANGUAGE INTERNAL
+ VOLATILE
+ AS 'pg_report_log';
+ 
+ CREATE OR REPLACE FUNCTION pg_report_log(loglevel text, message anyelement,
+  ishidestmt boolean DEFAULT true, 
detail text DEFAULT NULL,
+  hint text DEFAULT NULL, sqlstate 
text DEFAULT 'P0001')
+ RETURNS VOID
+ VOLATILE
+ AS
+ $$
+ SELECT pg_report_log($1::pg_catalog.text, $2::pg_catalog.text, $3, $4, $5, $6)
+ $$
+ LANGUAGE SQL;
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
new file mode 100644
index c0495d9..b1275bd
*** a/src/backend/utils/adt/misc.c
--- b/src/backend/utils/adt/misc.c
*** current_query(PG_FUNCTION_ARGS)
*** 75,80 
--- 75,212 
PG_RETURN_NULL();
  }
  
+ 
+ /*
+  * Parsing error levels
+  */
+ typedef struct
+ {
+   char *err_level;
+   int  ival;
+ } error_levels;
+ 
+ /*
+  * Translate text based elog level to integer value.
+  *
+  * Returns true, when it found known elog elevel else
+  * returns false;
+  */
+ static bool
+ parse_error_level(const char* err_level, int *ival)
+ {
+   error_levels err_levels[]={
+   {"DEBUG5", DEBUG5},
+   {"DEBUG4", DEBUG4},
+   {"DEBUG3", DEBUG3},
+   {"DEBUG2", DEBUG2},
+   {"DEBUG1", DEBUG1},
+   {"LOG", LOG},
+   {"INFO", INFO}, 
+   {"NOTICE", NOTICE},
+   {"WARNING", WARNING},
+   {"ERROR", ERROR},
+   /*
+* Adding PGERROR to elevels if WIN32
+*/
+   #ifdef WIN32
+   {"PGERROR", PGERROR},
+   #endif
+   {NULL, 0}
+   };
+ 
+   error_levels *current;
+ 
+   for (current = err_levels; current->err_level != NULL; current++)
+   {
+   if (pg_strcasecmp(current->err_level, err_level) == 0)
+   {
+   *ival = current->ival;
+ 
+   return true;
+   }
+   }
+ 
+   return false;
+ }
+ 
+ /*
+  * pg_report_log()
+  *
+  * Printing custom log messages in log file.
+  */
+ Datum
+ pg_report_log(PG_FUNCTION_ARGS)
+ {
+   int  elog_level;
+   char *elog_level_str;
+   int  sqlstate = 0;
+   char *sqlstate_str;
+   bool ishidestmt = false;
+   char *err_message = NULL;
+   char *err_detail = NULL;
+   char *err_hint = NULL;
+ 
+   /* log level */
+   if (PG_ARGISNULL(0))
+   ereport(ERROR,
+   (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+   

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-06 Thread Pavel Stehule
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
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index b3b78d2..c0b6a72
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** postgres=# SELECT * FROM pg_xlogfile_nam
*** 17925,17930 
--- 17925,17939 
  Return information about a file.
 

+   
+
+ pg_report_log(logleveltext, 
message anyelement[, ishidestmt 
boolean ] [, detail  text] [, 
hint text] [, sqlstate 
text])
+
+void
+
+ Report message or error.
+
+   
   
  
 
*** SELECT (pg_stat_file('filename')).modifi
*** 17993,17998 
--- 18002,18026 
  
 
  
+
+ pg_report_log
+
+
+ pg_report_log is useful to write custom messages
+ into current log destination and returns void.
+ This function don't support the PANIC, FATAL log levels due to their 
unique internal DB usage, which may cause the database instability. Using 
ishidestmt which default values is true, function can write or 
ignore the current SQL statement into log destination. Also, we can have 
DETAIL, HINT log messages by provding detail, hint 
as function arguments, which are NULL by default. Using 
iserrstate which default values is true, enables the function to 
raise the SQLSTATE as ERRCODE_RAISE_EXCEPTION for the only ERROR level.
+ 
+ Typical usages include:
+ 
+ postgres=# SELECT pg_report_log('NOTICE', 'Custom Message', true);
+ NOTICE:  Custom Message
+  pg_report_log 
+ ---
+  
+ (1 row)
+ 
+
+ 

  

diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
new file mode 100644
index ccc030f..1755335
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*** RETURNS jsonb
*** 940,942 
--- 940,961 
  LANGUAGE INTERNAL
  STRICT IMMUTABLE
  AS 'jsonb_set';
+ 
+ CREATE OR REPLACE FUNCTION pg_report_log(loglevel text, message text,
+  ishidestmt boolean DEFAULT true, 
detail text DEFAULT NULL,
+  hint text DEFAULT NULL, sqlstate 
text DEFAULT NULL)
+ RETURNS VOID
+ LANGUAGE INTERNAL
+ VOLATILE
+ AS 'pg_report_log';
+ 
+ CREATE OR REPLACE FUNCTION pg_report_log(loglevel text, message anyelement,
+  ishidestmt boolean DEFAULT true, 
detail text DEFAULT NULL,
+  hint text DEFAULT NULL, sqlstate 
text DEFAULT NULL)
+ RETURNS VOID
+ VOLATILE
+ AS
+ $$
+ SELECT pg_report_log($1::pg_catalog.text, $2::pg_catalog.text, $3, $4, $5, $6)
+ $$
+ LANGUAGE SQL;
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
new file mode 100644
index c0495d9..fd65aae
*** a/src/backend/utils/adt/misc.c
--- b/src/backend/utils/adt/misc.c
*** current_query(PG_FUNCTION_ARGS)
*** 75,80 
--- 75,210 
PG_RETURN_NULL();
  }
  
+ 
+ /*
+  * Parsing error levels
+  */
+ typedef struct
+ {
+   char *err_level;
+   int  ival;
+ } error_levels;
+ 
+ /*
+  * Translate text based elog level to integer value.
+  *
+  * Returns true, when it found known elog elevel else
+  * returns false;
+  */
+ static bool
+ parse_error_level(const char* err_level, int *ival)
+ {
+   error_levels err_levels[]={
+   {"DEBUG5", DEBUG5},
+   {"DEBUG4", DEBUG4},
+   {"DEBUG3", DEBUG3},
+   {"DEBUG2", DEBUG2},
+   {"DEBUG1", DEBUG1},
+   {"LOG", LOG},
+   {"INFO", INFO}, 
+   {"NOTICE", NOTICE},
+   {"WARNING", WARNING},
+   {"ERROR", ERROR},
+   /*
+* Adding PGERROR to elevels if WIN32
+*/
+   #ifdef WIN32
+   {"PGERROR", PGERROR},
+   #endif
+   {NULL, 0}
+   };
+ 
+   error_levels *current;
+ 
+   for (current = err_levels; current->err_level != NULL; current++)
+   {
+   if (pg_strcasecmp(current->err_level, err_level) == 0)
+   {
+   *ival = current->ival;
+ 
+   return true;
+   }
+   }
+ 
+   return false;
+ }
+ 
+ /*
+  * pg_report_log()
+  *
+  * Printing custom log messages in log file.
+  */
+ Datum
+ pg_report_log(PG_FUNCTION_ARGS)
+ {
+   int  elog_level;
+   char *elog_level_str;
+   int  sqlstate;
+   char *sqlstate_str;
+   bool ishidestmt = false;
+   char *err_message = NULL;
+   char *err_

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-05 Thread dinesh kumar
Hi Pavel,

On Sat, Sep 5, 2015 at 12:36 AM, Pavel Stehule 
wrote:

>
>
> 2015-09-05 8:35 GMT+02:00 dinesh kumar :
>
>>
>>
>> On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule 
>> wrote:
>>
>>>
>>>
>>> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>>>
 On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <
 pavel.steh...@gmail.com> wrote:

>
>
> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>
>>
>>
>> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>>
>>> Hi,
>>>
>>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
>>> pavel.steh...@gmail.com> wrote:
>>>
 Hi

 I am starting to work review of this patch

 2015-07-13 9:54 GMT+02:00 dinesh kumar :

> 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 

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-05 Thread Pavel Stehule
2015-09-05 8:35 GMT+02:00 dinesh kumar :

>
>
> On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule 
> wrote:
>
>>
>>
>> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>>
>>> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule >> > wrote:
>>>


 2015-09-01 6:59 GMT+02:00 Pavel Stehule :

>
>
> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>
>> Hi,
>>
>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
>> pavel.steh...@gmail.com> wrote:
>>
>>> Hi
>>>
>>> I am starting to work review of this patch
>>>
>>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>>
 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 par

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-04 Thread dinesh kumar
On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule 
wrote:

>
>
> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>
>> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule 
>> wrote:
>>
>>>
>>>
>>> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>>>


 2015-08-31 20:43 GMT+02:00 dinesh kumar :

> Hi,
>
> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
> pavel.steh...@gmail.com> wrote:
>
>> Hi
>>
>> I am starting to work review of this patch
>>
>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>
>>> 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 NU

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-04 Thread dinesh kumar
On Fri, Sep 4, 2015 at 1:08 PM, Pavel Stehule 
wrote:

>
>
> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>
>> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule 
>> wrote:
>>
>>>
>>>
>>> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>>>


 2015-08-31 20:43 GMT+02:00 dinesh kumar :

> Hi,
>
> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
> pavel.steh...@gmail.com> wrote:
>
>> Hi
>>
>> I am starting to work review of this patch
>>
>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>
>>> 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 NU

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-04 Thread Pavel Stehule
2015-09-02 21:49 GMT+02:00 dinesh kumar :

> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule 
> wrote:
>
>>
>>
>> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>>
>>>
>>>
>>> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>>>
 Hi,

 On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule >>> > wrote:

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

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-04 Thread Pavel Stehule
2015-09-02 21:49 GMT+02:00 dinesh kumar :

> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule 
> wrote:
>
>>
>>
>> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>>
>>>
>>>
>>> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>>>
 Hi,

 On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule >>> > wrote:

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

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-02 Thread dinesh kumar
On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule 
wrote:

>
>
> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>
>>
>>
>> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>>
>>> Hi,
>>>
>>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule 
>>> wrote:
>>>
 Hi

 I am starting to work review of this patch

 2015-07-13 9:54 GMT+02:00 dinesh kumar :

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

Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-02 Thread Robert Haas
On Tue, Sep 1, 2015 at 6:13 PM, Jim Nasby  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-01 Thread Pavel Stehule
2015-09-02 0:13 GMT+02:00 Jim Nasby :

> 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



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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-09-01 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-08-31 Thread Pavel Stehule
2015-09-01 7:20 GMT+02:00 Jim Nasby :

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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-08-31 Thread Jim Nasby

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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-08-31 Thread Pavel Stehule
2015-09-01 6:59 GMT+02:00 Pavel Stehule :

>
>
> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>
>> Hi,
>>
>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule 
>> wrote:
>>
>>> Hi
>>>
>>> I am starting to work review of this patch
>>>
>>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>>
 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


>
>> Thanks in advance.
>>
>> Regards,
>> Dinesh
>>
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>>

 Regards,
 Dinesh
 manojadinesh.blogspot.com

>>>
>>>
>>
>


Re: [HACKERS] [PATCH] SQL function to report log message

2015-08-31 Thread Pavel Stehule
2015-08-31 20:43 GMT+02:00 dinesh kumar :

> Hi,
>
> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule 
> wrote:
>
>> Hi
>>
>> I am starting to work review of this patch
>>
>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>
>>> Hi All,
>>>
>>> Greetings for the day.
>>>
>>> Would like to discuss on below feature here.
>>>
>>> Feature:
>>> Having an SQL function, to write messages to log destination.
>>>
>>> Justification:
>>> As of now, we don't have an SQL function to write custom/application
>>> messages to log destination. We have "RAISE" clause which is controlled by
>>> log_ parameters. If we have an SQL function which works irrespective of
>>> log settings, that would be a good for many log parsers. What i mean is, in
>>> DBA point of view, if we route all our native OS stats to log files in a
>>> proper format, then we can have our log reporting tools to give most
>>> effective reports. Also, Applications can log their own messages to
>>> postgres log files, which can be monitored by DBAs too.
>>>
>>> Implementation:
>>> Implemented a new function "pg_report_log" which takes one argument
>>> as text, and returns void. I took, "LOG" prefix for all the reporting
>>> messages.I wasn't sure to go with new prefix for this, since these are
>>> normal LOG messages. Let me know, if i am wrong here.
>>>
>>> Here is the attached patch.
>>>
>>
>> This patch is not complex, but the implementation doesn't cover a
>> "ereport" well.
>>
>> Although this functionality should be replaced by custom function in any
>> PL (now or near future), I am not against to have this function in core.
>> There are lot of companies with strong resistance against stored procedures
>> - and sometimes this functionality can help with SQL debugging.
>>
>> Issues:
>>
>> 1. Support only MESSAGE field in exception - I am expecting to support
>> all fields: HINT, DETAIL, ...
>>
>
> Added these functionalities too.
>
>
>> 2. Missing regress tests
>>
>
> Adding here.
>
>
>> 3. the parsing ereport level should be public function shared with
>> PLpgSQL and other PL
>>
>
> Sorry Pavel. I am not getting your point here. Would you give me an
> example.
>

The transformation: text -> error level is common task - and PLpgSQL it
does in pl_gram.y. My idea is to add new function to error utils named
"parse_error_level" and use it from PLpgSQL and from your code.


>
>
>> 4. should be hidestmt mandatory parameter?
>>
>
> I changed this argument's default value as "true".
>
>
>> 5. the function declaration is strange
>>
>> postgres=# \sf pg_report_log (text, anyelement, boolean)
>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, anyelement,
>> boolean)
>>  RETURNS void
>>  LANGUAGE sql
>>  STABLE STRICT COST 1
>> AS $function$SELECT pg_report_log($1::pg_catalog.text,
>> $2::pg_catalog.text, $3::boolean)$function$
>>
>> Why polymorphic? It is useless on any modern release
>>
>>
> I took quote_ident(anyelement) as referral code, for implementing this.
> Could you guide me if I am doing wrong here.
>

I was wrong - this is ok - it is emulation of force casting to text


>
>
>> postgres=# \sf pg_report_log (text, text, boolean)
>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text, boolean)
>>  RETURNS void
>>  LANGUAGE internal
>>  IMMUTABLE STRICT
>> AS $function$pg_report_log$function$
>>
>> Why stable, why immutable? This function should be volatile.
>>
>> Fixed these to volatile.
>
>
>> 6. using elog level enum as errcode is wrong idea - errcodes are defined
>> in table http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html
>>
>
> You mean, if the elevel is 'ERROR', then we need to allow errcode. Let me
> know your inputs.
>

I was blind, but the code was not good. Yes, error and higher needs error
code. From ANSI/SQL anything can has error code "00 is ok", "01 ..
warnings" ...

There is more possibilities - look to PLpgSQL implementation - it can be
optional parameter - it default can use ERRCODE_RAISE_EXCEPTION


>
> Adding new patch, with the above fixes.
>
> Thanks in advance.
>
> Regards,
> Dinesh
>
>>
>> Regards
>>
>> Pavel
>>
>>
>>>
>>> Regards,
>>> Dinesh
>>> manojadinesh.blogspot.com
>>>
>>
>>
>


Re: [HACKERS] [PATCH] SQL function to report log message

2015-08-31 Thread dinesh kumar
Hi,

On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule 
wrote:

> Hi
>
> I am starting to work review of this patch
>
> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>
>> Hi All,
>>
>> Greetings for the day.
>>
>> Would like to discuss on below feature here.
>>
>> Feature:
>> Having an SQL function, to write messages to log destination.
>>
>> Justification:
>> As of now, we don't have an SQL function to write custom/application
>> messages to log destination. We have "RAISE" clause which is controlled by
>> log_ parameters. If we have an SQL function which works irrespective of
>> log settings, that would be a good for many log parsers. What i mean is, in
>> DBA point of view, if we route all our native OS stats to log files in a
>> proper format, then we can have our log reporting tools to give most
>> effective reports. Also, Applications can log their own messages to
>> postgres log files, which can be monitored by DBAs too.
>>
>> Implementation:
>> Implemented a new function "pg_report_log" which takes one argument
>> as text, and returns void. I took, "LOG" prefix for all the reporting
>> messages.I wasn't sure to go with new prefix for this, since these are
>> normal LOG messages. Let me know, if i am wrong here.
>>
>> Here is the attached patch.
>>
>
> This patch is not complex, but the implementation doesn't cover a
> "ereport" well.
>
> Although this functionality should be replaced by custom function in any
> PL (now or near future), I am not against to have this function in core.
> There are lot of companies with strong resistance against stored procedures
> - and sometimes this functionality can help with SQL debugging.
>
> Issues:
>
> 1. Support only MESSAGE field in exception - I am expecting to support all
> fields: HINT, DETAIL, ...
>

Added these functionalities too.


> 2. Missing regress tests
>

Adding here.


> 3. the parsing ereport level should be public function shared with PLpgSQL
> and other PL
>

Sorry Pavel. I am not getting your point here. Would you give me an example.


> 4. should be hidestmt mandatory parameter?
>

I changed this argument's default value as "true".


> 5. the function declaration is strange
>
> postgres=# \sf pg_report_log (text, anyelement, boolean)
> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, anyelement,
> boolean)
>  RETURNS void
>  LANGUAGE sql
>  STABLE STRICT COST 1
> AS $function$SELECT pg_report_log($1::pg_catalog.text,
> $2::pg_catalog.text, $3::boolean)$function$
>
> Why polymorphic? It is useless on any modern release
>
>
I took quote_ident(anyelement) as referral code, for implementing this.
Could you guide me if I am doing wrong here.


> postgres=# \sf pg_report_log (text, text, boolean)
> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text, boolean)
>  RETURNS void
>  LANGUAGE internal
>  IMMUTABLE STRICT
> AS $function$pg_report_log$function$
>
> Why stable, why immutable? This function should be volatile.
>
> Fixed these to volatile.


> 6. using elog level enum as errcode is wrong idea - errcodes are defined
> in table http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html
>

You mean, if the elevel is 'ERROR', then we need to allow errcode. Let me
know your inputs.

Adding new patch, with the above fixes.

Thanks in advance.

Regards,
Dinesh

>
> Regards
>
> Pavel
>
>
>>
>> Regards,
>> Dinesh
>> manojadinesh.blogspot.com
>>
>
>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b3b78d2..1ee8945 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17925,6 +17925,15 @@ postgres=# SELECT * FROM 
pg_xlogfile_name_offset(pg_stop_backup());
 Return information about a file.

   
+  
+   
+pg_report_log(eleveltext, 
message anyelement, 
ishidestmtboolean, detail 
text, hint text, context 
text)
+   
+   void
+   
+Write message into log file as per log level.
+   
+  
  
 

@@ -17993,6 +18002,24 @@ SELECT (pg_stat_file('filename')).modification;
 

 
+   
+pg_report_log
+   
+   
+pg_report_log is useful to write custom messages
+into current log destination and returns void.
+This function don't support the PANIC, FATAL log levels due to their 
unique internal DB usage, which may cause the database instability. Using 
ishidestmt, function can write or ignore the current SQL 
statement into the log file. Also, we can have DETAIL, HINT, CONTEXT log 
messages by provding detail, hint and 
context as function arguments. By default, all these parameter 
values are EMPTY.
+Typical usages include:
+
+postgres=# SELECT pg_report_log('NOTICE', 'Custom Message', true);
+NOTICE:  Custom Message
+ pg_report_log 
+---
+ 
+(1 row)
+
+   
+
   
 
   
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index ccc030f..4105252 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -940,3 +940,18 @@ RETUR

Re: [HACKERS] [PATCH] SQL function to report log message

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

Regards,
Dinesh
manojadinesh.blogspot.com


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-08-29 Thread Pavel Stehule
Hi

I am starting to work review of this patch

2015-07-13 9:54 GMT+02:00 dinesh kumar :

> 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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-22 Thread dinesh kumar
On Wed, Jul 22, 2015 at 8:56 PM, Michael Paquier 
wrote:

> On Thu, Jul 23, 2015 at 10:56 AM, dinesh kumar 
> wrote:
> > Hi All,
> >
> > On Mon, Jul 13, 2015 at 2:12 PM, Tom Lane  wrote:
> >>
> >> Jim Nasby  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


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-22 Thread Michael Paquier
On Thu, Jul 23, 2015 at 10:56 AM, dinesh kumar  wrote:
> Hi All,
>
> On Mon, Jul 13, 2015 at 2:12 PM, Tom Lane  wrote:
>>
>> Jim Nasby  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-22 Thread dinesh kumar
Hi All,

On Mon, Jul 13, 2015 at 2:12 PM, Tom Lane  wrote:

> Jim Nasby  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
>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 76f77cb..43dbaec 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17850,6 +17850,15 @@ postgres=# SELECT * FROM 
pg_xlogfile_name_offset(pg_stop_backup());
 Return information about a file.

   
+  
+   
+pg_report_log(eleveltext, 
message anyelement, 
ishidestmtboolean)
+   
+   void
+   
+Write message into log file as per log level.
+   
+  
  
 

@@ -17918,6 +17927,24 @@ SELECT (pg_stat_file('filename')).modification;
 

 
+   
+pg_report_log
+   
+   
+pg_report_log is useful to write custom messages
+into current log destination and returns void.
+This function don't support the PANIC, FATAL log levels due to their 
unique internal DB usage, which may cause the database instability. Using 
ishidestmt, function can write or ignore the current SQL 
statement into the log file.
+Typical usages include:
+
+postgres=# SELECT pg_report_log('NOTICE', 'Custom Message', true);
+NOTICE:  Custom Message
+ pg_report_log 
+---
+ 
+(1 row)
+
+   
+
   
 
   
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index c0495d9..1c7c263 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -76,6 +76,91 @@ current_query(PG_FUNCTION_ARGS)
 }
 
 /*
+ * pg_report_log()
+ *
+ * Printing custom log messages in log file.
+ */
+
+typedef struct
+{
+   int ecode;
+   char *level;
+} errorlevels;
+
+Datum
+pg_report_log(PG_FUNCTION_ARGS)
+{
+
+   /*
+* Do not add FATAL, PANIC log levels to the below list.
+*/
+   errorlevels elevels[]={
+   {DEBUG5, "DEBUG5"}, {DEBUG4, "DEBUG4"}, {DEBUG3, 
"DEBUG3"},
+   {DEBUG2, "DEBUG2"}, {DEBUG1, "DEBUG1"}, {LOG, "LOG"},
+   {COMMERROR, "COMMERROR"}, {INFO, "INFO"}, {NOTICE, 
"NOTICE"},
+   {WARNING, "WARNING"}, {ERROR, "ERROR"}
+   /*
+* Adding PGERROR to elevels if WIN32
+*/
+   #ifdef WIN32
+   ,{PGERROR, "PGERROR"}
+   #endif
+   };
+
+   int itr = 0;
+   bool ishidestmt = false;
+   int noelevel = (int) sizeof(elevels)/sizeof(*elevels);
+   char *level;
+
+   level = text_to_cstring(PG_GETARG_TEXT_P(0));
+   ishidestmt = PG_GETARG_BOOL(2);
+
+   /*
+* Do not expose FATAL, PANIC log levels to outer world.
+*/
+   if(pg_strcasecmp("FATAL", level) == 0)
+   ereport(ERROR,
+   (errmsg("%s is an unsupported report log 
level.", level)));
+
+   else if(pg_strcasecmp("PANIC", 

Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread Tom Lane
Jim Nasby  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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread dinesh kumar
On Mon, Jul 13, 2015 at 1:56 PM, Jim Nasby  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
>> <
>> http://www.postgresql.org/message-id/53a8e96e.9060...@2ndquadrant.com>.
>>
>>
>> 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, Data Architect, Blue Treble Consulting, Austin TX
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread Jim Nasby

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
.


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread dinesh kumar
On Mon, Jul 13, 2015 at 1:29 PM, Jim Nasby  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
>> .
>>
>
> 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

-- 
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread Jim Nasby

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
.


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


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread dinesh kumar
On Mon, Jul 13, 2015 at 1:11 AM, Michael Paquier 
wrote:

> On Mon, Jul 13, 2015 at 4:54 PM, dinesh kumar 
> 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
.

Let me know if i miss anything here.

Best Regards,
Dinesh
manojadinesh.blogspot.com

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
>


Re: [HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread Michael Paquier
On Mon, Jul 13, 2015 at 4:54 PM, dinesh kumar  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


[HACKERS] [PATCH] SQL function to report log message

2015-07-13 Thread dinesh kumar
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.

Regards,
Dinesh
manojadinesh.blogspot.com
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 76f77cb..b2fc4cd 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17850,6 +17850,15 @@ postgres=# SELECT * FROM 
pg_xlogfile_name_offset(pg_stop_backup());
 Return information about a file.

   
+  
+   
+pg_report_log(message 
text])
+   
+   void
+   
+Write message into log file.
+   
+  
  
 

@@ -17918,6 +17927,18 @@ SELECT (pg_stat_file('filename')).modification;
 

 
+   
+pg_report_log
+   
+   
+pg_report_log is useful to write custom messages
+into current log destination and returns void.
+Typical usages include:
+
+SELECT pg_report_log('Message');
+
+   
+
   
 
   
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index c0495d9..6c54f3a 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -76,6 +76,23 @@ current_query(PG_FUNCTION_ARGS)
 }
 
 /*
+ * pg_report_log()
+ *
+ * Printing custom log messages in log file.
+ */
+
+Datum
+pg_report_log(PG_FUNCTION_ARGS)
+{
+
+   ereport(MESSAGE,
+   (errmsg("%s", text_to_cstring(PG_GETARG_TEXT_P(0))),
+   errhidestmt(true)));
+
+   PG_RETURN_VOID();
+}
+
+/*
  * Send a signal to another backend.
  *
  * The signal is delivered if the user is either a superuser or the same
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 088c714..2e8f547 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -302,7 +302,7 @@ errstart(int elevel, const char *filename, int lineno,
elevel == INFO);
}
 
-   /* Skip processing effort if non-error message will not be output */
+   /* Skip processing effort if non-error,custom message will not be 
output */
if (elevel < ERROR && !output_to_server && !output_to_client)
return false;
 
@@ -2062,6 +2062,7 @@ write_eventlog(int level, const char *line, int len)
case DEBUG3:
case DEBUG2:
case DEBUG1:
+   case MESSAGE:
case LOG:
case COMMERROR:
case INFO:
@@ -2917,6 +2918,7 @@ send_message_to_server_log(ErrorData *edata)
case DEBUG1:
syslog_level = LOG_DEBUG;
break;
+   case MESSAGE:
case LOG:
case COMMERROR:
case INFO:
@@ -3547,6 +3549,7 @@ error_severity(int elevel)
case DEBUG5:
prefix = _("DEBUG");
break;
+   case MESSAGE:
case LOG:
case COMMERROR:
prefix = _("LOG");
@@ -3666,6 +3669,9 @@ is_log_level_output(int elevel, int log_min_level)
/* Neither is LOG */
else if (elevel >= log_min_level)
return true;
+   /* If elevel is MESSAGE, then ignore log settings */
+   else if (elevel == MESSAGE)
+   return true;
 
return false;
 }
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 6fd1278..62c619a 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -5344,6 +5344,11 @@ DESCR("tsm_bernoulli_reset(internal)");
 DATA(insert OID = 3346 (  tsm_bernoulli_cost   PGNSP PGUID 12 1 0 0 0 
f f f f t f v 7 0 2278 "2281 2281 2281 2281 2281 2281 2281" _null_ _null_ 
_null_ _null_ _null_ tsm_bernoulli_cost _null_ _null_ _null_ ));
 DESCR("tsm_bernoulli_cost(internal)");
 
+/* Logging function */
+
+DATA(insert OID = 6015 (  pg_report_logPGNSP PGUID 12 1 0 0 0 
f f f f t f v 1 0 2278 "1043" _