Hello Amit,
>> One more comment:
>> +typedef enum { PART_NONE, PART_RANGE, PART_HASH, PART_UNKNOWN }
>> + partition_method_t;
>>
>> See, if we can eliminate PART_UNKNOWN.
I'm not very happy with this one, but I wanted to differentiate "we do
know that it is not partitioned" from "we do not know if it is
partitioned", and I did not have a better idea.
> I don't see much use of same.
Although it is not used afterwards, we could display the partitioning
information differently between the two cases. This is not done because I
did not want to add more lines on the "normal" case.
>> It is used at one place where we can set PART_NONE without much loss.
>> Having lesser invalid values makes code easier to follow.
>
> Looking more closely at this case:
> + else if (PQntuples(res) != 1)
> + {
> + /* unsure because multiple (or no) pgbench_accounts found */
> + partition_method = PART_UNKNOWN;
> + partitions = 0;
> + }
>
> Is it ever possible to have multiple pgbench_accounts considering we
> have unique index on (relname, relnamespace) for pg_class?
The issue is that it is not directly obvious which relnamespace will be
used by the queries which rely on non schema qualified "pgbench_accounts".
Each schema could theoretically hold a pgbench_accounts table. As this is
pretty unlikely, I did not attempt to add complexity to resolve taking
into account the search_path, but just skipped to unknown in this case,
which I expect nobody would hit in normal circumstances.
Another possible and unlikely issue is that pgbench_accounts could have
been deleted but not pgbench_branches which is used earlier to get the
current "scale". If so, the queries will fail later on anyway.
--
Fabien.
