Hi All,

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

>
> 2015-10-20 20:05 GMT+02:00 Robert Haas <robertmh...@gmail.com>:
>
>> On Tue, Oct 20, 2015 at 11:29 AM, Pavel Stehule <pavel.steh...@gmail.com>
>> wrote:
>> > 2015-10-20 17:15 GMT+02:00 Robert Haas <robertmh...@gmail.com>:
>> >> 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.
        </entry>
       </row>
+      <row>
+       <entry>
+        <literal><function>pg_report_log(<parameter>loglevel</><type>text</>, 
<parameter>message</> <type>anyelement</>[, <parameter>ishidestmt</> 
<type>boolean</> ] [, <parameter>detail</> <type> text</>] [, 
<parameter>hint</> <type>text</>] [, <parameter>sqlstate</> 
<type>text</>])</function></literal>
+       </entry>
+       <entry><type>void</type></entry>
+       <entry>
+        Report message or error.
+       </entry>
+      </row>
      </tbody>
     </tgroup>
    </table>
@@ -18034,6 +18043,32 @@ 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
+    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 <parameter>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 <parameter>detail</>, <parameter>hint</> as function
+    arguments, which are NULL by default. The parameter <parameter>sqlstate</>
+    allows to set a SQLSTATE of raised exception. Default value of this
+    parameter is 'P0001' for ERROR only level.
+
+    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..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';
+
+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
index c0495d9..20be263 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -75,6 +75,138 @@ current_query(PG_FUNCTION_ARGS)
                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),
+                                errmsg("log level must not be null")));
+
+       elog_level_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
+       if (!parse_error_level(elog_level_str, &elog_level))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("invalid or disallowed log level: 
\'%s\'", elog_level_str)));
+
+       /* message */
+       if (PG_ARGISNULL(1))
+               err_message = "The message is null";
+       else
+               err_message = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+       /* ishidestmt */
+       if (!PG_ARGISNULL(2))
+               ishidestmt = PG_GETARG_BOOL(2);
+
+       /* detail */
+       if (!PG_ARGISNULL(3))
+               err_detail = text_to_cstring(PG_GETARG_TEXT_PP(3));
+
+       /* hint */
+       if (!PG_ARGISNULL(4))
+               err_hint = text_to_cstring(PG_GETARG_TEXT_PP(4));
+
+       /* sqlstate */
+       if (!PG_ARGISNULL(5))
+       {
+               sqlstate_str = text_to_cstring(PG_GETARG_TEXT_PP(5));
+               if (strlen(sqlstate_str) != 5 ||
+                               strspn(sqlstate_str, 
"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ") != 5)
+                       ereport(ERROR,
+                                       
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("invalid SQLSTATE code: 
\'%s\'", sqlstate_str)));
+
+               sqlstate = MAKE_SQLSTATE(sqlstate_str[0],
+                                                                
sqlstate_str[1],
+                                                                
sqlstate_str[2],
+                                                                
sqlstate_str[3],
+                                                                
sqlstate_str[4]);
+       }
+       else if (elog_level >= ERROR)
+               ereport(ERROR,
+                               (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+                                errmsg("sqlstate must not be null when 
loglevel is ERROR")));
+
+       ereport(elog_level,
+                       ((sqlstate != 0) ? errcode(sqlstate) : 0,
+                        errmsg_internal("%s", err_message),
+                        (err_detail != NULL) ? errdetail_internal("%s", 
err_detail) : 0,
+                        (err_hint != NULL) ? errhint("%s", err_hint) : 0,
+                        errhidestmt(ishidestmt)));
+
+       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 f688454..1550d7d 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -5351,6 +5351,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 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_ "{loglevel, message, 
ishidestmt, detail, hint, sqlstate}" _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_ "{loglevel, message, 
ishidestmt, detail, hint, sqlstate}" _null_ _null_ "SELECT pg_report_log($1, 
$2::pg_catalog.text, $3, $4, $5, $6)" _null_ _null_ _null_ ));
+DESCR("write message to log file");
+
 /*
  * Symbolic values for proparallel column: these indicate whether a function
  * can be safely be run in a parallel backend, during parallelism but
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 7715719..63e177b 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
+ *             parse_error_level() 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/parallel_schedule 
b/src/test/regress/parallel_schedule
index c63abf4..747aabf 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 json_encoding 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 json_encoding 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 88dcd64..cab3c7a 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -159,3 +159,4 @@ test: with
 test: xml
 test: event_trigger
 test: stats
+test: reportlog
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to