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.
