On Sat, Sep 21, 2019 at 1:18 PM Fabien COELHO <coe...@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