Hi,

As discussed few days ago it would be beneficial if we could change the v3 
backend<->client protocol without breaking backwards compatibility.

Here is a working patch that exports a supported_binary_minor constant and a binary_minor session variable and a that can be used by clients to enable newer features.

I also added an example usage where the array encoding for constant size 
elements is optimized if binary_minor version is new enough.

I have coded the client side support for binary_minor for jdbc driver and 
tested that it worked. But lets first discuss if this is an acceptable path 
forward.

On 11/25/2011 02:20 AM, Oliver Jowett wrote:
> Re list vs. always-incrementing minor version, you could just use an
> integer and set bits to represent features, which would keep it simple
> but also let clients be more selective about which features they
> implement (you could support feature 21 and 23 without supporting 22)

I decided not to use a feature flag because when features start to depend on each other we need multiple negotiation round trips until the final feature set can be known. If in your example above the feature 23 depends on server side on feature 22, but the client only requests 21,23. The server must inform back that combination 21,23 is reduced to 21. And if then the client can not support 21 without 23 the final feature set will not contain 21 or 23.

-Mikko
commit d7ef5f1aef0941ec4b931f24745b65d77e7511e4
Author: Mikko Tiihonen <mikko.tiiho...@nitor.fi>
Date:   Sun Nov 27 14:12:59 2011 +0200

    Define binary_minor variable to control binary protocol minor version

diff --git a/src/backend/utils/adt/version.c b/src/backend/utils/adt/version.c
index 6ea5bd2..33ed4d3 100644
--- a/src/backend/utils/adt/version.c
+++ b/src/backend/utils/adt/version.c
@@ -16,6 +16,7 @@
 
 #include "utils/builtins.h"
 
