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 |
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 |