Re: [PATCHES] Eliminate more detoast copies for packed varlenas

2007-09-27 Thread Brendan Jurd 
On 9/22/07, Gregory Stark [EMAIL PROTECTED] wrote:
 Ok, this removes what should be most if not all of the call sites where we're
 detoasting text or byteas. In particular it gets all the regexp/like functions
 and all the trim/pad functions. It also gets hashtext and hash_any.

Looks like there's some more of this in src/tutorial/funcs.c and funcs_new.c.

On a related note, while I was trawling through header files trying to
wrap my head around all this toast and varlena business, I found the
following comment, in fmgr.h and reiterated in postgres.h:


WARNING: It is only safe to use PG_DETOAST_DATUM_UNPACKED() and
VARDATA_ANY() if you really don't care about the alignment.
/

Shouldn't this be PG_DETOAST_DATUM_PACKED()?  I'm emboldened by the
fact that there is no macro called PG_TOAST_DATUM_UNPACKED defined
anywhere in postgres.

Patch attached, in case I've got the right idea.

Regards,
BJ
Index: src/include/fmgr.h
===
--- src/include/fmgr.h  (revision 29144)
+++ src/include/fmgr.h  (working copy)
@@ -158,11 +158,11 @@
  * The resulting datum can be accessed using VARSIZE_ANY() and VARDATA_ANY()
  * (beware of multiple evaluations in those macros!)
  *
- * WARNING: It is only safe to use PG_DETOAST_DATUM_UNPACKED() and
- * VARDATA_ANY() if you really don't care about the alignment. Either because
- * you're working with something like text where the alignment doesn't matter
- * or because you're not going to access its constituent parts and just use
- * things like memcpy on it anyways.
+ * WARNING: It is only safe to use PG_DETOAST_DATUM_PACKED() and VARDATA_ANY()
+ * if you really don't care about the alignment. Either because you're working
+ * with something like text where the alignment doesn't matter or because
+ * you're not going to access its constituent parts and just use things like
+ * memcpy on it anyways.
  *
  * Note: it'd be nice if these could be macros, but I see no way to do that
  * without evaluating the arguments multiple times, which is NOT acceptable.
Index: src/include/postgres.h
===
--- src/include/postgres.h  (revision 29144)
+++ src/include/postgres.h  (working copy)
@@ -237,7 +237,7 @@
  * code that specifically wants to work with still-toasted Datums.
  *
  * WARNING: It is only safe to use VARDATA_ANY() -- typically with
- * PG_DETOAST_DATUM_UNPACKED() -- if you really don't care about the alignment.
+ * PG_DETOAST_DATUM_PACKED() -- if you really don't care about the alignment.
  * Either because you're working with something like text where the alignment
  * doesn't matter or because you're not going to access its constituent parts
  * and just use things like memcpy on it anyways.

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] Eliminate more detoast copies for packed varlenas

2007-09-27 Thread Tom Lane 
Brendan Jurd [EMAIL PROTECTED] writes:
 On 9/22/07, Gregory Stark [EMAIL PROTECTED] wrote:
 Ok, this removes what should be most if not all of the call sites where we're
 detoasting text or byteas. In particular it gets all the regexp/like 
 functions
 and all the trim/pad functions. It also gets hashtext and hash_any.

 Looks like there's some more of this in src/tutorial/funcs.c and funcs_new.c.

Well, those are just tutorial code, so I'd vote for keeping it simple.

 On a related note, while I was trawling through header files trying to
 wrap my head around all this toast and varlena business, I found the
 following comment, in fmgr.h and reiterated in postgres.h:
 WARNING: It is only safe to use PG_DETOAST_DATUM_UNPACKED() and
 VARDATA_ANY() if you really don't care about the alignment.
 Shouldn't this be PG_DETOAST_DATUM_PACKED()?

Yup, good catch.  In the context of fmgr.h, it seems like the comment
is more sensible if it refers to the underlying function
pg_detoast_datum_packed (which is what the preceding paragraph is
speaking of), so I made it say that instead.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

[PATCHES] Eliminate more detoast copies for packed varlenas

2007-09-21 Thread Gregory Stark 

Ok, this removes what should be most if not all of the call sites where we're
detoasting text or byteas. In particular it gets all the regexp/like functions
and all the trim/pad functions. It also gets hashtext and hash_any.


$ zcat packed-varlena-efficiency_v0.patch.gz | diffstat
 backend/access/hash/hashfunc.c|   12 !!
 backend/utils/adt/like.c  |   80 !!!
 backend/utils/adt/oracle_compat.c |  157 !!
 backend/utils/adt/regexp.c|  119 
 include/fmgr.h|1 
 5 files changed, 5 insertions(+), 364 modifications(!)



packed-varlena-efficiency_v0.patch.gz
Description: Binary data

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] Eliminate more detoast copies for packed varlenas

2007-09-21 Thread Tom Lane 
Gregory Stark [EMAIL PROTECTED] writes:
 Ok, this removes what should be most if not all of the call sites where we're
 detoasting text or byteas. In particular it gets all the regexp/like functions
 and all the trim/pad functions. It also gets hashtext and hash_any.

Applied with some fixes --- you'd missed like_match.c, which doubtless
explains Guillame's complaint that it didn't work ...

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org