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-19 19:11:13
Message-ID: alpine.DEB.2.21.1909190819560.24572@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Amit,

> [...] 'ps' itself won't be NULL in that case, the value it contains is
> NULL. I have debugged this case as well. 'ps' itself can be NULL only
> when you pass wrong column number or something like that to PQgetvalue.

Argh, you are right! I mixed up C NULL and SQL NULL:-(

>>> 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.
>
> I think this is not a possibility today and I don't know of the
> future. I don't think it is a good idea to add code which we can't
> reach today. You can probably add Assert if required.

I added a fail on an unexpected partition method, i.e. not 'r' or 'h',
and an Assert of PQgetvalue returns NULL.

I fixed the query so that it counts actual partitions, otherwise I was
getting one for a partitioned table without partitions attached, which
does not generate an error by the way. I just figured out that pgbench
does not check that UPDATE updates anything. Hmmm.

> Hmm, why you need to change the fill factor behavior? If it is not
> specifically required for the functionality of this patch, then I
> suggest keeping that behavior as it is.

The behavior is not actually changed, but I had to move fillfactor away
because it cannot be declared on partitioned tables, it must be declared
on partitions only. Once there is a function to handle that it is pretty
easy to add the test.

I can remove it but franckly there are only benefits: the default is now
tested by pgbench, the create query is smaller, and it would work with
older versions of pg, which does not matter but is good on principle.

>> 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".
>
> but how will that new partition method will be associated with a table
> created via pgbench?

The user could do a -i with a version of pgbench and bench with another
one. I do that often while developing…

> I think the previous check was good because it makes partition checking
> consistent throughout the patch.

This case now generates a fail.

v12:
- fixes NULL vs NULL
- works correctly with a partitioned table without partitions attached
- generates an error if the partition method is unknown
- adds an assert

--
Fabien.

Attachment Content-Type Size
pgbench-init-partitioned-2.patch text/x-diff 9.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2019-09-19 19:14:38 Re: Bug in GiST paring heap comparator
Previous Message Jonathan S. Katz 2019-09-19 18:37:29 Re: Define jsonpath functions as stable