Re: pgbench - allow to create partitioned tables

2020-01-03 Thread Fabien COELHO
Hello Peter, The documentation and pgbench --help output that accompanied this patch claims that the argument to pgbench --partition-method is optional and defaults to "range", but that is not actually the case, as the implementation requires an argument. Could you please sort this out?

Re: pgbench - allow to create partitioned tables

2020-01-03 Thread Peter Eisentraut
On 2020-01-03 11:04, Amit Kapila wrote: On Fri, Jan 3, 2020 at 3:24 PM Peter Eisentraut wrote: The documentation and pgbench --help output that accompanied this patch claims that the argument to pgbench --partition-method is optional and defaults to "range", but that is not actually the case,

Re: pgbench - allow to create partitioned tables

2020-01-03 Thread Amit Kapila
On Fri, Jan 3, 2020 at 3:24 PM Peter Eisentraut wrote: > > The documentation and pgbench --help output that accompanied this patch > claims that the argument to pgbench --partition-method is optional and > defaults to "range", but that is not actually the case, as the > implementation requires an

Re: pgbench - allow to create partitioned tables

2020-01-03 Thread Peter Eisentraut
The documentation and pgbench --help output that accompanied this patch claims that the argument to pgbench --partition-method is optional and defaults to "range", but that is not actually the case, as the implementation requires an argument. Could you please sort this out? Personally, I

Re: pgbench - allow to create partitioned tables

2019-10-03 Thread Ashutosh Sharma
On Thu, Oct 3, 2019 at 1:53 PM Fabien COELHO wrote: > > > Hello, > > > As partitions is an integer type variable, the maximum value it can > > hold is "2147483647". But if I specify partitions as "3147483647", > > atoi function returns a value lesser than zero and pgbench terminates > > with an

Re: pgbench - allow to create partitioned tables

2019-10-03 Thread Fabien COELHO
Hello, As partitions is an integer type variable, the maximum value it can hold is "2147483647". But if I specify partitions as "3147483647", atoi function returns a value lesser than zero and pgbench terminates with an error. However, if the value for number of partitions specified is

Re: pgbench - allow to create partitioned tables

2019-10-03 Thread Ashutosh Sharma
Hi Fabien, Amit, I could see that when an invalid number of partitions is specified, sometimes pgbench fails with an error "invalid number of partitions: ..." whereas many a times it doesn't, instead it creates number of partitions that hasn't been specified by the user. As partitions is an

Re: pgbench - allow to create partitioned tables

2019-10-02 Thread Fabien COELHO
Thanks, attached is a patch with minor modifications which I am planning to push after one more round of review on Thursday morning IST unless there are more comments by anyone else. Pushed. Thanks! -- Fabien.

Re: pgbench - allow to create partitioned tables

2019-10-02 Thread Amit Kapila
On Tue, Oct 1, 2019 at 10:20 AM Amit Kapila wrote: > On Mon, Sep 30, 2019 at 5:17 PM Fabien COELHO wrote: > > > > > I don't want to introduce a new pattern in tests which people can then > > > tomorrow copy at other places even though such code is not required. > > > OTOH, if there is a genuine

Re: pgbench - allow to create partitioned tables

2019-10-01 Thread Amit Kapila
On Tue, Oct 1, 2019 at 8:45 PM Rafia Sabih wrote: > On Tue, 1 Oct 2019 at 16:48, Fabien COELHO wrote: > >> >> >> Yeah, I know that, but this doesn't look quite right. I mean to say >> >> whatever we want to say via this message is correct, but I am not >> >> completely happy with the display

Re: pgbench - allow to create partitioned tables

2019-10-01 Thread Rafia Sabih
On Tue, 1 Oct 2019 at 16:48, Fabien COELHO wrote: > > >> Yeah, I know that, but this doesn't look quite right. I mean to say > >> whatever we want to say via this message is correct, but I am not > >> completely happy with the display part. How about something like: > >> "pgbench_accounts is

Re: pgbench - allow to create partitioned tables

