Break early when creating Memtuple binding from TupleDesc

[avamingli]

MemtupleBinding is column aligned by 4 or 8 bytes.

It can be set to 8 if we find an attribute's attalign is 'd' (8 bytes on most machines), there is no need to check other attributes. Add some comments about how to compute null bitmap entries space.

Co-authored-by: Mingli Zhang [email protected]

Here are some reminders before you submit the pull request

• [ ] Add tests for the change
• [ ] Document changes
• [ ] Communicate in the mailing list if needed
• [ ] Pass make installcheck
• [ ] Review a PR in return to support the community

[ashwinstar]

Understand the change, please can you post the motivation behind the change? What exactly high-lighted the problem and with the change how we have proven its resolved and such.

[ashwinstar]

Understand the change, please can you post the motivation behind the change? What exactly high-lighted the problem and with the change how we have proven its resolved and such.

@avamingli please can you answer the question to help take the PR forward

[avamingli]

Understand the change, please can you post the motivation behind the change? What exactly high-lighted the problem and with the change how we have proven its resolved and such.

@avamingli please can you answer the question to help take the PR forward

Hmm, it's a little bit long ago. I found this by reading codes.

MemtupleBinding is column aligned by 4 or 8 bytes. We could stop early If found a 'd' typalign column, or we will iterate all columns. This patch helps a little(every little helps) when an AO table has a lot of columns and one of these columns is a 'd' typalign with an earlier column-definition order.

ex:

create table t1(c1 int8, c2 bool, c3 bool) with(appendonly = true);
insert into t1 select i, true, true from generate_series(1, 1000) g(i);

I test it by the below codes with gdb on QE :

+       int count = 0;
        for(i = 0; i < tupdesc->natts; ++i)
        {
+               count++;
                Form_pg_attribute attr = TupleDescAttr(tupdesc, i);

                if (attr->attlen > 0 && attr->attalign == 'd')
+               {
                        pbind->column_align = 8;
+                       //break;
+               }
        }

exec sql select * from t1;

without break, until the end of the for range:

(gdb) p count
$1 = 3

and with this patch:

(gdb) p count
$1 = 1

Suppose a table has N (up to 1600? from upstream, I didn't check it on GPDB) columns and the first column is 'd' typalign, we will have to iterate N times in create_memtuple_binding(). And with this patch, we only need to iterate 1 time.

[kainwen]

I think this is fine to merge, do you have other comments? Thanks a lot @ashwinstar