On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <[email protected]>
wrote:
>
>
> 2015-09-01 6:59 GMT+02:00 Pavel Stehule <[email protected]>:
>
>>
>>
>> 2015-08-31 20:43 GMT+02:00 dinesh kumar <[email protected]>:
>>
>>> Hi,
>>>
>>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <[email protected]>
>>> wrote:
>>>
>>>> Hi
>>>>
>>>> I am starting to work review of this patch
>>>>
>>>> 2015-07-13 9:54 GMT+02:00 dinesh kumar <[email protected]>:
>>>>
>>>>> Hi All,
>>>>>
>>>>> Greetings for the day.
>>>>>
>>>>> Would like to discuss on below feature here.
>>>>>
>>>>> Feature:
>>>>> Having an SQL function, to write messages to log destination.
>>>>>
>>>>> Justification:
>>>>> As of now, we don't have an SQL function to write
>>>>> custom/application messages to log destination. We have "RAISE" clause
>>>>> which is controlled by
>>>>> log_ parameters. If we have an SQL function which works irrespective
>>>>> of log settings, that would be a good for many log parsers. What i mean
>>>>> is,
>>>>> in DBA point of view, if we route all our native OS stats to log files in
>>>>> a
>>>>> proper format, then we can have our log reporting tools to give most
>>>>> effective reports. Also, Applications can log their own messages to
>>>>> postgres log files, which can be monitored by DBAs too.
>>>>>
>>>>> Implementation:
>>>>> Implemented a new function "pg_report_log" which takes one
>>>>> argument as text, and returns void. I took, "LOG" prefix for all the
>>>>> reporting messages.I wasn't sure to go with new prefix for this, since
>>>>> these are normal LOG messages. Let me know, if i am wrong here.
>>>>>
>>>>> Here is the attached patch.
>>>>>
>>>>
>>>> This patch is not complex, but the implementation doesn't cover a
>>>> "ereport" well.
>>>>
>>>> Although this functionality should be replaced by custom function in
>>>> any PL (now or near future), I am not against to have this function in
>>>> core. There are lot of companies with strong resistance against stored
>>>> procedures - and sometimes this functionality can help with SQL debugging.
>>>>
>>>> Issues:
>>>>
>>>> 1. Support only MESSAGE field in exception - I am expecting to support
>>>> all fields: HINT, DETAIL, ...
>>>>
>>>
>>> Added these functionalities too.
>>>
>>>
>>>> 2. Missing regress tests
>>>>
>>>
>>> Adding here.
>>>
>>>
>>>> 3. the parsing ereport level should be public function shared with
>>>> PLpgSQL and other PL
>>>>
>>>
>>> Sorry Pavel. I am not getting your point here. Would you give me an
>>> example.
>>>
>>
>> The transformation: text -> error level is common task - and PLpgSQL it
>> does in pl_gram.y. My idea is to add new function to error utils named
>> "parse_error_level" and use it from PLpgSQL and from your code.
>>
>>
>>>
>>>
>>>> 4. should be hidestmt mandatory parameter?
>>>>
>>>
>>> I changed this argument's default value as "true".
>>>
>>>
>>>> 5. the function declaration is strange
>>>>
>>>> postgres=# \sf pg_report_log (text, anyelement, boolean)
>>>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, anyelement,
>>>> boolean)
>>>> RETURNS void
>>>> LANGUAGE sql
>>>> STABLE STRICT COST 1
>>>> AS $function$SELECT pg_report_log($1::pg_catalog.text,
>>>> $2::pg_catalog.text, $3::boolean)$function$
>>>>
>>>> Why polymorphic? It is useless on any modern release
>>>>
>>>>
>>> I took quote_ident(anyelement) as referral code, for implementing this.
>>> Could you guide me if I am doing wrong here.
>>>
>>
>> I was wrong - this is ok - it is emulation of force casting to text
>>
>>
>>>
>>>
>>>> postgres=# \sf pg_report_log (text, text, boolean)
>>>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text, boolean)
>>>> RETURNS void
>>>> LANGUAGE internal
>>>> IMMUTABLE STRICT
>>>> AS $function$pg_report_log$function$
>>>>
>>>> Why stable, why immutable? This function should be volatile.
>>>>
>>>> Fixed these to volatile.
>>>
>>>
>>>> 6. using elog level enum as errcode is wrong idea - errcodes are
>>>> defined in table
>>>> http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html
>>>>
>>>
>>> You mean, if the elevel is 'ERROR', then we need to allow errcode. Let
>>> me know your inputs.
>>>
>>
>> I was blind, but the code was not good. Yes, error and higher needs error
>> code. From ANSI/SQL anything can has error code "00 is ok", "01 ..
>> warnings" ...
>>
>> There is more possibilities - look to PLpgSQL implementation - it can be
>> optional parameter - it default can use ERRCODE_RAISE_EXCEPTION
>>
>>
>>>
>>> Adding new patch, with the above fixes.
>>>
>>
> the code looks better
>
> my objections:
>
> 1. I prefer default values be NULL
>
Fixed it.
> 2. readability:
> * parsing error level should be in alone cycle
> * you don't need to use more ereport calls - one is good enough - look
> on implementation of stmt_raise in PLpgSQL
>
Sorry for my ignorance. I have tried to implement parse_error_level in
pl_gram.y, but not able to do it. I was not able to parse the given string
with tokens, and return the error levels. I tried for a refferal code, but
not able to find any. Would you guide me on this.
In this attached patch, I have minimized the ereport calls. Kindly check
and let me know.
> 3. test should be enhanced for optional parameters
>
> Fixed it.
Regards,
Dinesh
manojadinesh.blogspot.com
Regards
>
> 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..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.
</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 EMPTY.
+ 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..da0d950 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -75,6 +75,99 @@ 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,
+ (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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
