Re: pgbench - allow to create partitioned tables - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: pgbench - allow to create partitioned tables
Date
Msg-id alpine.DEB.2.21.1909190819560.24572@lancre
Whole thread Raw
In response to Re: pgbench - allow to create partitioned tables  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: pgbench - allow to create partitioned tables
Re: pgbench - allow to create partitioned tables
List 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

pgsql-hackers by date:

Previous
From: "Jonathan S. Katz"
Date:
Subject: Re: Define jsonpath functions as stable
Next
From: Alexander Korotkov
Date:
Subject: Re: Bug in GiST paring heap comparator