parse callback allows inserting cursorpos when hide_stmt. - Mailing list pgsql-hackers

Hello.

Parse error callback sets cursor position even if hide_stmt is
true. So I see a strange message with meaningless 'at character
%d' notation.

2018-03-07 11:11:43.489 JST [10304] DEBUG:  removed 223/2049, age(-2s:121, -3s:121, *-30s:1584, -60s:223, -90s:0)
naccessed(0:223,1:0, 2:0) at character 15
 
<no STATEMENT follows>

I think hide_stmt is another reason to refrain from setting cursorpos.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 1014b486007e38d64d1edf53cb99b5e7cc09cf7c Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 7 Mar 2018 11:30:32 +0900
Subject: [PATCH] Don't add cursor position when hide_stmt

Cursor position is meaningless when a log message is not accompanied
by statement string. Refrain from setting it in the case.
---
 src/backend/parser/parse_node.c |  5 +++--
 src/backend/utils/error/elog.c  | 17 +++++++++++++++++
 src/include/utils/elog.h        |  1 +
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index d2672882d7..3f508bd308 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -169,14 +169,15 @@ cancel_parser_errposition_callback(ParseCallbackState *pcbstate)
  *
  * Note that this will be called for *any* error occurring while the
  * callback is installed.  We avoid inserting an irrelevant error location
- * if the error is a query cancel --- are there any other important cases?
+ * if the error is a query cancel and in the case of hide_stmt --- are there
+ * any other important cases?
  */
 static void
 pcb_error_callback(void *arg)
 {
     ParseCallbackState *pcbstate = (ParseCallbackState *) arg;
 
-    if (geterrcode() != ERRCODE_QUERY_CANCELED)
+    if (geterrcode() != ERRCODE_QUERY_CANCELED && !gethidestmt())
         (void) parser_errposition(pcbstate->pstate, pcbstate->location);
 }
 
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 16531f7a0f..fd69160d48 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1281,6 +1281,23 @@ getinternalerrposition(void)
     return edata->internalpos;
 }
 
+/*
+ * gethidestmt --- return true if STATEMENT is hidden
+ *
+ * This is only intended for use in error callback subroutines, since there
+ * is no other place outside elog.c where the concept is meaningful.
+ */
+bool
+gethidestmt(void)
+{
+    ErrorData  *edata = &errordata[errordata_stack_depth];
+
+    /* we don't bother incrementing recursion_depth */
+    CHECK_STACK_DEPTH();
+
+    return edata->hide_stmt;
+}
+
 
 /*
  * elog_start --- startup for old-style API
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7a9ba7f2ff..0031971f16 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -181,6 +181,7 @@ extern int    err_generic_string(int field, const char *str);
 extern int    geterrcode(void);
 extern int    geterrposition(void);
 extern int    getinternalerrposition(void);
+extern bool    gethidestmt(void);
 
 
 /*----------
-- 
2.16.2