Re: Batching in executor

Hi,

Some comments for v4:

0001
====

1) table_scan_getnextbatch()
"Assert(dir == ForwardScanDirection);" -> "Assert(ScanDirectionIsForward(dir));"

2) heapgettup_pagemode_batch()
"TupleDesc    tupdesc = key ? RelationGetDescr(rel) : NULL;" -> "TupleDesc    tupdesc = RelationGetDescr(rel);"
I think the latter is enough.

3) heapgettup_pagemode_batch()
```
            /* Are there more visible tuples left on this page? */
            lineindex = scan->rs_cindex + dir;
            linesleft = (lineindex <= (uint32) scan->rs_ntuples) ?
                (scan->rs_ntuples - lineindex) : 0;
            if (linesleft > 0)
                break;    /* continue on this page */
```
The "scan->rs_ntuples" is already an uint32.

4) heapgettup_pagemode_batch()
```
        Assert(lineindex <= (uint32) scan->rs_ntuples);
```
The "scan->rs_ntuples" is already an uint32. And I think this should be "Assert(lineindex < scan->rs_ntuples);", the
related
assert in heapgettup_pagemode() is also wrong.

5) heapgettup_pagemode_batch()
If the scan key filters out all tuples on a page, we may return 0 before reaching the end of scan, right?

6) heap_begin_batch()
```
    hb = palloc(sizeof(HeapBatch));
    hb->tupdata = palloc(sizeof(HeapTupleData) * maxitems);
```
Can we just use one palloc() for cache-friendly?

0002
====

1) heap_materialize_batch_all()
```
        slot->base.tts_flags &= ~(TTS_FLAG_EMPTY | TTS_FLAG_SHOULDFREE);
        slot->base.tts_tid = tuple->t_self;
        slot->base.tts_tableOid = tuple->t_tableOid;
        slot->base.tts_flags &= ~(TTS_FLAG_SHOULDFREE | TTS_FLAG_EMPTY);
```
Redundant of "slot->base.tts_flags &="?

2) TupleBatchCreate()
```
    inslots = palloc(sizeof(TupleTableSlot *) * capacity);
    outslots = palloc(sizeof(TupleTableSlot *) * capacity);
    for (int i = 0; i < capacity; i++)
        inslots[i] = MakeSingleTupleTableSlot(scandesc, &TTSOpsHeapTuple);

    b = (TupleBatch *) palloc(sizeof(TupleBatch));
```
Can we just use one palloc() for cache-friendly?

3) TupleBatchCreate()
```
    b->outslots = outslots;
    b->activeslots = NULL;
    b->outslots = outslots;
```
Redundant of "b->outslots = outslots;"?

4) TupleBatchReset()
```
    if (b == NULL)
        return;
```
This can never happen, convert to a assert or just delete it?

5) SeqNextBatch()
"Assert(direction == ForwardScanDirection);" -> "Assert(ScanDirectionIsForward(direction));"

--
Regards,
ChangAo Chen