Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbc4de01

Browse files
committed
Minor cleanup/code review for "indirect toast" stuff.
Fix some issues I noticed while fooling with an extension to allow anadditional kind of toast pointer. Much of this is just commentimprovement, but there are a couple of actual bugs, which might or mightnot be reachable today depending on what can happen during logicaldecoding. An example is that toast_flatten_tuple() failed to cover thepossibility of an indirection pointer in its input. Back-patch to 9.4just in case that is reachable now.In HEAD, also correct some really minor issues with recent compressionreorganization, such as dangerously underparenthesized macros.
1 parentc619c23 commitbc4de01

File tree

3 files changed

+66
-42
lines changed

3 files changed

+66
-42
lines changed

‎src/backend/access/heap/tuptoaster.c

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,20 @@
5050
*/
5151
typedefstructtoast_compress_header
5252
{
53-
int32vl_len_;/* varlena header (do not touch directly!) */
53+
int32vl_len_;/* varlena header (do not touch directly!) */
5454
int32rawsize;
5555
}toast_compress_header;
5656

5757
/*
5858
* Utilities for manipulation of header information for compressed
5959
* toast entries.
6060
*/
61-
#defineTOAST_COMPRESS_HDRSZ((int32) sizeof(toast_compress_header))
62-
#defineTOAST_COMPRESS_RAWSIZE(ptr)(((toast_compress_header *) ptr)->rawsize)
61+
#defineTOAST_COMPRESS_HDRSZ((int32) sizeof(toast_compress_header))
62+
#defineTOAST_COMPRESS_RAWSIZE(ptr)(((toast_compress_header *)(ptr))->rawsize)
6363
#defineTOAST_COMPRESS_RAWDATA(ptr) \
64-
(((char *) ptr) + TOAST_COMPRESS_HDRSZ)
64+
(((char *)(ptr)) + TOAST_COMPRESS_HDRSZ)
6565
#defineTOAST_COMPRESS_SET_RAWSIZE(ptr,len) \
66-
(((toast_compress_header *) ptr)->rawsize = len)
66+
(((toast_compress_header *)(ptr))->rawsize =(len))
6767

