Re: pgbench - allow to create partitioned tables

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-18 17:03:02
Message-ID: alpine.DEB.2.21.1909181547400.32172@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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.

Attachment Content-Type Size
pgbench-init-partitioned-11.patch text/x-diff 12.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2019-09-18 17:11:12 Re: dropdb --force
Previous Message Dagfinn Ilmari Mannsåker 2019-09-18 15:46:30 Re: Proposal: Add more compile-time asserts to expose inconsistencies.