On Sun, Sep 22, 2019 at 12:22 PM Fabien COELHO <coe...@cri.ensmp.fr> wrote: > >> sh> pgbench -T 10 > >> ... > >> 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? > > Why should we decide that they cannot do that? > > The user could be testing the overhead of no-op updates, which is > something interesting, and check what happens with partitioning in this > case. For that, they may delete pgbench_accounts contents or its > partitions for partitioned version, or only some partitions, or whatever. > > A valid (future) case is that hopefully dynamic partitioning could be > implemented, thus no partitions would be a perfectly legal state even with > the standard benchmarking practice. Maybe the user just wrote a clever > extension to do that with a trigger and wants to test the performance > overhead with pgbench. Fine! >
It is better for a user to write a custom script for such cases. Because after that "select-only" or "simple-update" script doesn't make any sense. In the "select-only" case why would anyone like test fetching zero rows, similarly in "simple-update" case, 2 out of 3 statements will be a no-op. In "tpcb-like" script, 2 out of 5 queries will be no-op and it won't be completely no-op updates as you are telling. Having said that, I see your point and don't mind allowing such cases until we don't have to write special checks in the code to support such cases. Now, we can have a detailed comment in printResults to explain why we have a different check there as compare to other code paths or change other code paths to have a similar check as printResults, but I am not convinced of any of those options. > >> > >> 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. > > Sure. > > > At the very least we should have a comment saying why we are always > > appending the fillfactor for partitions > > The patch does not do that, the condition is just before the call, not > inside it with a boolean passed as an argument. AFAICS the behavior of v14 > is exactly the same as your version and as the initial code. > Here, I am talking about the call to append_fillfactor in createPartitions() function. See, in my version, there are some comments. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com