Subject: Two small issues related to table_relation_copy_for_cluster() and CTAS with no data.

From: Paul Guo <[email-masked]>
Date: Thu, 21 Apr 2022 14:29:59 +0800

Hello hackers,

While reading the latest master branch code, I found something that we
may be able to improve.

1. The am table_relation_copy_for_cluster() interface.

  static inline void
  table_relation_copy_for_cluster(Relation OldTable, Relation NewTable,
                                  Relation OldIndex,
                                  bool use_sort,
                                  TransactionId OldestXmin,
                                  TransactionId *xid_cutoff,
                                  MultiXactId *multi_cutoff,
                                  double *num_tuples,
                                  double *tups_vacuumed,
                                  double *tups_recently_dead)

- Should add a line for parameter num_tuples below "Output parameters
" in comment
- Look at the caller code, i.e. copy_table_data(). It does initialize
*num_tuples,
   *tups_vacuumed and *tups_recently_dead at first. This does not seem
to be a good
   API design or implementation. We'd better let the am api return the
values without
   initializing from callers, right?

2. For CTAS (create table as) with no data. It seems that we won't run
into intorel_receive().
    intorel_startup() could be run into for "create table as t1
execute with no data". So it looks
    like we do not need to judge for into->skipData in
intorel_receive(). If we really want to
    check into->skipData we could add an assert check there or if I
missed some code paths
    in which we could be run into the code branch, we could instead
call below code in
    intorel_receive() to stop early, right?

    if (myState->into->skipData)
        return false;

Regards,
Paul