6868
staticvoidtoast_delete_datum(Relationrel,Datumvalue);
6969
staticDatumtoast_save_datum(Relationrel,Datumvalue,
@@ -90,8 +90,9 @@ static void toast_close_indexes(Relation *toastidxs, int num_indexes,
9090
*
9191
* This will return a datum that contains all the data internally, ie, not
9292
* relying on external storage or memory, but it can still be compressed or
93-
* have a short header.
94-
----------
93+
* have a short header. Note some callers assume that if the input is an
94+
* EXTERNAL datum, the result will be a pfree'able chunk.
95+
* ----------
9596
*/
9697
structvarlena*
9798
heap_tuple_fetch_attr(structvarlena*attr)
@@ -108,9 +109,7 @@ heap_tuple_fetch_attr(struct varlena * attr)
108109
elseif (VARATT_IS_EXTERNAL_INDIRECT(attr))
109110
{
110111
/*
111-
* copy into the caller's memory context. That's not required in all
112-
* cases but sufficient for now since this is mainly used when we need
113-
* to persist a Datum for unusually long time, like in a HOLD cursor.
112+
* This is an indirect pointer --- dereference it
114113
*/
115114
structvaratt_indirectredirect;
116115

@@ -120,11 +119,14 @@ heap_tuple_fetch_attr(struct varlena * attr)
120119
/* nested indirect Datums aren't allowed */
121120
Assert(!VARATT_IS_EXTERNAL_INDIRECT(attr));
122121

123-
/*doesn't make much sense, but better handle it */
124-
if (VARATT_IS_EXTERNAL_ONDISK(attr))
122+
/*recurse if value is still external in some other way */
123+
if (VARATT_IS_EXTERNAL(attr))
125124
returnheap_tuple_fetch_attr(attr);
126125

127-
/* copy datum verbatim */
126+
/*
127+
* Copy into the caller's memory context, in case caller tries to
128+
* pfree the result.
129+
*/
128130
result= (structvarlena*)palloc(VARSIZE_ANY(attr));
129131
memcpy(result,attr,VARSIZE_ANY(attr));
130132
}
@@ -144,7 +146,10 @@ heap_tuple_fetch_attr(struct varlena * attr)
144146
* heap_tuple_untoast_attr -
145147
*
146148
*Public entry point to get back a toasted value from compression
147-
*or external storage.
149+
*or external storage. The result is always non-extended varlena form.
150+
*
151+
* Note some callers assume that if the input is an EXTERNAL or COMPRESSED
152+
* datum, the result will be a pfree'able chunk.
148153
* ----------
149154
*/
150155
structvarlena*
@@ -160,12 +165,16 @@ heap_tuple_untoast_attr(struct varlena * attr)
160165
if (VARATT_IS_COMPRESSED(attr))
161166
{
162167
structvarlena*tmp=attr;
168+
163169
attr=toast_decompress_datum(tmp);
164170
pfree(tmp);
165171
}
166172
}
167173
elseif (VARATT_IS_EXTERNAL_INDIRECT(attr))
168174
{
175+
/*
176+
* This is an indirect pointer --- dereference it
177+
*/
169178
structvaratt_indirectredirect;
170179

171180
VARATT_EXTERNAL_GET_POINTER(redirect,attr);
@@ -174,7 +183,18 @@ heap_tuple_untoast_attr(struct varlena * attr)
174183
/* nested indirect Datums aren't allowed */
175184
Assert(!VARATT_IS_EXTERNAL_INDIRECT(attr));
176185

186+
/* recurse in case value is still extended in some other way */
177187
attr=heap_tuple_untoast_attr(attr);
188+
189+
/* if it isn't, we'd better copy it */
190+
if (attr== (structvarlena*)redirect.pointer)
191+
{
192+
structvarlena*result;
193+
194+
result= (structvarlena*)palloc(VARSIZE_ANY(attr));
195+
memcpy(result,attr,VARSIZE_ANY(attr));
196+
attr=result;
197+
}
178198
}
179199
elseif (VARATT_IS_COMPRESSED(attr))
180200
{
@@ -246,9 +266,12 @@ heap_tuple_untoast_attr_slice(struct varlena * attr,
246266
else
247267
preslice=attr;
248268

269+
Assert(!VARATT_IS_EXTERNAL(preslice));
270+
249271
if (VARATT_IS_COMPRESSED(preslice))
250272
{
251273
structvarlena*tmp=preslice;
274+
252275
preslice=toast_decompress_datum(tmp);
253276

254277
if (tmp!=attr)
@@ -448,8 +471,6 @@ toast_delete(Relation rel, HeapTuple oldtup)
448471
continue;
449472
elseif (VARATT_IS_EXTERNAL_ONDISK(PointerGetDatum(value)))
450473
toast_delete_datum(rel,value);
451-
elseif (VARATT_IS_EXTERNAL_INDIRECT(PointerGetDatum(value)))
452-
elog(ERROR,"attempt to delete tuple containing indirect datums");
453474
}
454475
}
455476
}
@@ -611,7 +632,8 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
611632

612633
/*
613634
* We took care of UPDATE above, so any external value we find
614-
* still in the tuple must be someone else's we cannot reuse.
635+
* still in the tuple must be someone else's that we cannot reuse
636+
* (this includes the case of an out-of-line in-memory datum).
615637
* Fetch it back (without decompression, unless we are forcing
616638
* PLAIN storage). If necessary, we'll push it out as a new
617639
* external value below.
@@ -1043,7 +1065,7 @@ toast_flatten_tuple(HeapTuple tup, TupleDesc tupleDesc)
10431065
new_value= (structvarlena*)DatumGetPointer(toast_values[i]);
10441066
if (VARATT_IS_EXTERNAL(new_value))
10451067
{
1046-
new_value=toast_fetch_datum(new_value);
1068+
new_value=heap_tuple_fetch_attr(new_value);
10471069
toast_values[i]=PointerGetDatum(new_value);
10481070
toast_free[i]= true;
10491071
}
@@ -1746,8 +1768,8 @@ toast_fetch_datum(struct varlena * attr)
17461768
intnum_indexes;
17471769
intvalidIndex;
17481770

1749-
if (VARATT_IS_EXTERNAL_INDIRECT(attr))
1750-
elog(ERROR,"shouldn't be called forindirect tuples");
1771+
if (!VARATT_IS_EXTERNAL_ONDISK(attr))
1772+
elog(ERROR,"toast_fetch_datumshouldn't be called fornon-ondisk datums");
17511773

17521774
/* Must copy to access aligned fields */
17531775
VARATT_EXTERNAL_GET_POINTER(toast_pointer,attr);
@@ -1923,7 +1945,8 @@ toast_fetch_datum_slice(struct varlena * attr, int32 sliceoffset, int32 length)
19231945
intnum_indexes;
19241946
intvalidIndex;
19251947

1926-
Assert(VARATT_IS_EXTERNAL_ONDISK(attr));
1948+
if (!VARATT_IS_EXTERNAL_ONDISK(attr))
1949+
elog(ERROR,"toast_fetch_datum_slice shouldn't be called for non-ondisk datums");
19271950

19281951
/* Must copy to access aligned fields */
19291952
VARATT_EXTERNAL_GET_POINTER(toast_pointer,attr);

‎src/include/access/tuptoaster.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@
9696
VARHDRSZ)
9797

