On 08.11.2011 11:22, Boszormenyi Zoltan wrote:
Hi,
I wanted to do some transformation on an inet value from
an SPI-using function. The inet Datum passed from SPI_getbinval()
to the values array in heap_form_tuple() obviously gives good data
to the frontend. When I use DatumGetInetP() on the Datum,
the structure passed to me is corrupt:
zozo=# select * from inet_test() as (id integer, i1 inet, i2 inet);
NOTICE: i1 family=CORRUPTED
NOTICE: i1 family=CORRUPTED
NOTICE: i1 family=CORRUPTED
id | i1 | i2
----+-------------+---------------
1 | 192.168.0.1 | 192.168.0.101
2 | 192.168.0.2 | 192.168.0.102
3 | 192.168.0.3 | 192.168.0.103
(3 rows)
I looked at utils/inet.h and DatumGetInetP() uses PG_DETOAST_DATUM_PACKED().
fmgr.h warns about PG_DETOAST_DATUM_PACKED() that it may give you
an unaligned pointer. Indeed, using PG_DETOAST_DATUM() instead of the
_PACKED variant on the Datum give me a well formed inet structure:
Hmm, it seems to be intentional, but I agree it's very much contrary to
the usual convention that DatumGetXXXP() returns a detoasted and
depacked datum. I think we should change it. I propose the attached
patch. It changes DatumGetInetP() to do PG_DETOAST_DATUM(), and adds new
DatumGetInetPP() macro to return the packed version. I also moved the
access macros like ip_family() from network.c to inet.h, so that they're
available for whoever wants to look at the fields without having to depack.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c
index 9aca1cc..a276d04 100644
--- a/src/backend/utils/adt/network.c
+++ b/src/backend/utils/adt/network.c
@@ -29,37 +29,6 @@ static int ip_addrsize(inet *inetptr);
static inet *internal_inetpl(inet *ip, int64 addend);
/*
- * Access macros. We use VARDATA_ANY so that we can process short-header
- * varlena values without detoasting them. This requires a trick:
- * VARDATA_ANY assumes the varlena header is already filled in, which is
- * not the case when constructing a new value (until SET_INET_VARSIZE is
- * called, which we typically can't do till the end). Therefore, we
- * always initialize the newly-allocated value to zeroes (using palloc0).
- * A zero length word will look like the not-1-byte case to VARDATA_ANY,
- * and so we correctly construct an uncompressed value.
- *
- * Note that ip_maxbits() and SET_INET_VARSIZE() require
- * the family field to be set correctly.
- */
-
-#define ip_family(inetptr) \
- (((inet_struct *) VARDATA_ANY(inetptr))->family)
-
-#define ip_bits(inetptr) \
- (((inet_struct *) VARDATA_ANY(inetptr))->bits)
-
-#define ip_addr(inetptr) \
- (((inet_struct *) VARDATA_ANY(inetptr))->ipaddr)
-
-#define ip_maxbits(inetptr) \
- (ip_family(inetptr) == PGSQL_AF_INET ? 32 : 128)
-
-#define SET_INET_VARSIZE(dst) \
- SET_VARSIZE(dst, VARHDRSZ + offsetof(inet_struct, ipaddr) + \
- ip_addrsize(dst))
-
-
-/*
* Return the number of bytes of address storage needed for this data type.
*/
static int
@@ -907,7 +876,7 @@ convert_network_to_scalar(Datum value, Oid typid)
case INETOID:
case CIDROID:
{
- inet *ip = DatumGetInetP(value);
+ inet *ip = DatumGetInetPP(value);
int len;
double res;
int i;
diff --git a/src/include/utils/inet.h b/src/include/utils/inet.h
index 9626a2d..7cb7337 100644
--- a/src/include/utils/inet.h
+++ b/src/include/utils/inet.h
@@ -53,6 +53,36 @@ typedef struct
inet_struct inet_data;
} inet;
+/*
+ * Access macros. We use VARDATA_ANY so that we can process short-header
+ * varlena values without detoasting them. This requires a trick:
+ * VARDATA_ANY assumes the varlena header is already filled in, which is
+ * not the case when constructing a new value (until SET_INET_VARSIZE is
+ * called, which we typically can't do till the end). Therefore, we
+ * always initialize the newly-allocated value to zeroes (using palloc0).
+ * A zero length word will look like the not-1-byte case to VARDATA_ANY,
+ * and so we correctly construct an uncompressed value.
+ *
+ * Note that ip_maxbits() and SET_INET_VARSIZE() require
+ * the family field to be set correctly.
+ */
+
+#define ip_family(inetptr) \
+ (((inet_struct *) VARDATA_ANY(inetptr))->family)
+
+#define ip_bits(inetptr) \
+ (((inet_struct *) VARDATA_ANY(inetptr))->bits)
+
+#define ip_addr(inetptr) \
+ (((inet_struct *) VARDATA_ANY(inetptr))->ipaddr)
+
+#define ip_maxbits(inetptr) \
+ (ip_family(inetptr) == PGSQL_AF_INET ? 32 : 128)
+
+#define SET_INET_VARSIZE(dst) \
+ SET_VARSIZE(dst, VARHDRSZ + offsetof(inet_struct, ipaddr) + \
+ ip_addrsize(dst))
+
/*
* This is the internal storage format for MAC addresses:
@@ -70,9 +100,11 @@ typedef struct macaddr
/*
* fmgr interface macros
*/
-#define DatumGetInetP(X) ((inet *) PG_DETOAST_DATUM_PACKED(X))
+#define DatumGetInetP(X) ((inet *) PG_DETOAST_DATUM(X))
+#define DatumGetInetPP(X) ((inet *) PG_DETOAST_DATUM_PACKED(X))
#define InetPGetDatum(X) PointerGetDatum(X)
#define PG_GETARG_INET_P(n) DatumGetInetP(PG_GETARG_DATUM(n))
+#define PG_GETARG_INET_PP(n) DatumGetInetP(PG_GETARG_DATUM_PACKED(n))
#define PG_RETURN_INET_P(x) return InetPGetDatum(x)
/* macaddr is a fixed-length pass-by-reference datatype */
#define DatumGetMacaddrP(X) ((macaddr *) DatumGetPointer(X))
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
