Previous title was: Add minor version to v3 protocol to allow changes without
breaking backwards compatibility
On 01/20/2012 04:45 AM, Noah Misch wrote:
On Thu, Jan 19, 2012 at 02:00:20PM -0500, Robert Haas wrote:
On Thu, Jan 19, 2012 at 10:37 AM, Noah Misch<n...@leadboat.com> wrote:
I agree with Merlin; the frontend/backend protocol is logically distinct from
the binary send/recv formats of data types. ?For one key point, the latter is
not exclusively core-defined; third-party extensions change their send/recv
formats on a different schedule. ?They can add myext.binary_format_version
GUCs of their own to cope in a similar way.
I agree. It occurs to me that we recently changed the default *text*
output format for bytea for reasons not dissimilar to those
contemplated here. Presumably, that's a much more disruptive change,
and yet we've had minimal complaints because anyone who gets bitten
can easily set bytea_output='escape' and the problem goes away. The
same thing seems like it would work here, only the number of people
needing to change the parameter will probably be even smaller, because
fewer people use binary than text.
Having said that, if we're to follow the precedent set by
bytea_format, maybe we ought to just add
binary_array_format={huge,ittybitty} and be done with it, rather than
inventing a minor protocol version GUC for something that isn't really
a protocol version change at all. We could introduce a
differently-named general mechanism, but I guess I'm not seeing the
point of that either. Just because someone has a
backward-compatibility issue with one change of this type doesn't mean
they have a similar issue with all of them. So I think adding a
special-purpose GUC is more logical and more parallel to what we've
done in the past, and it doesn't saddle us with having to be certain
that we've designed the mechanism generally enough to handle all the
cases that may come later.
That makes sense. An attraction of a single binary format version was avoiding
the "Is this worth a GUC?" conversation for each change. However, adding a GUC
should be no more notable than bumping a binary format version.
I see the main difference between the GUC per feature vs minor version being
that
in versioned changes old clients keep working because the have to explicitly
request a specific version. Whereas in separate GUC variables each feature will
be
enabled by default and users have to either keep up with new client versions or
figure out how to explicitly disable the changes.
However, due to popular vote I removed the minor version proposal for now.
Here is a second version of the patch. The binary array encoding changes
stay the same but all code around was rewritten.
Changes from previous versions based on received comments:
* removed the minor protocol version concept
* introduced a new GUC variable array_output copying the current
bytea_output type, with values "full" (old value) and
"smallfixed" (new default)
* added documentation for the new GUC variable
* used constants for the array flags variable values
-Mikko
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index e55b503..179a081
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** COPY postgres_log FROM '/full/path/to/lo
*** 4888,4893 ****
--- 4888,4910 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-array-output" xreflabel="array_output">
+ <term><varname>array_output</varname> (<type>enum</type>)</term>
+ <indexterm>
+ <primary><varname>array_output</> configuration parameter</primary>
+ </indexterm>
+ <listitem>
+ <para>
+ Sets the output format for binary encoding of values of
+ type <type>array</type>. Valid values are
+ <literal>smallfixed</literal> (the default)
+ and <literal>full</literal> (the traditional PostgreSQL
+ format). The <type>array</type> type always
+ accepts both formats on input, regardless of this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-xmlbinary" xreflabel="xmlbinary">
<term><varname>xmlbinary</varname> (<type>enum</type>)</term>
<indexterm>
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
new file mode 100644
index 5582a06..6192a2e
*** a/src/backend/utils/adt/arrayfuncs.c
--- b/src/backend/utils/adt/arrayfuncs.c
***************
*** 30,41 ****
--- 30,45 ----
* GUC parameter
*/
bool Array_nulls = true;
+ int array_output = ARRAY_OUTPUT_SMALLFIXED;
/*
* Local definitions
*/
#define ASSGN "="
+ #define FLAG_HASNULLS 1
+ #define FLAG_FIXEDLEN 2
+
typedef enum
{
ARRAY_NO_LEVEL,
*************** static void ReadArrayBinary(StringInfo b
*** 86,92 ****
FmgrInfo *receiveproc, Oid typioparam, int32 typmod,
int typlen, bool typbyval, char typalign,
Datum *values, bool *nulls,
! bool *hasnulls, int32 *nbytes);
static void CopyArrayEls(ArrayType *array,
Datum *values, bool *nulls, int nitems,
int typlen, bool typbyval, char typalign,
--- 90,96 ----
FmgrInfo *receiveproc, Oid typioparam, int32 typmod,
int typlen, bool typbyval, char typalign,
Datum *values, bool *nulls,
! bool *hasnulls, int32 *nbytes, bool fixedlen);
static void CopyArrayEls(ArrayType *array,
Datum *values, bool *nulls, int nitems,
int typlen, bool typbyval, char typalign,
*************** array_recv(PG_FUNCTION_ARGS)
*** 1242,1248 ****
ndim, MAXDIM)));
flags = pq_getmsgint(buf, 4);
! if (flags != 0 && flags != 1)
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
errmsg("invalid array flags")));
--- 1246,1252 ----
ndim, MAXDIM)));
flags = pq_getmsgint(buf, 4);
! if ((flags & ~(FLAG_HASNULLS|FLAG_FIXEDLEN)) != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
errmsg("invalid array flags")));
*************** array_recv(PG_FUNCTION_ARGS)
*** 1327,1333 ****
&my_extra->proc, typioparam, typmod,
typlen, typbyval, typalign,
dataPtr, nullsPtr,
! &hasnulls, &nbytes);
if (hasnulls)
{
dataoffset = ARR_OVERHEAD_WITHNULLS(ndim, nitems);
--- 1331,1337 ----
&my_extra->proc, typioparam, typmod,
typlen, typbyval, typalign,
dataPtr, nullsPtr,
! &hasnulls, &nbytes, (flags & FLAG_FIXEDLEN) != 0);
if (hasnulls)
{
dataoffset = ARR_OVERHEAD_WITHNULLS(ndim, nitems);
*************** ReadArrayBinary(StringInfo buf,
*** 1390,1396 ****
Datum *values,
bool *nulls,
bool *hasnulls,
! int32 *nbytes)
{
int i;
bool hasnull;
--- 1394,1401 ----
Datum *values,
bool *nulls,
bool *hasnulls,
! int32 *nbytes,
! bool fixedlen)
{
int i;
bool hasnull;
*************** ReadArrayBinary(StringInfo buf,
*** 1403,1409 ****
char csave;
/* Get and check the item length */
! itemlen = pq_getmsgint(buf, 4);
if (itemlen < -1 || itemlen > (buf->len - buf->cursor))
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
--- 1408,1414 ----
char csave;
/* Get and check the item length */
! itemlen = fixedlen ? typlen : pq_getmsgint(buf, 4);
if (itemlen < -1 || itemlen > (buf->len - buf->cursor))
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
*************** array_send(PG_FUNCTION_ARGS)
*** 1496,1501 ****
--- 1501,1507 ----
int bitmask;
int nitems,
i;
+ int flags;
int ndim,
*dim;
StringInfoData buf;
*************** array_send(PG_FUNCTION_ARGS)
*** 1535,1540 ****
--- 1541,1550 ----
typbyval = my_extra->typbyval;
typalign = my_extra->typalign;
+ flags = ARR_HASNULL(v) ? FLAG_HASNULLS : 0;
+ if (array_output == ARRAY_OUTPUT_SMALLFIXED && flags == 0 && typlen > 0)
+ flags |= FLAG_FIXEDLEN;
+
ndim = ARR_NDIM(v);
dim = ARR_DIMS(v);
nitems = ArrayGetNItems(ndim, dim);
*************** array_send(PG_FUNCTION_ARGS)
*** 1543,1549 ****
/* Send the array header information */
pq_sendint(&buf, ndim, 4);
! pq_sendint(&buf, ARR_HASNULL(v) ? 1 : 0, 4);
pq_sendint(&buf, element_type, sizeof(Oid));
for (i = 0; i < ndim; i++)
{
--- 1553,1559 ----
/* Send the array header information */
pq_sendint(&buf, ndim, 4);
! pq_sendint(&buf, flags, 4);
pq_sendint(&buf, element_type, sizeof(Oid));
for (i = 0; i < ndim; i++)
{
*************** array_send(PG_FUNCTION_ARGS)
*** 1571,1577 ****
itemvalue = fetch_att(p, typbyval, typlen);
outputbytes = SendFunctionCall(&my_extra->proc, itemvalue);
! pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
pq_sendbytes(&buf, VARDATA(outputbytes),
VARSIZE(outputbytes) - VARHDRSZ);
pfree(outputbytes);
--- 1581,1588 ----
itemvalue = fetch_att(p, typbyval, typlen);
outputbytes = SendFunctionCall(&my_extra->proc, itemvalue);
! if ((flags & FLAG_FIXEDLEN) == 0)
! pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
pq_sendbytes(&buf, VARDATA(outputbytes),
VARSIZE(outputbytes) - VARHDRSZ);
pfree(outputbytes);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index 9fc96b2..636b8d3
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 64,69 ****
--- 64,70 ----
#include "storage/predicate.h"
#include "tcop/tcopprot.h"
#include "tsearch/ts_cache.h"
+ #include "utils/array.h"
#include "utils/builtins.h"
#include "utils/bytea.h"
#include "utils/guc_tables.h"
*************** static const struct config_enum_entry by
*** 225,230 ****
--- 226,243 ----
};
/*
+ * Options for enum values defined in this module.
+ *
+ * NOTE! Option values may not contain double quotes!
+ */
+
+ static const struct config_enum_entry array_output_options[] = {
+ {"full", ARRAY_OUTPUT_FULL, false},
+ {"smallfixed", ARRAY_OUTPUT_SMALLFIXED, false},
+ {NULL, 0, false}
+ };
+
+ /*
* We have different sets for client and server message level options because
* they sort slightly different (see "log" level)
*/
*************** static struct config_enum ConfigureNames
*** 3047,3052 ****
--- 3060,3075 ----
NULL, NULL, NULL
},
+ {
+ {"array_output", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
+ gettext_noop("Sets the binary output format for arrays."),
+ NULL
+ },
+ &bytea_output,
+ ARRAY_OUTPUT_SMALLFIXED, array_output_options,
+ NULL, NULL, NULL
+ },
+
{
{"client_min_messages", PGC_USERSET, LOGGING_WHEN,
gettext_noop("Sets the message levels that are sent to the client."),
diff --git a/src/include/utils/array.h b/src/include/utils/array.h
new file mode 100644
index c6d0ad6..58658f6
*** a/src/include/utils/array.h
--- b/src/include/utils/array.h
***************
*** 58,63 ****
--- 58,71 ----
#include "fmgr.h"
+ typedef enum
+ {
+ ARRAY_OUTPUT_FULL,
+ ARRAY_OUTPUT_SMALLFIXED
+ } ArrayOutputType;
+
+ extern int array_output; /* ArrayOutputType, but int for GUC enum */
+
/*
* Arrays are varlena objects, so must meet the varlena convention that
* the first int32 of the object contains the total object size in bytes.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
