On Fri, Sep 13, 2019 at 11:06 PM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>
> Hello Amit,
>
> >>> + res = PQexec(con,
> >>> + "select p.partstrat, count(*) "
> >>> + "from pg_catalog.pg_class as c "
> >>> + "left join pg_catalog.pg_partitioned_table as p on (p.partrelid = c.oid) "
> >>> + "left join pg_catalog.pg_inherits as i on (c.oid = i.inhparent) "
> >>> + "where c.relname = 'pgbench_accounts' "
> >>> + "group by 1, c.oid");
> >>>
> >>> Can't we write this query with inner join instead of left join? What
> >>> additional purpose you are trying to serve by using left join?
> >>
> >> 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 find using left join to always
get one row as an ugly way to manipulate the results later. We
shouldn't go in that direction unless we can't handle this with some
simple code.
Some more comments:
*
- '--initialize --init-steps=dtpvg --scale=1 --unlogged-tables
--fillfactor=98 --foreign-keys --quiet --tablespace=pg_default
--index-tablespace=pg_default',
+ '--initialize --init-steps=dtpvg --scale=1 --unlogged-tables
--fillfactor=98 --foreign-keys --quiet
--tablespace=regress_pgbench_tap_1_ts
--index-tablespace=regress_pgbench_tap_1_ts --partitions=2
--partition-method=hash',
What is the need of using regress_pgbench_tap_1_ts in this test? I
think we don't need to change existing tests unless required for the
new functionality.
*
- '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?
*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.
* 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
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com