|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
> [...] '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
> 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.
- 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
|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|