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

Hello Amit,

>>   - use search_path to find at most one pgbench_accounts
>>     It still uses left join because I still think that it is appropriate.
>>     I added a lateral to avoid repeating the array_position call
>>     to manage the search_path, and use explicit pg_catalog everywhere.
>
> It would be good if you can add some more comments to explain the
> intent of query.

Indeed, I put too few comments on the query.

> + if (ps == NULL)
> + partition_method = PART_NONE;
>
> When can we expect ps as NULL?  If this is not a valid case, then
> probably and Assert would be better.

No, ps is really NULL if there is no partitioning, because of the LEFT 
JOIN and pg_partitioned_table is just empty in that case.

The last else where there is an unexpected entry is different, see 
comments about v11 below.

> + else if (PQntuples(res) == 0)
> + {
> + /* no pgbench_accounts found, builtin script should fail later */
> + partition_method = PART_NONE;
> + partitions = -1;
>
> If we don't find pgbench_accounts, let's give error here itself rather
> than later unless you have a valid case in mind.

I thought of it, but decided not to: Someone could add a builtin script 
which does not use pgbench_accounts, or a parallel running script could 
create a table dynamically, whatever, so I prefer the error to be raised 
by the script itself, rather than deciding that it will fail before even 
trying.

> + /*
> + * Partition information. Assume no partitioning on any failure, so as
> + * to avoid failing on an older version.
> + */
> ..
> + if (PQresultStatus(res) != PGRES_TUPLES_OK)
> + {
> + /* probably an older version, coldly assume no partitioning */
> + partition_method = PART_NONE;
> + partitions = 0;
> + }
>
> So, here we are silently absorbing the error when pgbench is executed
> against older server version which doesn't support partitioning.

Yes, exactly.

> If that is the case, then I think if user gives --partitions for the old 
> server version, it will also give an error?

Yes, on -i it will fail because the syntax will not be recognized.

> It is not clear in documentation whether we support or not using pgbench 
> with older server versions.

Indeed. We more or less do in practice. Command "psql" works back to 8 
AFAICR, and pgbench as well.

> I guess it didn't matter, but with this feature, it can matter.  Do we 
> need to document this?

This has been discussed in the past, and the conclusion was that it was 
not worth the effort. We just try not to break things if it is avoidable. 
On this regard, the patch slightly changes FILLFACTOR output, which is 
removed if the value is 100 (%) as it is the default, which means that 
table creation would work on very very old version which did not support 
fillfactor, unless you specify a lower percentage.

Attached v11:

  - add quite a few comments on the pg_catalog query

  - reverts the partitions >= 1 test; If some new partition method is
    added that pgbench does not know about, the failure mode will be that
    nothing is printed rather than printing something strange like
    "method none with 2 partitions".

-- 
Fabien.