Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3a48005

Browse files
committed
Improve code inferring length of bitmap for JITed tuple deforming.
While discussing comment improvements (see next commit) by Justin Pryzby, Tom complained about a few details of the logic to infer the length of the NULL bitmap when building the JITed tuple deforming function. That bitmap allows to avoid checking the tuple header's natts, a check which often causes a pipeline stall Improvements: a) As long as missing columns aren't taken into account, we can continue to infer the length of the NULL bitmap from NOT NULL columns following it. Previously we stopped at the first missing column. It's unlikely to matter much in practice, but the alternative would have been to document why we stop. b) For robustness reasons it seems better to also check against attisdropped - RemoveAttributeById() sets attnotnull to false, but an additional check is trivial. c) Improve related comments Discussion: https://postgr.es/m/20637.1555957068@sss.pgh.pa.us Backpatch: -
1 parent e03ff73 commit 3a48005

File tree

1 file changed

+20
-17
lines changed

1 file changed

+20
-17
lines changed

src/backend/jit/llvm/llvmjit_deform.c

+20-17
Original file line numberDiff line numberDiff line change
@@ -103,27 +103,27 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
103103
funcname = llvm_expand_funcname(context, "deform");
104104

105105
/*
106-
* Check which columns do have to exist, so we don't have to check the
107-
* rows natts unnecessarily.
106+
* Check which columns have to exist, so we don't have to check the row's
107+
* natts unnecessarily.
108108
*/
109109
for (attnum = 0; attnum < desc->natts; attnum++)
110110
{
111111
Form_pg_attribute att = TupleDescAttr(desc, attnum);
112112

113113
/*
114-
* If the column is possibly missing, we can't rely on its (or
115-
* subsequent) NOT NULL constraints to indicate minimum attributes in
116-
* the tuple, so stop here.
114+
* If the column is declared NOT NULL then it must be present in every
115+
* tuple, unless there's a "missing" entry that could provide a
116+
* non-NULL value for it. That in turn guarantees that the NULL bitmap
117+
* - if there are any NULLable columns - is at least long enough to
118+
* cover columns up to attnum.
119+
*
120+
* Be paranoid and also check !attisdropped, even though the
121+
* combination of attisdropped && attnotnull combination shouldn't
122+
* exist.
117123
*/
118-
if (att->atthasmissing)
119-
break;
120-
121-
/*
122-
* Column is NOT NULL and there've been no preceding missing columns,
123-
* it's guaranteed that all columns up to here exist at least in the
124-
* NULL bitmap.
125-
*/
126-
if (att->attnotnull)
124+
if (att->attnotnull &&
125+
!att->atthasmissing &&
126+
!att->attisdropped)
127127
guaranteed_column_number = attnum;
128128
}
129129

@@ -298,9 +298,12 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
298298
}
299299

300300
/*
301-
* Check if's guaranteed the all the desired attributes are available in
302-
* tuple. If so, we can start deforming. If not, need to make sure to
303-
* fetch the missing columns.
301+
* Check if it is guaranteed that all the desired attributes are available
302+
* in the tuple (but still possibly NULL), by dint of either the last
303+
* to-be-deformed column being NOT NULL, or subsequent ones not accessed
304+
* here being NOT NULL. If that's not guaranteed the tuple headers natt's
305+
* has to be checked, and missing attributes potentially have to be
306+
* fetched (using slot_getmissingattrs().
304307
*/
305308
if ((natts - 1) <= guaranteed_column_number)
306309
{

0 commit comments

Comments
 (0)