outfuncs.c utility statement support
While debugging some other stuff, I was wondering to what extent node
types supporting utility statements should be supported in outfuncs.c.
Running with --debug-print-parse=on, executing
create table test1 (a int, b text);
gives output that is truncated somewhere in the middle (possibly a null
byte), and
drop table test1;
shows (among other things)
:utilityStmt ?
Is there a way to check that everything that needs to be there is there
and that the stuff that is there works correctly or is reachable at all?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
While debugging some other stuff, I was wondering to what extent node
types supporting utility statements should be supported in outfuncs.c.
We've largely not tried too hard in that department. From a debugging
standpoint it'd be useful to have more complete coverage, but I don't
know how much additional code and maintenance effort would be required
to get to full coverage.
(Hmm .. I think copyfuncs.c does have complete coverage, and it's about
5500 lines vs. 4200 lines for outfuncs.c. So perhaps this would mean
about 30% growth of outfuncs.c and another 20KB or so in the executable.)
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6/13/17 11:25, Peter Eisentraut wrote:
Running with --debug-print-parse=on, executing
create table test1 (a int, b text);
gives output that is truncated somewhere in the middle (possibly a null
byte)
So this seems to be a pretty basic bug. Some node fields of type char
may be zero, and so printing them as a zero byte just truncates the
whole output string. This could be fixed by printing chars like strings
with the full escaping mechanism. See attached patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Fix-output-of-char-node-fields.patchtext/plain; charset=UTF-8; name=0001-Fix-output-of-char-node-fields.patch; x-mac-creator=0; x-mac-type=0Download
From 61c0226abbef26c8df6daa3f56c848a1a1d7e1e7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 13 Jun 2017 23:44:54 -0400
Subject: [PATCH] Fix output of char node fields
WRITE_CHAR_FIELD() didn't do any escaping, so that for example a zero
byte would cause the whole output string to be truncated. To fix, pass
the char through outToken(), so it is escaped like a string. The
reading side already handled this.
---
src/backend/nodes/outfuncs.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index c348bdcde3..46b61f9dbf 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -32,6 +32,8 @@
#include "utils/datum.h"
#include "utils/rel.h"
+static void outChar(StringInfo str, char c);
+
/*
* Macros to simplify output of different kinds of fields. Use these
@@ -62,7 +64,8 @@
/* Write a char field (ie, one ascii character) */
#define WRITE_CHAR_FIELD(fldname) \
- appendStringInfo(str, " :" CppAsString(fldname) " %c", node->fldname)
+ (appendStringInfo(str, " :" CppAsString(fldname) " "), \
+ outChar(str, node->fldname))
/* Write an enumerated-type field as an integer code */
#define WRITE_ENUM_FIELD(fldname, enumtype) \
@@ -140,6 +143,21 @@ outToken(StringInfo str, const char *s)
}
}
+/*
+ * Convert one char. Goes through outToken() so that special characters are
+ * escaped.
+ */
+static void
+outChar(StringInfo str, char c)
+{
+ char in[2];
+
+ in[0] = c;
+ in[1] = '\0';
+
+ outToken(str, in);
+}
+
static void
_outList(StringInfo str, const List *node)
{
--
2.13.1
On 2017/06/14 12:49, Peter Eisentraut wrote:
On 6/13/17 11:25, Peter Eisentraut wrote:
Running with --debug-print-parse=on, executing
create table test1 (a int, b text);
gives output that is truncated somewhere in the middle (possibly a null
byte)So this seems to be a pretty basic bug. Some node fields of type char
may be zero, and so printing them as a zero byte just truncates the
whole output string. This could be fixed by printing chars like strings
with the full escaping mechanism. See attached patch.
+1. I've been meaning to report about
zero-byte-truncating-the-whole-output-string thing for some time now.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jun 13, 2017 at 11:59 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
So this seems to be a pretty basic bug. Some node fields of type char
may be zero, and so printing them as a zero byte just truncates the
whole output string. This could be fixed by printing chars like strings
with the full escaping mechanism. See attached patch.+1. I've been meaning to report about
zero-byte-truncating-the-whole-output-string thing for some time now.
I've been bitten by this, too.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
So this seems to be a pretty basic bug. Some node fields of type char
may be zero, and so printing them as a zero byte just truncates the
whole output string. This could be fixed by printing chars like strings
with the full escaping mechanism. See attached patch.
+1 for fixing this, but you have not handled the read side correctly.
pg_strtok doesn't promise to return a null-terminated string, so without
changes, readfuncs.c would not successfully decode a zero-byte char field.
Also it would do the wrong thing for any character code that outToken had
decided to prefix with a backslash. I think you could fix both problems
like this:
/* Read a char field (ie, one ascii character) */
#define READ_CHAR_FIELD(fldname) \
token = pg_strtok(&length); /* skip :fldname */ \
token = pg_strtok(&length); /* get field value */ \
- local_node->fldname = token[0]
+ local_node->fldname = debackslash(token, length)[0]although that's annoyingly expensive for the normal case where no
special processing is needed. Maybe better
+ local_node->fldname = (length == 0) ? '\0' : (token[0] == '\\') ? token[1] : token[0]
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6/14/17 12:05, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
So this seems to be a pretty basic bug. Some node fields of type char
may be zero, and so printing them as a zero byte just truncates the
whole output string. This could be fixed by printing chars like strings
with the full escaping mechanism. See attached patch.+1 for fixing this, but you have not handled the read side correctly.
pg_strtok doesn't promise to return a null-terminated string, so without
changes, readfuncs.c would not successfully decode a zero-byte char field.
Also it would do the wrong thing for any character code that outToken had
decided to prefix with a backslash. I think you could fix both problems
like this:/* Read a char field (ie, one ascii character) */ #define READ_CHAR_FIELD(fldname) \ token = pg_strtok(&length); /* skip :fldname */ \ token = pg_strtok(&length); /* get field value */ \ - local_node->fldname = token[0] + local_node->fldname = debackslash(token, length)[0]
An empty token produces "<>", so just debackslashing wouldn't work. Maybe
local_node->fldname = length ? token[0] : '\0';
?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
An empty token produces "<>", so just debackslashing wouldn't work.
pg_strtok recognizes "<>" and returns length = 0, so debackslash()
would produce the right answer AFAICS (admittedly, I haven't tested).
But I don't really want to do it like that because of the wasted
palloc space and cycles.
Maybe
local_node->fldname = length ? token[0] : '\0';
?
Doesn't cope with backslash-quoted characters. If we're going to bother
to do anything here, I think we ought to make it reversible for all
possible characters.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6/18/17 10:14, Tom Lane wrote:
pg_strtok recognizes "<>" and returns length = 0, so debackslash()
would produce the right answer AFAICS (admittedly, I haven't tested).
But I don't really want to do it like that because of the wasted
palloc space and cycles.Maybe
local_node->fldname = length ? token[0] : '\0';
?Doesn't cope with backslash-quoted characters. If we're going to bother
to do anything here, I think we ought to make it reversible for all
possible characters.
Makes sense. Updated patch attached.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Fix-output-of-char-node-fields.patchtext/plain; charset=UTF-8; name=v2-0001-Fix-output-of-char-node-fields.patch; x-mac-creator=0; x-mac-type=0Download
From 3b221dc64e1e9eb74b48a97fba4bfa18fad4bf4a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 21 Jun 2017 22:57:23 -0400
Subject: [PATCH v2] Fix output of char node fields
WRITE_CHAR_FIELD() didn't do any escaping, so that for example a zero
byte would cause the whole output string to be truncated. To fix, pass
the char through outToken(), so it is escaped like a string. Adjust the
reading side to handle this.
---
src/backend/nodes/outfuncs.c | 20 +++++++++++++++++++-
src/backend/nodes/readfuncs.c | 3 ++-
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 3a23f0bb16..b0abe9ec10 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -32,6 +32,8 @@
#include "utils/datum.h"
#include "utils/rel.h"
+static void outChar(StringInfo str, char c);
+
/*
* Macros to simplify output of different kinds of fields. Use these
@@ -62,7 +64,8 @@
/* Write a char field (ie, one ascii character) */
#define WRITE_CHAR_FIELD(fldname) \
- appendStringInfo(str, " :" CppAsString(fldname) " %c", node->fldname)
+ (appendStringInfo(str, " :" CppAsString(fldname) " "), \
+ outChar(str, node->fldname))
/* Write an enumerated-type field as an integer code */
#define WRITE_ENUM_FIELD(fldname, enumtype) \
@@ -140,6 +143,21 @@ outToken(StringInfo str, const char *s)
}
}
+/*
+ * Convert one char. Goes through outToken() so that special characters are
+ * escaped.
+ */
+static void
+outChar(StringInfo str, char c)
+{
+ char in[2];
+
+ in[0] = c;
+ in[1] = '\0';
+
+ outToken(str, in);
+}
+
static void
_outList(StringInfo str, const List *node)
{
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 2988e8bd16..1380703cbc 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -86,7 +86,8 @@
#define READ_CHAR_FIELD(fldname) \
token = pg_strtok(&length); /* skip :fldname */ \
token = pg_strtok(&length); /* get field value */ \
- local_node->fldname = token[0]
+ /* avoid overhead of calling debackslash() for one char */ \
+ local_node->fldname = (length == 0) ? '\0' : (token[0] == '\\' ? token[1] : token[0])
/* Read an enumerated-type field that was written as an integer code */
#define READ_ENUM_FIELD(fldname, enumtype) \
--
2.13.1
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 6/18/17 10:14, Tom Lane wrote:
Doesn't cope with backslash-quoted characters. If we're going to bother
to do anything here, I think we ought to make it reversible for all
possible characters.
Makes sense. Updated patch attached.
That looks sane to me, though I've still not actually tested any
interesting cases.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6/21/17 23:40, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 6/18/17 10:14, Tom Lane wrote:
Doesn't cope with backslash-quoted characters. If we're going to bother
to do anything here, I think we ought to make it reversible for all
possible characters.Makes sense. Updated patch attached.
That looks sane to me, though I've still not actually tested any
interesting cases.
I have committed this. I had to build a bunch more stuff around it to
be able to test it, which I will tidy up and submit at some later point.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers