Hello Amit,
>>> Is there a reason why we treat "partitions = 0" as a valid value?
>>
>> Yes. It is an explicit "do not create partitioned tables", which differ
>> from 1 which says "create a partitionned table with just one partition".
>
> Why would anyone want to use --partitions option in the first case
> ("do not create partitioned tables")?
Having an explicit value for the default is generally a good idea, eg for
a script to tests various partitioning settings:
for nparts in 0 1 2 3 4 5 6 7 8 9 ; do
pgbench -i --partitions=$nparts ... ;
...
done
Otherwise you would need significant kludging to add/remove the option.
Allowing 0 does not harm anyone.
Now if the consensus is to remove an explicit 0, it is simple enough to
change it, but my opinion is that it is better to have it.
>>> I think we should print the information about partitions in
>>> printResults. It can help users while analyzing results.
>
> + res = PQexec(con,
> + "select p.partstrat, count(*) "
> + "from pg_catalog.pg_class as c "
> + "left join pg_catalog.pg_partitioned_table as p on (p.partrelid = c.oid) "
> + "left join pg_catalog.pg_inherits as i on (c.oid = i.inhparent) "
> + "where c.relname = 'pgbench_accounts' "
> + "group by 1, c.oid");
>
> Can't we write this query with inner join instead of left join? What
> additional purpose you are trying to serve by using left join?
I'm ensuring that there is always a one line answer, whether it is
partitioned or not. Maybe the count(*) should be count(something in p) to
get 0 instead of 1 on non partitioned tables, though, but this is hidden
in the display anyway.
> +partition_method_t partition_method = PART_NONE;
>
> It is better to make this static.
I do agree, but this would depart from all other global variables around
which are currently not static. Maybe a separate patch could turn them all
as static, but ISTM that this patch should not change the current style.
--
Fabien.