2019-10-01 Thread Fabien COELHO
Yeah, I know that, but this doesn't look quite right. I mean to say whatever we want to say via this message is correct, but I am not completely happy with the display part. How about something like: "pgbench_accounts is missing, you need to do initialization (\"pgbench -i\") in database

Re: pgbench - allow to create partitioned tables

2019-10-01 Thread Rafia Sabih
On Tue, 1 Oct 2019 at 15:39, Amit Kapila wrote: > On Tue, Oct 1, 2019 at 11:51 AM Fabien COELHO wrote: > >> >> Hello Amit, >> >> > 1. ran pgindent >> > 2. As per Alvaro's suggestions move few function definitions. >> > 3. Changed one or two comments and fixed spelling at one place. >> >> Thanks

Re: pgbench - allow to create partitioned tables

2019-10-01 Thread Amit Kapila
On Tue, Oct 1, 2019 at 11:51 AM Fabien COELHO wrote: > > Hello Amit, > > > 1. ran pgindent > > 2. As per Alvaro's suggestions move few function definitions. > > 3. Changed one or two comments and fixed spelling at one place. > > Thanks for the improvements. > > Not sure why you put "XXX - " in

Re: pgbench - allow to create partitioned tables

2019-10-01 Thread Fabien COELHO
Hello Amit, 1. ran pgindent 2. As per Alvaro's suggestions move few function definitions. 3. Changed one or two comments and fixed spelling at one place. Thanks for the improvements. Not sure why you put "XXX - " in front of "append_fillfactor" comment, though. + fprintf(stderr, + "no

Re: pgbench - allow to create partitioned tables

2019-09-30 Thread Amit Kapila
On Mon, Sep 30, 2019 at 5:17 PM Fabien COELHO wrote: > > > > I don't want to introduce a new pattern in tests which people can then > > tomorrow copy at other places even though such code is not required. > > OTOH, if there is a genuine need for the same, then I am fine. > > Hmmm. The committer

Re: pgbench - allow to create partitioned tables

