| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> | 
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> | 
| Cc: | Asif Rehman <asifr(dot)rehman(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> | 
| Subject: | Re: pgbench - allow to create partitioned tables | 
| Date: | 2019-09-21 07:48:31 | 
| Message-ID: | alpine.DEB.2.21.1909210813010.21682@lancre | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
Hello Amit,
> 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
   query mode: simple
   number of clients: 1
   number of threads: 1
   duration: 10 s
   number of transactions actually processed: 2314
   latency average = 4.323 ms
   tps = 231.297122 (including connections establishing)
   tps = 231.549125 (excluding connections establishing)
As postgres does not break, there is no good reason to forbid it.
> [...] Sure, even in that case your older version of pgbench will be able 
> to detect by below code [...] "unexpected partition method: " [...].
Yes, that is what I was saying.
> Hmm, you have just written what each part of the query is doing which I 
> think one can identify if we write some general comment as I have in the 
> patch to explain the overall intent. Even if we write what each part of 
> the statement is doing, the comment explaining overall intent is 
> required.
There was some comments.
> I personally don't like writing a comment for each sub-part of the query 
> as that makes reading the query difficult.  See the patch sent by me in 
> my previous email.
I did not notice there was an attachment.
> I have done that in some of the cases in the patch attached by me in
> my last email.  Have you looked at those changes?
Nope, as I was not expected one.
> Try to make those changes in the next version unless you see something 
> wrong is written in comments.
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.
See attached v14.
-- 
Fabien.
| Attachment | Content-Type | Size | 
|---|---|---|
| pgbench-init-partitioned-14.patch | text/x-diff | 13.8 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Fetter | 2019-09-21 09:15:27 | Re: WIP: Generic functions for Node types using generated metadata | 
| Previous Message | Andrew Gierth | 2019-09-21 06:29:25 | Re: Efficient output for integer types |