On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule <pavel.steh...@gmail.com>
wrote:
>
>
> 2015-09-02 21:49 GMT+02:00 dinesh kumar <dineshkuma...@gmail.com>:
>
>> 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 <pavel.steh...@gmail.com>:
>>>
>>>>
>>>>
>>>> 2015-08-31 20:43 GMT+02:00 dinesh kumar <dineshkuma...@gmail.com>:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
>>>>> pavel.steh...@gmail.com> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> I am starting to work review of this patch
>>>>>>
>>>>>> 2015-07-13 9:54 GMT+02:00 dinesh kumar <dineshkuma...@gmail.com>:
>>>>>>
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Greetings for the day.
>>>>>>>
>>>>>>> Would like to discuss on below feature here.
>>>>>>>
>>>>>>> Feature:
>>>>>>> Having an SQL function, to write messages to log destination.
>>>>>>>
>>>>>>> Justification:
>>>>>>> As of now, we don't have an SQL function to write
>>>>>>> custom/application messages to log destination. We have "RAISE" clause
>>>>>>> which is controlled by
>>>>>>> log_ parameters. If we have an SQL function which works irrespective
>>>>>>> of log settings, that would be a good for many log parsers. What i mean
>>>>>>> is,
>>>>>>> in DBA point of view, if we route all our native OS stats to log files
>>>>>>> in a
>>>>>>> proper format, then we can have our log reporting tools to give most
>>>>>>> effective reports. Also, Applications can log their own messages to
>>>>>>> postgres log files, which can be monitored by DBAs too.
>>>>>>>
>>>>>>> Implementation:
>>>>>>> Implemented a new function "pg_report_log" which takes one
>>>>>>> argument as text, and returns void. I took, "LOG" prefix for all the
>>>>>>> reporting messages.I wasn't sure to go with new prefix for this, since
>>>>>>> these are normal LOG messages. Let me know, if i am wrong here.
>>>>>>>
>>>>>>> Here is the attached patch.
>>>>>>>
>>>>>>
>>>>>> This patch is not complex, but the implementation doesn't cover a
>>>>>> "ereport" well.
>>>>>>
>>>>>> Although this functionality should be replaced by custom function in
>>>>>> any PL (now or near future), I am not against to have this function in
>>>>>> core. There are lot of companies with strong resistance against stored
>>>>>> procedures - and sometimes this functionality can help with SQL
>>>>>> debugging.
>>>>>>
>>>>>> Issues:
>>>>>>
>>>>>> 1. Support only MESSAGE field in exception - I am expecting to
>>>>>> support all fields: HINT, DETAIL, ...
>>>>>>
>>>>>
>>>>> Added these functionalities too.
>>>>>
>>>>>
>>>>>> 2. Missing regress tests
>>>>>>
>>>>>
>>>>> Adding here.
>>>>>
>>>>>
>>>>>> 3. the parsing ereport level should be public function shared with
>>>>>> PLpgSQL and other PL
>>>>>>
>>>>>
>>>>> Sorry Pavel. I am not getting your point here. Would you give me an
>>>>> example.
>>>>>
>>>>
>>>> The transformation: text -> error level is common task - and PLpgSQL it
>>>> does in pl_gram.y. My idea is to add new function to error utils named
>>>> "parse_error_level" and use it from PLpgSQL and from your code.
>>>>
>>>>
>>>>>
>>>>>
>>>>>> 4. should be hidestmt mandatory parameter?
>>>>>>
>>>>>
>>>>> I changed this argument's default value as "true".
>>>>>
>>>>>
>>>>>> 5. the function declaration is strange
>>>>>>
>>>>>> postgres=# \sf pg_report_log (text, anyelement, boolean)
>>>>>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, anyelement,
>>>>>> boolean)
>>>>>> RETURNS void
>>>>>> LANGUAGE sql
>>>>>> STABLE STRICT COST 1
>>>>>> AS $function$SELECT pg_report_log($1::pg_catalog.text,
>>>>>> $2::pg_catalog.text, $3::boolean)$function$
>>>>>>
>>>>>> Why polymorphic? It is useless on any modern release
>>>>>>
>>>>>>
>>>>> I took quote_ident(anyelement) as referral code, for implementing
>>>>> this. Could you guide me if I am doing wrong here.
>>>>>
>>>>
>>>> I was wrong - this is ok - it is emulation of force casting to text
>>>>
>>>>
>>>>>
>>>>>
>>>>>> postgres=# \sf pg_report_log (text, text, boolean)
>>>>>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text,
>>>>>> boolean)
>>>>>> RETURNS void
>>>>>> LANGUAGE internal
>>>>>> IMMUTABLE STRICT
>>>>>> AS $function$pg_report_log$function$
>>>>>>
>>>>>> Why stable, why immutable? This function should be volatile.
>>>>>>
>>>>>> Fixed these to volatile.
>>>>>
>>>>>
>>>>>> 6. using elog level enum as errcode is wrong idea - errcodes are
>>>>>> defined in table
>>>>>> http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html
>>>>>>
>>>>>
>>>>> You mean, if the elevel is 'ERROR', then we need to allow errcode. Let
>>>>> me know your inputs.
>>>>>
>>>>
>>>> I was blind, but the code was not good. Yes, error and higher needs
>>>> error code. From ANSI/SQL anything can has error code "00 is ok", "01 ..
>>>> warnings" ...
>>>>
>>>> There is more possibilities - look to PLpgSQL implementation - it can
>>>> be optional parameter - it default can use ERRCODE_RAISE_EXCEPTION
>>>>
>>>>
>>>>>
>>>>> Adding new patch, with the above fixes.
>>>>>
>>>>
>>> the code looks better
>>>
>>> my objections:
>>>
>>> 1. I prefer default values be NULL
>>>
>>
>> Fixed it.
>>
>>
>>> 2. readability:
>>> * parsing error level should be in alone cycle
>>> * you don't need to use more ereport calls - one is good enough - look
>>> on implementation of stmt_raise in PLpgSQL
>>>
>>
>> Sorry for my ignorance. I have tried to implement parse_error_level in
>> pl_gram.y, but not able to do it. I was not able to parse the given string
>> with tokens, and return the error levels. I tried for a refferal code, but
>> not able to find any. Would you guide me on this.
>>
>> In this attached patch, I have minimized the ereport calls. Kindly check
>> and let me know.
>>
>>
>>> 3. test should be enhanced for optional parameters
>>>
>>> Fixed it.
>>
>
> only few points:
>
> 1. missing to set errstate - any exception should to have some errcode
> value. There can be default like PLpgSQL ERRCODE_RAISE_EXCEPTION for any
> where elog_level >= error
>
>
Fixed It.
> 2. the explicit setting context is not consistent with our PL - the
> context is implicit value only - not possible to set it explicitly. The
> behave of this field is not clear - but in this moment, the context is
> related to PostgreSQL area - not to application area.
>
OK. Shall i remove the context field from this function.
Regards,
Dinesh
manojadinesh.blogspot.com
Regards
>
> Pavel
>
>
>> Regards,
>> Dinesh
>> manojadinesh.blogspot.com
>>
>> Regards
>>>
>>> Pavel
>>>
>>>
>>>>
>>>>> 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..ff20a9f 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.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_report_log(<parameter>elevel</><type>text</>,
<parameter>message</> <type>anyelement</type>,
<parameter>ishidestmt</><type>boolean</>, <parameter>detail</>
<type>text</type>, <parameter>hint</> <type>text</type>, <parameter>context</>
<type>text</type>)</function></literal>
+ </entry>
+ <entry><type>void</type></entry>
+ <entry>
+ Write message into log file as per log level.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -17993,6 +18002,24 @@ SELECT (pg_stat_file('filename')).modification;
</programlisting>
</para>
+ <indexterm>
+ <primary>pg_report_log</primary>
+ </indexterm>
+ <para>
+ <function>pg_report_log</> is useful to write custom messages
+ into current log destination and returns <type>void</type>.
+ This function don't support the PANIC, FATAL log levels due to their
unique internal DB usage, which may cause the database instability. Using
<parameter>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 <parameter>detail</>, <parameter>hint</> and
<parameter>context</> as function arguments. By default, all these parameter
values are NULL.
+ Typical usages include:
+<programlisting>
+postgres=# SELECT pg_report_log('NOTICE', 'Custom Message', true);
+NOTICE: Custom Message
+ pg_report_log
+---------------
+
+(1 row)
+</programlisting>
+ </para>
+
</sect2>
<sect2 id="functions-advisory-locks">
diff --git a/src/backend/catalog/system_views.sql
b/src/backend/catalog/system_views.sql
index ccc030f..cd6cc0f 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -940,3 +940,18 @@ RETURNS jsonb
LANGUAGE INTERNAL
STRICT IMMUTABLE
AS 'jsonb_set';
+
+CREATE OR REPLACE FUNCTION pg_report_log(elevel TEXT, message TEXT, ishidestmt
BOOLEAN DEFAULT TRUE, detail TEXT DEFAULT NULL, hint TEXT DEFAULT NULL, context
TEXT DEFAULT NULL)
+RETURNS VOID
+LANGUAGE INTERNAL
+VOLATILE
+AS 'pg_report_log';
+
+CREATE OR REPLACE FUNCTION pg_report_log(elevel TEXT, message anyelement,
ishidestmt BOOLEAN DEFAULT TRUE, detail TEXT DEFAULT NULL, hint TEXT DEFAULT
NULL, context TEXT DEFAULT NULL)
+RETURNS VOID
+VOLATILE
+AS
+$$
+SELECT pg_report_log($1::pg_catalog.text, $2::pg_catalog.text,
$3::pg_catalog.bool, $4::pg_catalog.text, $5::pg_catalog.text,
$6::pg_catalog.text)
+$$
+LANGUAGE SQL;
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index c0495d9..99f99e0 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -75,6 +75,100 @@ current_query(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
+
+/*
+ * Parsing error levels
+ */
+typedef struct
+{
+ int ecode;
+ char *level;
+} errorlevels;
+
+static int parse_error_level(const char* elevel)
+{
+ errorlevels elevels[]={
+ {DEBUG5, "DEBUG5"}, {DEBUG4, "DEBUG4"}, {DEBUG3,
"DEBUG3"},
+ {DEBUG2, "DEBUG2"}, {DEBUG1, "DEBUG1"}, {LOG, "LOG"},
+ {COMMERROR, "COMMERROR"}, {INFO, "INFO"}, {NOTICE,
"NOTICE"},
+ {WARNING, "WARNING"}, {ERROR, "ERROR"}, {FATAL,
"FATAL"}, {PANIC, "PANIC"}
+ /*
+ * Adding PGERROR to elevels if WIN32
+ */
+ #ifdef WIN32
+ ,{PGERROR, "PGERROR"}
+ #endif
+ };
+ int noelevel = (int) sizeof(elevels)/sizeof(*elevels);
+ int itr = 0;
+
+ while (itr < noelevel)
+ {
+ if (pg_strcasecmp(elevels[itr].level, elevel) == 0)
+ break;
+ itr++;
+ }
+
+ if (itr != noelevel)
+ return elevels[itr].ecode;
+
+ else
+ /* Invalid log level */
+ return 0;
+}
+
+/*
+ * pg_report_log()
+ *
+ * Printing custom log messages in log file.
+ */
+
+Datum
+pg_report_log(PG_FUNCTION_ARGS)
+{
+ int elevel;
+ bool ishidestmt = false;
+ char *loglevel, *detail, *hint, *cntxt;
+
+ loglevel = PG_ARGISNULL(0) ? NULL :
text_to_cstring(PG_GETARG_TEXT_P(0));
+ detail = PG_ARGISNULL(3) ? NULL :
text_to_cstring(PG_GETARG_TEXT_P(3));
+ hint = PG_ARGISNULL(4) ? NULL :
text_to_cstring(PG_GETARG_TEXT_P(4));
+ cntxt = PG_ARGISNULL(5) ? NULL :
text_to_cstring(PG_GETARG_TEXT_P(5));
+ ishidestmt = PG_GETARG_BOOL(2);
+
+ if(!loglevel)
+ ereport(ERROR,
+ (errmsg("NULL is an unsupported report log
level.")));
+
+ elevel = parse_error_level(loglevel);
+
+ /*
+ * Do not expose FATAL, PANIC log levels to outer world.
+ */
+ if(elevel && elevel==FATAL)
+ ereport(ERROR,
+ (errmsg("%s is an unsupported report log
level.", loglevel)));
+
+ else if(elevel && elevel==PANIC)
+ ereport(ERROR,
+ (errmsg("%s is an unsupported report log
level.", loglevel)));
+
+ else if(elevel)
+ ereport(elevel,
+ ((elevel>=ERROR) ?
errcode(ERRCODE_RAISE_EXCEPTION) : 0,
+ (errmsg("%s", PG_ARGISNULL(1) ? "" :
text_to_cstring(PG_GETARG_TEXT_P(1))),
+ detail ? errdetail("%s",
detail) : 0,
+ hint ? errhint("%s", hint) : 0,
+ cntxt ? errcontext_msg("%s",
cntxt) : 0,
+ errhidestmt(ishidestmt)
+ )));
+ else
+ ereport(ERROR,
+ (errmsg("%s is an unknown report log level.",
loglevel)));
+
+ PG_RETURN_VOID();
+}
+
/*
* Send a signal to another backend.
*
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index ddf7c67..7c4fe87 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -5349,6 +5349,13 @@ DESCR("row security for current context active on table
by table oid");
DATA(insert OID = 3299 ( row_security_active PGNSP PGUID 12 1 0 0 0 f f f
f t f s 1 0 16 "25" _null_ _null_ _null_ _null_ _null_
row_security_active_name _null_ _null_ _null_ ));
DESCR("row security for current context active on table by table name");
+/* Logging function */
+
+DATA(insert OID = 6015 ( pg_report_log PGNSP PGUID 12 1 0 0 0
f f f f f f v 6 0 2278 "25 25 16 25 25 25" _null_ _null_ "{elevel, message,
ishidestmt, detail, hint, context}" _null_ _null_ pg_report_log _null_ _null_
_null_ ));
+DESCR("write message to log file");
+DATA(insert OID = 6016 ( pg_report_log PGNSP PGUID 14 1 0 0 0
f f f f f f v 6 0 2278 "25 2283 16 25 25 25" _null_ _null_ "{elevel, message,
ishidestmt, detail, hint, context}" _null_ _null_ "SELECT
pg_report_log($1::pg_catalog.text, $2::pg_catalog.text, $3::pg_catalog.bool,
$4::pg_catalog.text, $5::pg_catalog.text, $6::pg_catalog.text)" _null_ _null_
_null_ ));
+DESCR("write message to log file");
+
/*
* Symbolic values for proargmodes column. Note that these must agree with
* the FunctionParameterMode enum in parsenodes.h; we declare them here to
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index fc1679e..0dd1425 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -495,6 +495,7 @@ extern Datum pg_typeof(PG_FUNCTION_ARGS);
extern Datum pg_collation_for(PG_FUNCTION_ARGS);
extern Datum pg_relation_is_updatable(PG_FUNCTION_ARGS);
extern Datum pg_column_is_updatable(PG_FUNCTION_ARGS);
+extern Datum pg_report_log(PG_FUNCTION_ARGS);
/* oid.c */
extern Datum oidin(PG_FUNCTION_ARGS);
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7684717..fcb7218 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -16,6 +16,13 @@
#include <setjmp.h>
+/*
+ * XXX
+ * If you are adding another elevel, make sure you update the
+ * pg_report_log() in src/backend/utils/adt/misc.c, with the
+ * new elevel
+ */
+
/* Error level codes */
#define DEBUG5 10 /* Debugging messages, in
categories of
* decreasing
detail. */
diff --git a/src/test/regress/expected/reportlog.out
b/src/test/regress/expected/reportlog.out
new file mode 100644
index 0000000..fd639b4
--- /dev/null
+++ b/src/test/regress/expected/reportlog.out
@@ -0,0 +1,84 @@
+--
+-- Test for Report Log With WARNING
+--
+SELECT pg_catalog.pg_report_log('WARNING', 'Custom Message'); --OK
+WARNING: Custom Message
+ pg_report_log
+---------------
+
+(1 row)
+
+--
+-- Test for ERROR with default ishidestmt
+--
+SELECT pg_catalog.pg_report_log('ERROR', 'Custom Message'); --ERROR
+ERROR: Custom Message
+--
+-- Test for ERROR with ishidestmt
+--
+SELECT pg_catalog.pg_report_log('ERROR', 'Custom Message', true); --ERROR
+ERROR: Custom Message
+--
+-- Test for anyelement
+--
+SELECT pg_catalog.pg_report_log('WARNING', -1234.34); --OK
+WARNING: -1234.34
+ pg_report_log
+---------------
+
+(1 row)
+
+--
+-- Test for denial of FATAL
+--
+SELECT pg_catalog.pg_report_log('FATAL', 'Fatal Message'); --OK
+ERROR: FATAL is an unsupported report log level.
+--
+-- Test for optional arguements
+--
+SELECT pg_catalog.pg_report_log('WARNING', 'Warning Message', true, 'WARNING
DETAIL'); --OK
+WARNING: Warning Message
+DETAIL: WARNING DETAIL
+ pg_report_log
+---------------
+
+(1 row)
+
+--
+-- Test for NULL elevel
+--
+SELECT pg_catalog.pg_report_log(NULL, NULL); --ERROR
+ERROR: NULL is an unsupported report log level.
+--
+-- Test for NULL Message
+--
+SELECT pg_catalog.pg_report_log('NOTICE', NULL); --OK
+NOTICE:
+ pg_report_log
+---------------
+
+(1 row)
+
+--
+-- Test for all NULL inputs, except elevel
+--
+SELECT pg_catalog.pg_report_log('WARNING', NULL, NULL, NULL, NULL, NULL); --OK
+WARNING:
+ pg_report_log
+---------------
+
+(1 row)
+
+--
+-- Test for all NOT NULL arguments
+--
+SELECT pg_catalog.pg_report_log('WARNING', 'Warning Message', true, 'DETAIL',
'HINT', 'CONTEXT'); --OK
+WARNING: Warning Message
+DETAIL: DETAIL
+HINT: HINT
+CONTEXT: CONTEXT
+ pg_report_log
+---------------
+
+(1 row)
+
diff --git a/src/test/regress/parallel_schedule
b/src/test/regress/parallel_schedule
index 6fc5d1e..4cd193d 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -97,7 +97,7 @@ test: rules
# ----------
# Another group of parallel tests
# ----------
-test: select_views portals_p2 foreign_key cluster dependency guc bitmapops
combocid tsearch tsdicts foreign_data window xmlmap functional_deps
advisory_lock json jsonb indirect_toast equivclass
+test: select_views portals_p2 foreign_key cluster dependency guc bitmapops
combocid tsearch tsdicts foreign_data window xmlmap functional_deps
advisory_lock json jsonb indirect_toast reportlog equivclass
# ----------
# Another group of parallel tests
# NB: temp.sql does a reconnect which transiently uses 2 connections,
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 2ae51cf..6c9f5c3 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -158,3 +158,4 @@ test: with
test: xml
test: event_trigger
test: stats
+test: reportlog
diff --git a/src/test/regress/sql/reportlog.sql
b/src/test/regress/sql/reportlog.sql
new file mode 100644
index 0000000..153f03e
--- /dev/null
+++ b/src/test/regress/sql/reportlog.sql
@@ -0,0 +1,51 @@
+--
+-- Test for Report Log With WARNING
+--
+SELECT pg_catalog.pg_report_log('WARNING', 'Custom Message'); --OK
+
+--
+-- Test for ERROR with default ishidestmt
+--
+SELECT pg_catalog.pg_report_log('ERROR', 'Custom Message'); --ERROR
+
+--
+-- Test for ERROR with ishidestmt
+--
+SELECT pg_catalog.pg_report_log('ERROR', 'Custom Message', true); --ERROR
+
+--
+-- Test for anyelement
+--
+SELECT pg_catalog.pg_report_log('WARNING', -1234.34); --OK
+
+--
+-- Test for denial of FATAL
+--
+SELECT pg_catalog.pg_report_log('FATAL', 'Fatal Message'); --OK
+
+--
+-- Test for optional arguements
+--
+SELECT pg_catalog.pg_report_log('WARNING', 'Warning Message', true, 'WARNING
DETAIL'); --OK
+
+--
+-- Test for NULL elevel
+--
+SELECT pg_catalog.pg_report_log(NULL, NULL); --ERROR
+
+
+--
+-- Test for NULL Message
+--
+SELECT pg_catalog.pg_report_log('NOTICE', NULL); --OK
+
+
+--
+-- Test for all NULL inputs, except elevel
+--
+SELECT pg_catalog.pg_report_log('WARNING', NULL, NULL, NULL, NULL, NULL); --OK
+
+--
+-- Test for all NOT NULL arguments
+--
+SELECT pg_catalog.pg_report_log('WARNING', 'Warning Message', true, 'DETAIL',
'HINT', 'CONTEXT'); --OK
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