2019-09-30 Thread Fabien COELHO
Hello Amit, $node->safe_psql('postgres', "CREATE TABLESPACE regress_pgbench_tap_1_ts LOCATION '$ets';" I think that this last command fails if the path contains a "'", so the '-escaping is necessary. I had to make changes in TAP tests before because it was not working when the path

Re: pgbench - allow to create partitioned tables

2019-09-30 Thread Amit Kapila
On Mon, Sep 30, 2019 at 2:26 PM Fabien COELHO wrote: > > I am leaning towards approach (b) unless you and or Alvaro feels (a) > > is good for now or if you have some other idea. > > No other idea. I put back the tablespace creation which I just removed, > with comments about why it is there. > >

Re: pgbench - allow to create partitioned tables

2019-09-30 Thread Fabien COELHO
Hello Amit, Attached v18: - remove the test tablespace I had to work around a strange issue around partitioned tables and the default tablespace. - if (tablespace != NULL) + if (tablespace != NULL && strcmp(tablespace, "pg_default") != 0) [...] I don't think we need any such

Re: pgbench - allow to create partitioned tables

2019-09-30 Thread Amit Kapila
On Sat, Sep 28, 2019 at 11:41 AM Fabien COELHO wrote: > > > Hello Amit, > > > I think we might also need to use pg_get_partkeydef along with > > pg_partition_tree to fetch the partition method information. However, > > I think to find reloid of pgbench_accounts in the current search path, > > we

Re: pgbench - allow to create partitioned tables

2019-09-28 Thread Fabien COELHO
Hello Amit, I think we might also need to use pg_get_partkeydef along with pg_partition_tree to fetch the partition method information. However, I think to find reloid of pgbench_accounts in the current search path, we might need to use some part of query constructed by Fabien. Fabien, what

Re: pgbench - allow to create partitioned tables

2019-09-27 Thread Amit Kapila
On Fri, Sep 27, 2019 at 7:05 PM Alvaro Herrera wrote: > > On 2019-Sep-27, Amit Kapila wrote: > > > The other thing is that the query used in patch to fetch partition > > information seems correct to me, but maybe there is a better way to > > get that information. > > I hadn't looked at that, but

Re: pgbench - allow to create partitioned tables

2019-09-27 Thread Alvaro Herrera
On 2019-Sep-27, Amit Kapila wrote: > Thanks, Alvaro, both seem like good suggestions to me. However, there > are a few more things where your feedback can help: > a. With new options, we will partition pgbench_accounts and the > reason is that because that is the largest table. Do we need to

Re: pgbench - allow to create partitioned tables

2019-09-26 Thread Amit Kapila
On Fri, Sep 27, 2019 at 2:36 AM Alvaro Herrera wrote: > On 2019-Sep-26, Fabien COELHO wrote: > > > pgbench's main() is overly long already, and the new code being added > > > seems to pollute it even more. Can we split it out into a static > > > function that gets placed, say, just below

Re: pgbench - allow to create partitioned tables

2019-09-26 Thread Alvaro Herrera
On 2019-Sep-26, Fabien COELHO wrote: > > Hello Alvaro, > > > pgbench's main() is overly long already, and the new code being added > > seems to pollute it even more. Can we split it out into a static > > function that gets placed, say, just below disconnect_all() or maybe > > after

Re: pgbench - allow to create partitioned tables

2019-09-26 Thread Fabien COELHO
Hello Alvaro, pgbench's main() is overly long already, and the new code being added seems to pollute it even more. Can we split it out into a static function that gets placed, say, just below disconnect_all() or maybe after runInitSteps? I agree that refactoring is a good idea, but I do

Re: pgbench - allow to create partitioned tables

2019-09-26 Thread Alvaro Herrera
pgbench's main() is overly long already, and the new code being added seems to pollute it even more. Can we split it out into a static function that gets placed, say, just below disconnect_all() or maybe after runInitSteps? (Also, we seem to be afraid of function prototypes. Why not move the

Re: pgbench - allow to create partitioned tables

2019-09-26 Thread Fabien COELHO
Okay, I think making the check consistent is a step forward. The latest patch is not compiling for me. Argh, shame on me! [...] We don't recommend to start messages with a capital letter. See "Upper Case vs. Lower Case" section in docs [1]. It is not that we have not used it anywhere

Re: pgbench - allow to create partitioned tables

2019-09-25 Thread Amit Kapila
On Tue, Sep 24, 2019 at 6:59 PM Fabien COELHO wrote: > > > For you or me, it might be okay as we have discussed this case, but it > > won't be apparent to others. This doesn't buy us much, so it is better > > to keep this code consistent with other places that check for > > partitions. > >

Re: pgbench - allow to create partitioned tables

2019-09-24 Thread Fabien COELHO
Hello Amit, {...] If you agree with this, then why haven't you changed below check in patch: + if (partition_method != PART_NONE) + printf("partition method: %s\npartitions: %d\n", +PARTITION_METHOD[partition_method], partitions); This is exactly the thing bothering me. It won't be easy

Re: pgbench - allow to create partitioned tables

2019-09-23 Thread Amit Kapila
On Mon, Sep 23, 2019 at 11:58 AM Fabien COELHO wrote: > > > Hello Amit, > > > It is better for a user to write a custom script for such cases. > > I kind-of agree, but IMHO this is not for pgbench to decide what is better > for the user and to fail on a script that would not fail. > > > Because

Re: pgbench - allow to create partitioned tables

2019-09-23 Thread Fabien COELHO
Hello Amit, It is better for a user to write a custom script for such cases. I kind-of agree, but IMHO this is not for pgbench to decide what is better for the user and to fail on a script that would not fail. Because after that "select-only" or "simple-update" script doesn't make any

Re: pgbench - allow to create partitioned tables

2019-09-22 Thread Amit Kapila
On Sun, Sep 22, 2019 at 12:22 PM Fabien COELHO wrote: > >>sh> pgbench -T 10 > >>... > >>partitions: 0 > > > > I am not sure how many users would be able to make out that it is a > > run where actual partitions are not present unless they beforehand > > know and detect such a condition

Re: pgbench - allow to create partitioned tables

2019-09-21 Thread Amit Kapila
On Sat, Sep 21, 2019 at 1:18 PM Fabien COELHO wrote: > > Yes, this code is correct. I am not sure if you understood the point, > > so let me try again. I am bothered about below code in the patch: > > + /* only print partitioning information if some partitioning was detected > > */ > > + if

Re: pgbench - allow to create partitioned tables

2019-09-21 Thread Fabien COELHO
Hello Amit, Yes, this code is correct. I am not sure if you understood the point, so let me try again. I am bothered about below code in the patch: + /* only print partitioning information if some partitioning was detected */ + if (partition_method != PART_NONE) This is the only place now

Re: pgbench - allow to create partitioned tables

2019-09-20 Thread Amit Kapila
On Sat, Sep 21, 2019 at 12:26 AM Fabien COELHO wrote: > > >> I would not bother to create a patch for so small an improvement. This > >> makes sense in passing because the created function makes it very easy, > >> but otherwise I'll just drop it. > > > > I would prefer to drop for now. > >

Re: pgbench - allow to create partitioned tables

2019-09-20 Thread Fabien COELHO
Hello Amit, I would not bother to create a patch for so small an improvement. This makes sense in passing because the created function makes it very easy, but otherwise I'll just drop it. I would prefer to drop for now. Attached v13 does that, I added a comment instead. I do not think that

Re: pgbench - allow to create partitioned tables

2019-09-20 Thread Amit Kapila
On Fri, Sep 20, 2019 at 10:29 AM Fabien COELHO wrote: > > >> 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 > >>

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Fabien COELHO
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:

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Fabien COELHO
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 You seem to have attached some previous version (v2) of this patch. I could see old issues in the patch which we

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Amit Kapila
On Fri, Sep 20, 2019 at 12:41 AM Fabien COELHO wrote: > > > 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

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Amit Kapila
On Fri, Sep 20, 2019 at 12:41 AM Fabien COELHO wrote: > > 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 > You seem to

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Fabien COELHO
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

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Dilip Kumar
On Thu, Sep 19, 2019 at 11:47 AM Amit Kapila wrote: > > On Thu, Sep 19, 2019 at 10:25 AM Fabien COELHO wrote: > > Hello Amit, > > > > >> Yes, on -i it will fail because the syntax will not be recognized. > > > > > > Maybe we should be checking the server version, which would allow to > > >

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Amit Langote
Hi Fabien, On Thu, Sep 19, 2019 at 1:55 PM Fabien COELHO wrote: > Hello Amit, > > >> Yes, on -i it will fail because the syntax will not be recognized. > > > > Maybe we should be checking the server version, which would allow to > > produce more useful error messages when these options are used

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Amit Kapila
On Thu, Sep 19, 2019 at 10:25 AM Fabien COELHO wrote: > Hello Amit, > > >> Yes, on -i it will fail because the syntax will not be recognized. > > > > Maybe we should be checking the server version, which would allow to > > produce more useful error messages when these options are used against > >

Re: pgbench - allow to create partitioned tables

2019-09-19 Thread Amit Kapila
On Wed, Sep 18, 2019 at 10:33 PM Fabien COELHO wrote: > > > Hello Amit, > > >> - use search_path to find at most one pgbench_accounts > >> It still uses left join because I still think that it is appropriate. > >> I added a lateral to avoid repeating the array_position call > >> to

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Fabien COELHO
Hello Amit, Yes, on -i it will fail because the syntax will not be recognized. Maybe we should be checking the server version, which would allow to produce more useful error messages when these options are used against older servers, like if (sversion < 1) fprintf(stderr, "cannot

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Amit Langote
Hi Fabien, On Thu, Sep 19, 2019 at 2:03 AM Fabien COELHO wrote: > > If that is the case, then I think if user gives --partitions for the old > > server version, it will also give an error? > > Yes, on -i it will fail because the syntax will not be recognized. Maybe we should be checking the

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Fabien COELHO
Hello Amit, - use search_path to find at most one pgbench_accounts It still uses left join because I still think that it is appropriate. I added a lateral to avoid repeating the array_position call to manage the search_path, and use explicit pg_catalog everywhere. It would be

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Fabien COELHO
+ "group by 1, 2 " I have a question, wouldn't it be sufficient to just group by 1? Conceptually yes, it is what is happening in practice, but SQL requires that non aggregated columns must appear explicitely in the GROUP BY clause, so I have to put it even if it will not change groups.

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Amit Kapila
On Wed, Sep 18, 2019 at 12:19 AM Fabien COELHO wrote: > > > Attached v9: > > - remove the PART_UNKNOWN and use partitions = -1 to tell > that there is an error, and partitions >= 1 to print info > - use search_path to find at most one pgbench_accounts > It still uses left join because

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Dilip Kumar
On Wed, Sep 18, 2019 at 1:02 PM Fabien COELHO wrote: > */ + res = PQexec(con, + "select o.n, p.partstrat, pg_catalog.count(p.partrelid) " + "from pg_catalog.pg_class as c " + "join pg_catalog.pg_namespace as n on (n.oid = c.relnamespace) " + "cross join lateral (select

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Fabien COELHO
Hello Amit, +fprintf(stderr, "invalid partition type, expecting \"range\" or \"hash\"," How about "partitioning method" instead of "partition type"? Indeed, this is a left over from a previous version. +fprintf(stderr, "--partition-method requires actual

Re: pgbench - allow to create partitioned tables

2019-09-18 Thread Amit Langote
Hi Fabien, On Wed, Sep 18, 2019 at 3:49 AM Fabien COELHO wrote: > Attached v9: Thanks. This seems to work well. Couple of nitpicks on parameter error messages. +fprintf(stderr, "invalid partition type, expecting \"range\" or \"hash\"," How about "partitioning method"

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Fabien COELHO
Hello Erikjan, [pgbench-init-partitioned-9.patch] Turns out this patch needed a dos2unix treatment. It's easy to do but it takes time to figure it out (I'm dumb). I for one would be happy to receive patches not so encumbered :) AFAICR this is usually because your mailer does not

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Erikjan Rijkers
On 2019-09-17 20:49, Fabien COELHO wrote: Attached v9: [pgbench-init-partitioned-9.patch] Turns out this patch needed a dos2unix treatment. It's easy to do but it takes time to figure it out (I'm dumb). I for one would be happy to receive patches not so encumbered :) thanks, Erik

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Fabien COELHO
Attached v9: - remove the PART_UNKNOWN and use partitions = -1 to tell that there is an error, and partitions >= 1 to print info - use search_path to find at most one pgbench_accounts It still uses left join because I still think that it is appropriate. I added a lateral to avoid

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Amit Kapila
On Tue, Sep 17, 2019 at 6:38 PM Fabien COELHO wrote: > > > >> It is used at one place where we can set PART_NONE without much loss. > >> Having lesser invalid values makes code easier to follow. > > > > Looking more closely at this case: > > + else if (PQntuples(res) != 1) > > + { > > + /* unsure

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Fabien COELHO
Hello Amit, Why can't we change it as attached? I think that your version works, but I do not like much the condition for the normal case which is implicitely assumed. The solution I took has 3 clear-cut cases: 1 error against a server without partition support, detect multiple

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Fabien COELHO
Hello Amit, One more comment: +typedef enum { PART_NONE, PART_RANGE, PART_HASH, PART_UNKNOWN } + partition_method_t; See, if we can eliminate PART_UNKNOWN. I'm not very happy with this one, but I wanted to differentiate "we do know that it is not partitioned" from "we do not know if it

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Amit Kapila
On Tue, Sep 17, 2019 at 4:24 PM Amit Kapila wrote: > > On Sat, Sep 14, 2019 at 6:35 PM Fabien COELHO wrote: > > One more comment: > +typedef enum { PART_NONE, PART_RANGE, PART_HASH, PART_UNKNOWN } > + partition_method_t; > > See, if we can eliminate PART_UNKNOWN. I don't see much use of same.

Re: pgbench - allow to create partitioned tables

2019-09-17 Thread Amit Kapila
On Sat, Sep 14, 2019 at 6:35 PM Fabien COELHO wrote: > I'm ensuring that there is always a one line answer, whether it is > partitioned or not. Maybe the count(*) should be count(something in p) to > get 0 instead of 1 on non partitioned tables, though, but this is hidden > in

Re: pgbench - allow to create partitioned tables

2019-09-14 Thread Fabien COELHO
Hello Amit, I'm ensuring that there is always a one line answer, whether it is partitioned or not. Maybe the count(*) should be count(something in p) to get 0 instead of 1 on non partitioned tables, though, but this is hidden in the display anyway. Sure, but I feel the code will be

Re: pgbench - allow to create partitioned tables

2019-09-14 Thread Amit Kapila
On Fri, Sep 13, 2019 at 11:06 PM Fabien COELHO wrote: > > Hello Amit, > > >>> + res = PQexec(con, > >>> + "select p.partstrat, count(*) " > >>> + "from pg_catalog.pg_class as c " > >>> + "left join pg_catalog.pg_partitioned_table as p on (p.partrelid = > >>> c.oid) " > >>> + "left join

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Amit Kapila
On Fri, Sep 13, 2019 at 6:36 PM Alvaro Herrera wrote: > > On 2019-Sep-13, Amit Kapila wrote: > > > I would like to take inputs from others as well for the display part > > of this patch. After this patch, for a simple-update pgbench test, > > the changed output will be as follows (note:

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Fabien COELHO
Hello Amit, + res = PQexec(con, + "select p.partstrat, count(*) " + "from pg_catalog.pg_class as c " + "left join pg_catalog.pg_partitioned_table as p on (p.partrelid = c.oid) " + "left join pg_catalog.pg_inherits as i on (c.oid = i.inhparent) " + "where c.relname = 'pgbench_accounts' " +

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Alvaro Herrera
On 2019-Sep-13, Amit Kapila wrote: > I would like to take inputs from others as well for the display part > of this patch. After this patch, for a simple-update pgbench test, > the changed output will be as follows (note: partition method and > partitions): > pgbench.exe -c 4 -j 4 -T 10 -N

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Alvaro Herrera
On 2019-Sep-13, Amit Kapila wrote: > On Fri, Sep 13, 2019 at 1:50 PM Fabien COELHO wrote: > > >>> Is there a reason why we treat "partitions = 0" as a valid value? > > >> > > >> Yes. It is an explicit "do not create partitioned tables", which differ > > >> from 1 which says "create a

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Amit Kapila
On Fri, Sep 13, 2019 at 1:50 PM Fabien COELHO wrote: > >>> Is there a reason why we treat "partitions = 0" as a valid value? > >> > >> Yes. It is an explicit "do not create partitioned tables", which differ > >> from 1 which says "create a partitionned table with just one partition". > > > > Why

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Dilip Kumar
On Fri, Sep 13, 2019 at 2:05 PM Fabien COELHO wrote: > > > Hello Dilip, > > > + /* For RANGE, we use open-ended partitions at the beginning and end */ > > + if (p == 1) > > + sprintf(minvalue, "minvalue"); > > + else > > + sprintf(minvalue, INT64_FORMAT, (p-1) * part_size + 1); > > + > > + if (p

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Fabien COELHO
Hello Dilip, + /* For RANGE, we use open-ended partitions at the beginning and end */ + if (p == 1) + sprintf(minvalue, "minvalue"); + else + sprintf(minvalue, INT64_FORMAT, (p-1) * part_size + 1); + + if (p < partitions) + sprintf(maxvalue, INT64_FORMAT, p * part_size + 1); + else +

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Dilip Kumar
On Fri, Sep 13, 2019 at 1:35 PM Fabien COELHO wrote: Thanks for the updated version of the patch. > > Generally, we give one blank line between the variable declaration and > > the first statement of the block. > > Ok. > > > (p-1) -> (p - 1) > > Ok. > > > I am just wondering will it be a good

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Fabien COELHO
Hello Amit, Is there a reason why we treat "partitions = 0" as a valid value? Yes. It is an explicit "do not create partitioned tables", which differ from 1 which says "create a partitionned table with just one partition". Why would anyone want to use --partitions option in the first case

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Fabien COELHO
Hello Dilip, Generally, we give one blank line between the variable declaration and the first statement of the block. Ok. (p-1) -> (p - 1) Ok. I am just wondering will it be a good idea to expand it to support multi-level partitioning? ISTM that how the user could specify multi-level

Re: pgbench - allow to create partitioned tables

2019-09-13 Thread Amit Kapila
On Wed, Sep 11, 2019 at 6:08 PM Fabien COELHO wrote: > > I would like to take inputs from others as well for the display part of this patch. After this patch, for a simple-update pgbench test, the changed output will be as follows (note: partition method and partitions): pgbench.exe -c 4 -j 4

Re: pgbench - allow to create partitioned tables

2019-09-12 Thread Dilip Kumar
On Wed, Sep 11, 2019 at 6:08 PM Fabien COELHO wrote: > Attached an updated version. I have reviewed the patch and done some basic testing. It works as per the expectation I have a few cosmetic comments 1. + if (partitions >= 1) + { + char ff[64]; + ff[0] = '\0'; + append_fillfactor(ff,

Re: pgbench - allow to create partitioned tables

2019-09-11 Thread Fabien COELHO
Hello Amit, Thanks for the feedback. + case 11: /* partitions */ + initialization_option_set = true; + partitions = atoi(optarg); + if (partitions < 0) + { + fprintf(stderr, "invalid number of partitions: \"%s\"\n", + optarg); + exit(1); + } + break; Is there a reason why we treat

Re: pgbench - allow to create partitioned tables

2019-09-09 Thread Amit Kapila
On Mon, Aug 26, 2019 at 11:04 PM Fabien COELHO wrote: > > > > Just one suggestion --partition-method option should be made dependent > > on --partitions, because it has no use unless used with --partitions. > > What do you think? > Some comments: * + case 11: /* partitions */ +

Re: pgbench - allow to create partitioned tables

2019-08-27 Thread Asif Rehman
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:not tested Thanks. All looks good, making it ready for committer. Regards,

Re: pgbench - allow to create partitioned tables

2019-08-26 Thread Fabien COELHO
Just one suggestion --partition-method option should be made dependent on --partitions, because it has no use unless used with --partitions. What do you think? Why not. V4 attached. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index

Re: pgbench - allow to create partitioned tables

2019-08-26 Thread Asif Rehman
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:not tested Hi, The patch looks good to me, Just one suggestion

Re: pgbench - allow to create partitioned tables

2019-07-26 Thread Fabien COELHO
Attached v3 fixes strcasecmp non portability on windows, per postgresql patch tester. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 816f9cc4c7..3e8e292e39 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -306,6

Re: pgbench - allow to create partitioned tables

2019-07-24 Thread Fabien COELHO
# and look at latency: # no parts = 0.071 ms # 1 hash = 0.071 ms (did someone optimize this case?!) # 2 hash ~ 0.126 ms (+ 0.055 ms) # 50 hash ~ 0.155 ms # 100 hash ~ 0.178 ms # 150 hash ~ 0.232 ms # 200 hash ~ 0.279 ms # overhead ~ (0.050 + [0.0005-0.0008] * nparts) ms

Re: pgbench - allow to create partitioned tables

2019-07-24 Thread Fabien COELHO
Hello Simon, While doing some performance tests and reviewing patches, I needed to create partitioned tables. Given the current syntax this is time consumming. Good idea. I wonder why we didn't have it already. Probably because I did not have to create partitioned table for some

Re: pgbench - allow to create partitioned tables

2019-07-23 Thread Simon Riggs
On Tue, 23 Jul 2019 at 19:26, Fabien COELHO wrote: > > Hello devs, > > While doing some performance tests and reviewing patches, I needed to > create partitioned tables. Given the current syntax this is time > consumming. > Good idea. I wonder why we didn't have it already. > The attached

pgbench - allow to create partitioned tables

2019-07-23 Thread Fabien COELHO
Hello devs, While doing some performance tests and reviewing patches, I needed to create partitioned tables. Given the current syntax this is time consumming. The attached patch adds two options to create a partitioned "account" table in pgbench. It allows to answer quickly simple