Skip to content

P
Postgres FD Implementation
Commit d5406dea authored by Robert Haas's avatar Robert Haas
Browse files
Options

Code cleanup for toast_fetch_datum and toast_fetch_datum_slice. 

Rework some of the checks for bad TOAST chunks to be a bit simpler
and easier to understand. These checks verify that (1) we get all
and only the chunk numbers we expect to see and (2) each chunk has
the expected size. However, the existing code was a bit hard to
understand, at least for me; try to make it clearer.

As part of that, have toast_fetch_datum_slice check the relationship
between endchunk and totalchunks only with an Assert() rather than
checking every chunk number against both values. There's no need to
check that relationship in production builds because it's not a
function of whether on-disk corruption is present; it's just a
question of whether the code does the right math.

Also, have toast_fetch_datum_slice() use ereport(ERROR) rather than
elog(ERROR). Commit fd6ec93b made
the two functions inconsistent with each other.

Rename assorted variables for better clarity and consistency, and
move assorted variables from function scope to the function's main
loop. Remove a few variables that are used only once entirely.

Patch by me, reviewed by Peter Eisentraut.

Discussion: http://postgr.es/m/CA+TgmobBzxwFojJ0zV0Own3dr09y43hp+OzU2VW+nos4PMXWEg@mail.gmail.com
parent da41d710
Hide whitespace changes
Inline Side-by-side
Showing with 102 additions and 118 deletions
src/backend/access/common/detoast.c
View file @ d5406dea
...@@ -25,7 +25,8 @@ ...@@ -25,7 +25,8 @@
static struct varlena *toast_fetch_datum(struct varlena *attr); static struct varlena *toast_fetch_datum(struct varlena *attr);
static struct varlena *toast_fetch_datum_slice(struct varlena *attr, static struct varlena *toast_fetch_datum_slice(struct varlena *attr,
int32 sliceoffset, int32 length); int32 sliceoffset,
int32 slicelength);
static struct varlena *toast_decompress_datum(struct varlena *attr); static struct varlena *toast_decompress_datum(struct varlena *attr);
static struct varlena *toast_decompress_datum_slice(struct varlena *attr, int32 slicelength); static struct varlena *toast_decompress_datum_slice(struct varlena *attr, int32 slicelength);
...@@ -331,14 +332,9 @@ toast_fetch_datum(struct varlena *attr) ...@@ -331,14 +332,9 @@ toast_fetch_datum(struct varlena *attr)
TupleDesc toasttupDesc; TupleDesc toasttupDesc;
struct varlena *result; struct varlena *result;
struct varatt_external toast_pointer; struct varatt_external toast_pointer;
int32 ressize; int32 attrsize;
int32 residx, int32 expectedchunk;
nextidx; int32 totalchunks;
int32 numchunks;
Pointer chunk;
bool isnull;
char *chunkdata;
int32 chunksize;
int num_indexes; int num_indexes;
int validIndex; int validIndex;
SnapshotData SnapshotToast; SnapshotData SnapshotToast;
...@@ -349,15 +345,15 @@ toast_fetch_datum(struct varlena *attr) ...@@ -349,15 +345,15 @@ toast_fetch_datum(struct varlena *attr)
/* Must copy to access aligned fields */ /* Must copy to access aligned fields */
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
ressize = toast_pointer.va_extsize; attrsize = toast_pointer.va_extsize;
numchunks = ((ressize - 1) / TOAST_MAX_CHUNK_SIZE) + 1; totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
result = (struct varlena *) palloc(ressize + VARHDRSZ); result = (struct varlena *) palloc(attrsize + VARHDRSZ);
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
SET_VARSIZE_COMPRESSED(result, ressize + VARHDRSZ); SET_VARSIZE_COMPRESSED(result, attrsize + VARHDRSZ);
else else
SET_VARSIZE(result, ressize + VARHDRSZ); SET_VARSIZE(result, attrsize + VARHDRSZ);
/* /*
* Open the toast relation and its indexes * Open the toast relation and its indexes
...@@ -386,17 +382,24 @@ toast_fetch_datum(struct varlena *attr) ...@@ -386,17 +382,24 @@ toast_fetch_datum(struct varlena *attr)
* see the chunks in chunkidx order, even though we didn't explicitly ask * see the chunks in chunkidx order, even though we didn't explicitly ask
* for it. * for it.
*/ */
nextidx = 0; expectedchunk = 0;
init_toast_snapshot(&SnapshotToast); init_toast_snapshot(&SnapshotToast);
toastscan = systable_beginscan_ordered(toastrel, toastidxs[validIndex], toastscan = systable_beginscan_ordered(toastrel, toastidxs[validIndex],
&SnapshotToast, 1, &toastkey); &SnapshotToast, 1, &toastkey);
while ((ttup = systable_getnext_ordered(toastscan, ForwardScanDirection)) != NULL) while ((ttup = systable_getnext_ordered(toastscan, ForwardScanDirection)) != NULL)
{ {
int32 curchunk;
Pointer chunk;
bool isnull;
char *chunkdata;
int32 chunksize;
int32 expected_size;
/* /*
* Have a chunk, extract the sequence number and the data * Have a chunk, extract the sequence number and the data
*/ */
residx = DatumGetInt32(fastgetattr(ttup, 2, toasttupDesc, &isnull)); curchunk = DatumGetInt32(fastgetattr(ttup, 2, toasttupDesc, &isnull));
Assert(!isnull); Assert(!isnull);
chunk = DatumGetPointer(fastgetattr(ttup, 3, toasttupDesc, &isnull)); chunk = DatumGetPointer(fastgetattr(ttup, 3, toasttupDesc, &isnull));
Assert(!isnull); Assert(!isnull);
...@@ -424,63 +427,50 @@ toast_fetch_datum(struct varlena *attr) ...@@ -424,63 +427,50 @@ toast_fetch_datum(struct varlena *attr)
/* /*
* Some checks on the data we've found * Some checks on the data we've found
*/ */
if (residx != nextidx) if (curchunk != expectedchunk)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED), (errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("unexpected chunk number %d (expected %d) for toast value %u in %s", errmsg_internal("unexpected chunk number %d (expected %d) for toast value %u in %s",
residx, nextidx, curchunk, expectedchunk,
toast_pointer.va_valueid, toast_pointer.va_valueid,
RelationGetRelationName(toastrel)))); RelationGetRelationName(toastrel))));
if (residx < numchunks - 1) if (curchunk > totalchunks - 1)
{
if (chunksize != TOAST_MAX_CHUNK_SIZE)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("unexpected chunk size %d (expected %d) in chunk %d of %d for toast value %u in %s",
chunksize, (int) TOAST_MAX_CHUNK_SIZE,
residx, numchunks,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel))));
}
else if (residx == numchunks - 1)
{
if ((residx * TOAST_MAX_CHUNK_SIZE + chunksize) != ressize)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("unexpected chunk size %d (expected %d) in final chunk %d for toast value %u in %s",
chunksize,
(int) (ressize - residx * TOAST_MAX_CHUNK_SIZE),
residx,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel))));
}
else
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED), (errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("unexpected chunk number %d (out of range %d..%d) for toast value %u in %s", errmsg_internal("unexpected chunk number %d (out of range %d..%d) for toast value %u in %s",
residx, curchunk,
0, numchunks - 1, 0, totalchunks - 1,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel))));
expected_size = curchunk < totalchunks - 1 ? TOAST_MAX_CHUNK_SIZE
: attrsize % TOAST_MAX_CHUNK_SIZE;
if (chunksize != expected_size)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("unexpected chunk size %d (expected %d) in chunk %d of %d for toast value %u in %s",
chunksize, expected_size,
curchunk, totalchunks,
toast_pointer.va_valueid, toast_pointer.va_valueid,
RelationGetRelationName(toastrel)))); RelationGetRelationName(toastrel))));
/* /*
* Copy the data into proper place in our result * Copy the data into proper place in our result
*/ */
memcpy(VARDATA(result) + residx * TOAST_MAX_CHUNK_SIZE, memcpy(VARDATA(result) + curchunk * TOAST_MAX_CHUNK_SIZE,
chunkdata, chunkdata,
chunksize); chunksize);
nextidx++; expectedchunk++;
} }
/* /*
* Final checks that we successfully fetched the datum * Final checks that we successfully fetched the datum
*/ */
if (nextidx != numchunks) if (expectedchunk != totalchunks)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED), (errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("missing chunk number %d for toast value %u in %s", errmsg_internal("missing chunk number %d for toast value %u in %s",
nextidx, expectedchunk,
toast_pointer.va_valueid, toast_pointer.va_valueid,
RelationGetRelationName(toastrel)))); RelationGetRelationName(toastrel))));
...@@ -506,7 +496,8 @@ toast_fetch_datum(struct varlena *attr) ...@@ -506,7 +496,8 @@ toast_fetch_datum(struct varlena *attr)
* ---------- * ----------
*/ */
static struct varlena * static struct varlena *
toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset,
int32 slicelength)
{ {
Relation toastrel; Relation toastrel;
Relation *toastidxs; Relation *toastidxs;
...@@ -518,20 +509,10 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) ...@@ -518,20 +509,10 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
struct varlena *result; struct varlena *result;
struct varatt_external toast_pointer; struct varatt_external toast_pointer;
int32 attrsize; int32 attrsize;
int32 residx; int32 expectedchunk;
int32 nextidx;
int numchunks;
int startchunk; int startchunk;
int endchunk; int endchunk;
int32 startoffset;
int32 endoffset;
int totalchunks; int totalchunks;
Pointer chunk;
bool isnull;
char *chunkdata;
int32 chunksize;
int32 chcpystrt;
int32 chcpyend;
int num_indexes; int num_indexes;
int validIndex; int validIndex;
SnapshotData SnapshotToast; SnapshotData SnapshotToast;
...@@ -555,7 +536,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) ...@@ -555,7 +536,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
if (sliceoffset >= attrsize) if (sliceoffset >= attrsize)
{ {
sliceoffset = 0; sliceoffset = 0;
length = 0; slicelength = 0;
} }
/* /*
...@@ -563,28 +544,25 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) ...@@ -563,28 +544,25 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
* rawsize tracking amount of raw data, which is stored at the beginning * rawsize tracking amount of raw data, which is stored at the beginning
* as an int32 value). * as an int32 value).
*/ */
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0) if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && slicelength > 0)
length = length + sizeof(int32); slicelength = slicelength + sizeof(int32);
if (((sliceoffset + length) > attrsize) || length < 0) if (((sliceoffset + slicelength) > attrsize) || slicelength < 0)
length = attrsize - sliceoffset; slicelength = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ); result = (struct varlena *) palloc(slicelength + VARHDRSZ);
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ); SET_VARSIZE_COMPRESSED(result, slicelength + VARHDRSZ);
else else
SET_VARSIZE(result, length + VARHDRSZ); SET_VARSIZE(result, slicelength + VARHDRSZ);
if (length == 0) if (slicelength == 0)
return result; /* Can save a lot of work at this point! */ return result; /* Can save a lot of work at this point! */
startchunk = sliceoffset / TOAST_MAX_CHUNK_SIZE; startchunk = sliceoffset / TOAST_MAX_CHUNK_SIZE;
endchunk = (sliceoffset + length - 1) / TOAST_MAX_CHUNK_SIZE; endchunk = (sliceoffset + slicelength - 1) / TOAST_MAX_CHUNK_SIZE;
numchunks = (endchunk - startchunk) + 1; Assert(endchunk < totalchunks);
startoffset = sliceoffset % TOAST_MAX_CHUNK_SIZE;
endoffset = (sliceoffset + length - 1) % TOAST_MAX_CHUNK_SIZE;
/* /*
* Open the toast relation and its indexes * Open the toast relation and its indexes
...@@ -610,7 +588,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) ...@@ -610,7 +588,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
/* /*
* Use equality condition for one chunk, a range condition otherwise: * Use equality condition for one chunk, a range condition otherwise:
*/ */
if (numchunks == 1) if (startchunk == endchunk)
{ {
ScanKeyInit(&toastkey[1], ScanKeyInit(&toastkey[1],
(AttrNumber) 2, (AttrNumber) 2,
...@@ -637,15 +615,24 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) ...@@ -637,15 +615,24 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
* The index is on (valueid, chunkidx) so they will come in order * The index is on (valueid, chunkidx) so they will come in order
*/ */
init_toast_snapshot(&SnapshotToast); init_toast_snapshot(&SnapshotToast);
nextidx = startchunk; expectedchunk = startchunk;
toastscan = systable_beginscan_ordered(toastrel, toastidxs[validIndex], toastscan = systable_beginscan_ordered(toastrel, toastidxs[validIndex],
&SnapshotToast, nscankeys, toastkey); &SnapshotToast, nscankeys, toastkey);
while ((ttup = systable_getnext_ordered(toastscan, ForwardScanDirection)) != NULL) while ((ttup = systable_getnext_ordered(toastscan, ForwardScanDirection)) != NULL)
{ {
int32 curchunk;
Pointer chunk;
bool isnull;
char *chunkdata;
int32 chunksize;
int32 expected_size;
int32 chcpystrt;
int32 chcpyend;
/* /*
* Have a chunk, extract the sequence number and the data * Have a chunk, extract the sequence number and the data
*/ */
residx = DatumGetInt32(fastgetattr(ttup, 2, toasttupDesc, &isnull)); curchunk = DatumGetInt32(fastgetattr(ttup, 2, toasttupDesc, &isnull));
Assert(!isnull); Assert(!isnull);
chunk = DatumGetPointer(fastgetattr(ttup, 3, toasttupDesc, &isnull)); chunk = DatumGetPointer(fastgetattr(ttup, 3, toasttupDesc, &isnull));
Assert(!isnull); Assert(!isnull);
...@@ -673,63 +660,60 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) ...@@ -673,63 +660,60 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
/* /*
* Some checks on the data we've found * Some checks on the data we've found
*/ */
if ((residx != nextidx) || (residx > endchunk) || (residx < startchunk)) if (curchunk != expectedchunk)
elog(ERROR, "unexpected chunk number %d (expected %d) for toast value %u in %s", ereport(ERROR,
residx, nextidx, (errcode(ERRCODE_DATA_CORRUPTED),
toast_pointer.va_valueid, errmsg_internal("unexpected chunk number %d (expected %d) for toast value %u in %s",
RelationGetRelationName(toastrel)); curchunk, expectedchunk,
if (residx < totalchunks - 1) toast_pointer.va_valueid,
{ RelationGetRelationName(toastrel))));
if (chunksize != TOAST_MAX_CHUNK_SIZE) if (curchunk > endchunk)
elog(ERROR, "unexpected chunk size %d (expected %d) in chunk %d of %d for toast value %u in %s when fetching slice", ereport(ERROR,
chunksize, (int) TOAST_MAX_CHUNK_SIZE, (errcode(ERRCODE_DATA_CORRUPTED),
residx, totalchunks, errmsg_internal("unexpected chunk number %d (out of range %d..%d) for toast value %u in %s",
toast_pointer.va_valueid, curchunk,
RelationGetRelationName(toastrel)); startchunk, endchunk,
} toast_pointer.va_valueid,
else if (residx == totalchunks - 1) RelationGetRelationName(toastrel))));
{ expected_size = curchunk < totalchunks - 1 ? TOAST_MAX_CHUNK_SIZE
if ((residx * TOAST_MAX_CHUNK_SIZE + chunksize) != attrsize) : attrsize % TOAST_MAX_CHUNK_SIZE;
elog(ERROR, "unexpected chunk size %d (expected %d) in final chunk %d for toast value %u in %s when fetching slice", if (chunksize != expected_size)
chunksize, ereport(ERROR,
(int) (attrsize - residx * TOAST_MAX_CHUNK_SIZE), (errcode(ERRCODE_DATA_CORRUPTED),
residx, errmsg_internal("unexpected chunk size %d (expected %d) in chunk %d of %d for toast value %u in %s",
toast_pointer.va_valueid, chunksize, expected_size,
RelationGetRelationName(toastrel)); curchunk, totalchunks,
} toast_pointer.va_valueid,
else RelationGetRelationName(toastrel))));
elog(ERROR, "unexpected chunk number %d (out of range %d..%d) for toast value %u in %s",
residx,
0, totalchunks - 1,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel));
/* /*
* Copy the data into proper place in our result * Copy the data into proper place in our result
*/ */
chcpystrt = 0; chcpystrt = 0;
chcpyend = chunksize - 1; chcpyend = chunksize - 1;
if (residx == startchunk) if (curchunk == startchunk)
chcpystrt = startoffset; chcpystrt = sliceoffset % TOAST_MAX_CHUNK_SIZE;
if (residx == endchunk) if (curchunk == endchunk)
chcpyend = endoffset; chcpyend = (sliceoffset + slicelength - 1) % TOAST_MAX_CHUNK_SIZE;
memcpy(VARDATA(result) + memcpy(VARDATA(result) +
(residx * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt, (curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt,
chunkdata + chcpystrt, chunkdata + chcpystrt,
(chcpyend - chcpystrt) + 1); (chcpyend - chcpystrt) + 1);
nextidx++; expectedchunk++;
} }
/* /*
* Final checks that we successfully fetched the datum * Final checks that we successfully fetched the datum
*/ */
if (nextidx != (endchunk + 1)) if (expectedchunk != (endchunk + 1))
elog(ERROR, "missing chunk number %d for toast value %u in %s", ereport(ERROR,
nextidx, (errcode(ERRCODE_DATA_CORRUPTED),
toast_pointer.va_valueid, errmsg_internal("missing chunk number %d for toast value %u in %s",
RelationGetRelationName(toastrel)); expectedchunk,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel))));
/* /*
* End scan and close relations * End scan and close relations
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment