Horton

Re: Compressed TOAST Slicing

Stephen Frost <sfrost@snowman.net>

View thread
From: Stephen Frost <sfrost@snowman.net>
To: Paul Ramsey <pramsey@cleverelephant.ca>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>, Regina Obe <r@pcorp.us>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2019-03-19T11:47:45Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add support for partial TOAST decompression

  2. Remove remaining hard-wired OID references in the initial catalog data.

  3. Rephrase references to "time qualification".

Greetings,

* Paul Ramsey (pramsey@cleverelephant.ca) wrote:
> > On Mar 18, 2019, at 7:34 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> > +1.  I think Paul had it right originally.
> 
> In that spirit, here is a “one pglz_decompress function, new parameter” version for commit.

Alright, I've been working through this and have made a few improvements
(the big comment block at the top of pg_lzcompress.c needed updating,
among a couple other minor things), but I was trying to wrap my head
around this:

+/* ----------
+ * toast_decompress_datum_slice -
+ *
+ * Decompress the front of a compressed version of a varlena datum.
+ * offset handling happens in heap_tuple_untoast_attr_slice.
+ * Here we just decompress a slice from the front.
+ */
+static struct varlena *
+toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
+{
+	struct varlena *result;
+	int32 rawsize;
+
+	Assert(VARATT_IS_COMPRESSED(attr));
+
+	result = (struct varlena *) palloc(slicelength + VARHDRSZ);
+	SET_VARSIZE(result, slicelength + VARHDRSZ);
+
+	rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
+						VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
+						VARDATA(result),
+						slicelength, false);
+	if (rawsize < 0)
 		elog(ERROR, "compressed data is corrupted");
 
+	SET_VARSIZE(result, rawsize + VARHDRSZ);
 	return result;
 }

Specifically, the two SET_VARSIZE() calls, do we really need both..?
Are we sure that we're setting the length correctly there..?  Is there
any cross-check we can do?
 
I have to admit that I find the new argument to pglz_decompress() a bit
awkward to describe and document; if you have any thoughts as to how
that could be improved, that'd be great.

Thanks!

Stephen