+int binary_minor = 0;
 
 Datum
 pgsql_version(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index da7b6d4..67e80f1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -64,6 +64,7 @@
 #include "storage/predicate.h"
 #include "tcop/tcopprot.h"
 #include "tsearch/ts_cache.h"
+#include "utils/binaryminorversion.h"
 #include "utils/builtins.h"
 #include "utils/bytea.h"
 #include "utils/guc_tables.h"
@@ -2053,6 +2054,18 @@ static struct config_int ConfigureNamesInt[] =
        },
 
        {
+               {"binary_minor", PGC_USERSET, CLIENT_CONN_LOCALE,
+                       gettext_noop("Sets the binary protocol minor version 
that controls enabling"
+                                                "of newer features."),
+                       gettext_noop("The default value is 0 which uses the 
binary protocol features"
+                                                "as specified in postgres 9.1 
release.")
+               },
+               &binary_minor,
+               0, 0, SUPPORTED_BINARY_MINOR_VERSION,
+               NULL, NULL, NULL
+       },
+
+       {
                {"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN,
                        gettext_noop("Sets the minimum execution time above 
which "
                                                 "statements will be logged."),
diff --git a/src/include/utils/binaryminorversion.h 
b/src/include/utils/binaryminorversion.h
new file mode 100644
index 0000000..40ba970
--- /dev/null
+++ b/src/include/utils/binaryminorversion.h
@@ -0,0 +1,32 @@
+/*-------------------------------------------------------------------------
+ *
+ * binaryminorversion.h
+ *       "Binary protocol encoding minor version number" for PostgreSQL.
+ *
+ * The catalog version number is used to flag incompatible changes in
+ * the PostgreSQL v3 binary protocol.  Whenever anyone changes the
+ * format of binary protocol, or modifies the standard types in such a
+ * way that an updated client wouldn't work with an old database
+ * (or vice versa), the binary protocol encoding version number
+ * should be changed.
+ *
+ * The point of this feature is to provide a way to allow introducing
+ * small new features to the binary encoding of types or the actual
+ * v3 protocol while still allowing full backward compatibility with
+ * old clients. The client can be an application using postgres,
+ * any tool using COPY BINARY or another postgres backend using 
+ * replication (TODO: does this affect replication?).
+ *
+ * Portions Copyright (c) 1996-2011, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/util/binprotoversion.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef BINARYMINORVERSION_H
+#define BINARYMINORVERSION_H
+
+#define SUPPORTED_BINARY_MINOR_VERSION 1
+
+#endif
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 47a1412..c201d66 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -461,6 +461,8 @@ extern Datum pg_sleep(PG_FUNCTION_ARGS);
 extern Datum pg_get_keywords(PG_FUNCTION_ARGS);
 extern Datum pg_typeof(PG_FUNCTION_ARGS);
 
+extern PGDLLIMPORT int binary_minor;
+
 /* oid.c */
 extern Datum oidin(PG_FUNCTION_ARGS);
 extern Datum oidout(PG_FUNCTION_ARGS);
commit 06af85f130e2150f3ab5bcbd2c49dbecb69d98ec
Author: Mikko Tiihonen <mikko.tiiho...@nitor.fi>
Date:   Mon Nov 28 09:53:01 2011 +0200

    Add supported_binary_minor startup option

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 67e80f1..6e5c908 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -477,6 +477,7 @@ static int  segment_size;
 static int     wal_block_size;
 static int     wal_segment_size;
 static bool integer_datetimes;
+static int  supported_binary_minor;
 static int     effective_io_concurrency;
 
 /* should be static, but commands/variable.c needs to get at this */
@@ -1976,6 +1977,19 @@ static struct config_int ConfigureNamesInt[] =
        },
 
        {
+               {"supported_binary_minor", PGC_INTERNAL, PRESET_OPTIONS,
+                       gettext_noop("Maximum supported binary minor version."),
+                       NULL,
+                       GUC_REPORT | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+               },
+               &supported_binary_minor,
+               SUPPORTED_BINARY_MINOR_VERSION,
+               SUPPORTED_BINARY_MINOR_VERSION,
+               SUPPORTED_BINARY_MINOR_VERSION,
+               NULL, NULL, NULL
+       },
+
+       {
                {"wal_buffers", PGC_POSTMASTER, WAL_SETTINGS,
                        gettext_noop("Sets the number of disk-page buffers in 
shared memory for WAL."),
                        NULL,
diff --git a/src/backend/utils/adt/arrayfuncs.c 
b/src/backend/utils/adt/arrayfuncs.c
index bfb6065..d1028be 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -86,7 +86,7 @@ static void ReadArrayBinary(StringInfo buf, int nitems,
                                FmgrInfo *receiveproc, Oid typioparam, int32 
typmod,
                                int typlen, bool typbyval, char typalign,
                                Datum *values, bool *nulls,
-                               bool *hasnulls, int32 *nbytes);
+                               bool *hasnulls, int32 *nbytes, bool fixedlen);
 static void CopyArrayEls(ArrayType *array,
                         Datum *values, bool *nulls, int nitems,
                         int typlen, bool typbyval, char typalign,
@@ -1242,7 +1242,7 @@ array_recv(PG_FUNCTION_ARGS)
                                                ndim, MAXDIM)));
 
        flags = pq_getmsgint(buf, 4);
-       if (flags != 0 && flags != 1)
+       if ((flags & ~3) != 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
                                 errmsg("invalid array flags")));
@@ -1327,7 +1327,7 @@ array_recv(PG_FUNCTION_ARGS)
                                        &my_extra->proc, typioparam, typmod,
                                        typlen, typbyval, typalign,
                                        dataPtr, nullsPtr,
-                                       &hasnulls, &nbytes);
+                                       &hasnulls, &nbytes, (flags & 2) != 0);
        if (hasnulls)
        {
                dataoffset = ARR_OVERHEAD_WITHNULLS(ndim, nitems);
@@ -1390,7 +1390,8 @@ ReadArrayBinary(StringInfo buf,
                                Datum *values,
                                bool *nulls,
                                bool *hasnulls,
-                               int32 *nbytes)
+                               int32 *nbytes,
+                               bool fixedlen)
 {
        int                     i;
        bool            hasnull;
@@ -1403,7 +1404,7 @@ ReadArrayBinary(StringInfo buf,
                char            csave;
 
                /* Get and check the item length */
-               itemlen = pq_getmsgint(buf, 4);
+               itemlen = fixedlen ? typlen : pq_getmsgint(buf, 4);
                if (itemlen < -1 || itemlen > (buf->len - buf->cursor))
                        ereport(ERROR,
                                        
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
@@ -1496,6 +1497,7 @@ array_send(PG_FUNCTION_ARGS)
        int                     bitmask;
        int                     nitems,
                                i;
+       int                     flags;
        int                     ndim,
                           *dim;
        StringInfoData buf;
@@ -1535,6 +1537,10 @@ array_send(PG_FUNCTION_ARGS)
        typbyval = my_extra->typbyval;
        typalign = my_extra->typalign;
 
+       flags = ARR_HASNULL(v) ? 1 : 0;
+       if (binary_minor >= 1 && flags == 0 && typlen > 0)
+               flags |= 2;
+
        ndim = ARR_NDIM(v);
        dim = ARR_DIMS(v);
        nitems = ArrayGetNItems(ndim, dim);
@@ -1543,7 +1549,7 @@ array_send(PG_FUNCTION_ARGS)
 
        /* Send the array header information */
        pq_sendint(&buf, ndim, 4);
-       pq_sendint(&buf, ARR_HASNULL(v) ? 1 : 0, 4);
+       pq_sendint(&buf, flags, 4);
        pq_sendint(&buf, element_type, sizeof(Oid));
        for (i = 0; i < ndim; i++)
        {
@@ -1571,7 +1577,8 @@ array_send(PG_FUNCTION_ARGS)
 
                        itemvalue = fetch_att(p, typbyval, typlen);
                        outputbytes = SendFunctionCall(&my_extra->proc, 
itemvalue);
-                       pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
+                       if ((flags & 2) == 0)
+                               pq_sendint(&buf, VARSIZE(outputbytes) - 
VARHDRSZ, 4);
                        pq_sendbytes(&buf, VARDATA(outputbytes),
                                                 VARSIZE(outputbytes) - 
VARHDRSZ);
                        pfree(outputbytes);
-- 
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