Re: pgbench - allow to create partitioned tables - Mailing list pgsql-hackers

Hello Amit,

>>>> 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.
>>>
>>> Sure, but I feel the code will be simplified.  I see no reason for
>>> using left join here.
>>
>> Without a left join, the query result is empty if there are no partitions,
>> whereas there is one line with it. This fact simplifies managing the query
>> result afterwards because we are always expecting 1 row in the "normal"
>> case, whether partitioned or not.
>
> Why can't we change it as attached?

I think that your version works, but I do not like much the condition for 
the normal case which is implicitely assumed. The solution I took has 3 
clear-cut cases: 1 error against a server without partition support, 
detect multiple pgbench_accounts table -- argh, and then the normal 
expected case, whether partitioned or not. Your solution has 4 cases 
because of the last implicit zero-row select that relies on default, which 
would need some explanations.

> I find using left join to always get one row as an ugly way to 
> manipulate the results later.

Hmmm. It is really a matter of taste. I do not share your distate for left 
join on principle. In the case at hand, I find that getting one row in all 
cases pretty elegant because there is just one code for handling them all.

> We shouldn't go in that direction unless we can't handle this with some 
> simple code.

Hmmm. Left join does not strike me as over complex code. I wish my student 
would remember that this thing exists:-)

> What is the need of using regress_pgbench_tap_1_ts in this test?

I wanted to check that tablespace options work appropriately with 
partition tables, as I changed the create table stuff significantly, and 
just using "pg_default" is kind of cheating.

> I think we don't need to change existing tests unless required for the 
> new functionality.

I do agree, but there was a motivation behind the addition.

> *
> - 'pgbench scale 1 initialization');
> + 'pgbench scale 1 initialization with options');
>
> Similar to the above, it is not clear to me why we need to change this?

Because I noticed that it had the same description as the previous one, so 
I made the test name distinct and more precise, while I was adding options 
on it.

> *pgbench(
> -
>  # given the expected rate and the 2 ms tx duration, at most one is executed
>  '-t 10 --rate=100000 --latency-limit=1 -n -r',
>  0,
>
> The above appears to be a spurious line change.

Indeed. I think that this empty line is a typo, but I can let it as it is.

> * I think we need to change the docs [1] to indicate the new step for
> partitioning.  See section --init-steps=init_steps
>
> [1] - https://www.postgresql.org/docs/devel/pgbench.html

The partitioned table generation is integrated into the existing create 
table step, it is not a separate step because I cannot see an interest to 
do something in between the table creations.

Patch v8 attached adds some comments around partition detection, ensures 
that 0 is returned for the no partition case and let the spurious empty 
line where it is.

-- 
Fabien.