On Sat, Sep 21, 2019 at 1:18 PM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > Yes, this code is correct. I am not sure if you understood the point,
> > so let me try again. I am bothered about below code in the patch:
> > + /* only print partitioning information if some partitioning was detected */
> > + if (partition_method != PART_NONE)
> >
> > This is the only place now where we check 'whether there are any
> > partitions' differently. I am suggesting to make this similar to
> > other checks (if (partitions > 0)).
>
> As I said somewhere up thread, you can have a partitioned table with zero
> partitions, and it works fine (yep! the update just does not do anything…)
> so partitions > 0 is not a good way to know whether there is a partitioned
> table when running a bench. It is a good way for initialization, though,
> because we are creating them.
>
> sh> pgbench -i --partitions=1
> sh> psql -c 'DROP TABLE pgbench_accounts_1'
> sh> pgbench -T 10
> ...
> transaction type: <builtin: TPC-B (sort of)>
> scaling factor: 1
> partition method: hash
> partitions: 0
>
I am not sure how many users would be able to make out that it is a
run where actual partitions are not present unless they beforehand
know and detect such a condition in their scripts. What is the use of
such a run which completes without actual updates? I think it is
better if give an error for such a case rather than allowing to
execute it and then give some information which doesn't make much
sense.
>
> I incorporated most of them, although I made them terser, and fixed them
> when inaccurate.
>
> I did not buy moving the condition inside the fillfactor function.
>
I also don't agree with your position. My main concern here is that
we can't implicitly assume that fillfactor need to be appended. At
the very least we should have a comment saying why we are always
appending the fillfactor for partitions, something like I had in my
patch.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com