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.
> 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.
</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..4105252 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 '', hint TEXT DEFAULT '', context
TEXT DEFAULT '')
+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 '', hint TEXT DEFAULT '',
context TEXT DEFAULT '')
+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..be25422 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -76,6 +76,103 @@ 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, *detail, *hint, *cntxt;
+
+ level = text_to_cstring(PG_GETARG_TEXT_P(0));
+ detail = text_to_cstring(PG_GETARG_TEXT_P(3));
+ hint = text_to_cstring(PG_GETARG_TEXT_P(4));
+ cntxt = text_to_cstring(PG_GETARG_TEXT_P(5));
+
+ 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", level) == 0)
+ ereport(ERROR,
+ (errmsg("%s is an unsupported report log
level.", level)));
+ else
+ {
+ while (itr < noelevel)
+ {
+ if (pg_strcasecmp(elevels[itr].level, level) == 0)
+ {
+ /*
+ * Is errhide statement true
+ */
+ if (ishidestmt == true)
+ {
+ ereport(elevels[itr].ecode,
+ (errmsg("%s",
text_to_cstring(PG_GETARG_TEXT_P(1))),
+
(pg_strcasecmp(detail, "") != 0) ? errdetail("%s", detail) : 0,
+
(pg_strcasecmp(hint, "") != 0) ? errhint("%s", hint) : 0,
+
(pg_strcasecmp(cntxt, "") != 0) ? errcontext_msg("%s", cntxt) : 0,
+
errhidestmt(true)
+ ));
+ break;
+ }
+ else
+ {
+ ereport(elevels[itr].ecode,
+ (errmsg("%s",
text_to_cstring(PG_GETARG_TEXT_P(1))),
+
(pg_strcasecmp(detail, "") != 0) ? errdetail("%s", detail) : 0,
+
(pg_strcasecmp(hint, "") != 0) ? errhint("%s", hint) : 0,
+
(pg_strcasecmp(cntxt, "") != 0) ? errcontext_msg("%s", cntxt) : 0
+ ));
+ break;
+ }
+ }
+ itr++;
+ }
+
+ /* Invalid log level */
+ if (itr == noelevel)
+ ereport(ERROR,
+ (errmsg("Unknown log level \"%s\"",
level)));
+ else
+ 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/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index ddf7c67..da772e2 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -5349,6 +5349,14 @@ 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 t 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 t 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..a352564
--- /dev/null
+++ b/src/test/regress/expected/reportlog.out
@@ -0,0 +1,35 @@
+--
+-- 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.
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..eb9ee90
--- /dev/null
+++ b/src/test/regress/sql/reportlog.sql
@@ -0,0 +1,24 @@
+--
+-- 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