9898
/* Size of an EXTERNAL datum that contains a standard TOAST pointer */
99-
#defineTOAST_POINTER_SIZE (VARHDRSZ_EXTERNAL + sizeof(structvaratt_external))
99+
#defineTOAST_POINTER_SIZE (VARHDRSZ_EXTERNAL + sizeof(varatt_external))
100100

101-
/* Size of anindirect datum that containsa standard TOAST pointer */
102-
#defineINDIRECT_POINTER_SIZE (VARHDRSZ_EXTERNAL + sizeof(structvaratt_indirect))
101+
/* Size of anEXTERNAL datum that containsan indirection pointer */
102+
#defineINDIRECT_POINTER_SIZE (VARHDRSZ_EXTERNAL + sizeof(varatt_indirect))
103103

104104
/*
105105
* Testing whether an externally-stored value is compressed now requires

‎src/include/postgres.h

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@
5454
*/
5555

5656
/*
57-
* struct varatt_external is a "TOAST pointer", that is, the information needed
58-
* to fetch a Datum storedin anout-of-lineon-disk Datum. The data is
59-
* compressed if and only if va_extsize < va_rawsize - VARHDRSZ. This struct
60-
* must not contain any padding, because we sometimes compare pointers using
61-
* memcmp.
57+
* struct varatt_external is atraditional"TOAST pointer", that is, the
58+
*information neededto fetch a Datum stored out-of-linein a TOAST table.
59+
*The data iscompressed if and only if va_extsize < va_rawsize - VARHDRSZ.
60+
*This structmust not contain any padding, because we sometimes compare
61+
*these pointers usingmemcmp.
6262
*
6363
* Note that this information is stored unaligned within actual tuples, so
6464
* you need to memcpy from the tuple into a local struct variable before
@@ -74,22 +74,23 @@ typedef struct varatt_external
7474
}varatt_external;
7575

7676
/*
77-
* Out-of-line Datum thats stored in memory in contrast to varatt_external
78-
* pointers which points to data in an external toast relation.
77+
* struct varatt_indirect is a "TOAST pointer" representing an out-of-line
78+
* Datum that's stored in memory, not in an external toast relation.
79+
* The creator of such a Datum is entirely responsible that the referenced
80+
* storage survives for as long as referencing pointer Datums can exist.
7981
*
80-
* Note that just as varatt_external's this is stored unaligned within the
81-
* tuple.
82+
* Note that just asfor structvaratt_external, thisstructis stored
83+
*unaligned within any containingtuple.
8284
*/
8385
typedefstructvaratt_indirect
8486
{
8587
structvarlena*pointer;/* Pointer to in-memory varlena */
8688
}varatt_indirect;
8789

88-
8990
/*
90-
* Typeof external toast datum stored. The peculiar value for VARTAG_ONDISK
91-
* comes fromthe requirement for on-disk compatibility with the older
92-
*definitions of varattrib_1b_e where v_tagwasnamed va_len_1be...
91+
* Typetag for the various sorts of "TOAST pointer" datums. The peculiar
92+
*value for VARTAG_ONDISKcomes froma requirement for on-disk compatibility
93+
*with a previous notion that the tag fieldwasthe pointer datum's length.
9394
*/
9495
typedefenumvartag_external
9596
{
@@ -98,9 +99,9 @@ typedef enum vartag_external
9899
}vartag_external;
99100

100101
#defineVARTAG_SIZE(tag) \
101-
((tag) == VARTAG_INDIRECT ? sizeof(varatt_indirect) :\
102+
((tag) == VARTAG_INDIRECT ? sizeof(varatt_indirect) :\
102103
(tag) == VARTAG_ONDISK ? sizeof(varatt_external) : \
103-
TrapMacro(true, "unknown vartag"))
104+
TrapMacro(true, "unrecognized TOAST vartag"))
104105

105106
/*
106107
* These structs describe the header of a varlena object that may have been
@@ -132,7 +133,7 @@ typedef struct
132133
charva_data[1];/* Data begins here */
133134
}varattrib_1b;
134135

135-
/*inline portion of ashort varlena pointing to anexternal resource */
136+
/*TOAST pointers are asubset of varattrib_1b with anidentifying tag byte */
136137
typedefstruct
137138
{
138139
uint8va_header;/* Always 0x80 or 0x01 */
@@ -162,8 +163,8 @@ typedef struct
162163
* this lets us disambiguate alignment padding bytes from the start of an
163164
* unaligned datum. (We now *require* pad bytes to be filled with zero!)
164165
*
165-
* In TOASTdatums thetag fieldin varattrib_1b_e is used to discern whether
166-
*its an indirection pointer or more commonly an on-disk tuple.
166+
* In TOASTpointers theva_tag field(see varattrib_1b_e) is used to discern
167+
*the specific type and length of the pointer datum.
167168
*/
168169

169170
/*

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp