The patch needs a rebase in the documentation because of the xml-ilization
of the sgml doc.
Thank you for the notification! Attached rebased patch.
Ok. The new version works for me.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your
On Wed, Oct 18, 2017 at 5:32 AM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
>>> Attached the updated version patch.
>>
>>
>> Applies, compiles, make check & tap test ok, doc is fine.
>>
>> All is well for me: the feature is useful, code is simple and clean, it is
>> easy to invoke, easy to ext
Hello Masahiko-san,
Attached the updated version patch.
Applies, compiles, make check & tap test ok, doc is fine.
All is well for me: the feature is useful, code is simple and clean, it is
easy to invoke, easy to extend as well, which I'm planning to do once it gets
in.
I switched the pa
On Thu, Sep 21, 2017 at 5:23 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
>>> ISTM that you probably intended "\(.*\)" (actual parenthesis) instead of
>>> "(.*)" (memorization) in the data generation message check.
>>
>>
>> Thank you, fixed it.
>>
>>> Otherwise all is well for me.
>>>
>>
>>
Hello Masahiko-san,
ISTM that you probably intended "\(.*\)" (actual parenthesis) instead of
"(.*)" (memorization) in the data generation message check.
Thank you, fixed it.
Otherwise all is well for me.
Attached the updated version patch.
Applies, compiles, make check & tap test ok, d
On Wed, Sep 20, 2017 at 3:26 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
> v14 applies, compiles and works. TAP tests provide good coverage.
>
> ISTM that you probably intended "\(.*\)" (actual parenthesis) instead of
> "(.*)" (memorization) in the data generation message check.
>
Thank yo
Hello Masahiko-san,
v14 applies, compiles and works. TAP tests provide good coverage.
ISTM that you probably intended "\(.*\)" (actual parenthesis) instead of
"(.*)" (memorization) in the data generation message check.
Otherwise all is well for me.
--
Fabien.
--
Sent via pgsql-hackers mai
On Tue, Sep 19, 2017 at 12:41 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
>> Attached the latest version patch incorporated the tap tests.
>> Please review it.
>
>
> Patch applies, compilation & make check ok.
>
> Tests are simple and provide good coverage of new functionalities.
>
> I woul
Hello Masahiko-san,
Attached the latest version patch incorporated the tap tests.
Please review it.
Patch applies, compilation & make check ok.
Tests are simple and provide good coverage of new functionalities.
I would suggest to add '--unlogged-tables' so speedup the tests a little.
Comme
On Fri, Sep 8, 2017 at 9:52 AM, Masahiko Sawada wrote:
> On Thu, Sep 7, 2017 at 4:15 PM, Fabien COELHO wrote:
>>
Very very minor comments that I should have noticed before, sorry for
this
additional round trip.
>>>
>>>
>>> Thank you for the dedicated review!
>>
>>
>> I'm someone at
On Thu, Sep 7, 2017 at 4:15 PM, Fabien COELHO wrote:
>
>>> Very very minor comments that I should have noticed before, sorry for
>>> this
>>> additional round trip.
>>
>>
>> Thank you for the dedicated review!
>
>
> I'm someone at times pigheaded, I think in the good sense if it is possible,
> and
Very very minor comments that I should have noticed before, sorry for this
additional round trip.
Thank you for the dedicated review!
I'm someone at times pigheaded, I think in the good sense if it is
possible, and I like to finish what I start:-)
Patch applies, compiles, works, everythin
On Wed, Sep 6, 2017 at 4:01 PM, Fabien COELHO wrote:
>
> Applies, compiles, works for me.
>
> Very very minor comments that I should have noticed before, sorry for this
> additional round trip.
Thank you for the dedicated review!
>
> In the help line, move -I just after -i, to put short options
Applies, compiles, works for me.
Very very minor comments that I should have noticed before, sorry for this
additional round trip.
In the help line, move -I just after -i, to put short options in
alphabetical and decreasing importance order. On this line, also add the
information about the
On Wed, Sep 6, 2017 at 12:11 AM, Fabien COELHO wrote:
>
>> Sorry, I don't follow that. You meant I should add a newline before
>> pg_realloc()? That is,
>>
>> +initialize_cmds =
>> +(char *) pg_realloc(initialize_cmds,
>> +
Sorry, I don't follow that. You meant I should add a newline before
pg_realloc()? That is,
+initialize_cmds =
+(char *) pg_realloc(initialize_cmds,
+sizeof(char) * n_cmds + 1);
Yes. Or maybe my
On Tue, Sep 5, 2017 at 4:06 PM, Fabien COELHO wrote:
>
>> Attached the latest patch. Please review it.
>
>
> Patch applies and compiles cleanly.
>
> Three very minor points:
>
> Typo added in the documentation: ".Each" -> ". Each".
>
> In "case 8:" there is a very long line which lacks a newline b
Attached the latest patch. Please review it.
Patch applies and compiles cleanly.
Three very minor points:
Typo added in the documentation: ".Each" -> ". Each".
In "case 8:" there is a very long line which lacks a newline before
pg_realloc second argument.
I think that the check should si
On Tue, Sep 5, 2017 at 2:33 AM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
Currently TRUNCATE pgbench_accounts command is executed within a
transaction started immediately before it. If we move it out of the
transaction, the table data will be truncated even if the copying data
Hello Masahiko-san,
Currently TRUNCATE pgbench_accounts command is executed within a
transaction started immediately before it. If we move it out of the
transaction, the table data will be truncated even if the copying data
failed. Maybe we can do TRUNCATE pgbench_accounts, pgbench_history
inst
On Fri, Sep 1, 2017 at 11:29 PM, Fabien COELHO wrote:
>
>>> I'm wondering whether this truncation should be yet another available
>>> command? Hmmm... maybe not.
>>
>>
>> Currently TRUNCATE pgbench_accounts command is executed within a
>> transaction started immediately before it. If we move it ou
I'm wondering whether this truncation should be yet another available
command? Hmmm... maybe not.
Currently TRUNCATE pgbench_accounts command is executed within a
transaction started immediately before it. If we move it out of the
transaction, the table data will be truncated even if the copyi
On Fri, Sep 1, 2017 at 4:42 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
> Patch applies and compiles.
>
> One bug found, and some minor points again. Sorry for this hopefully last
> iteration... I'm kind of an iterative person...
>
> I've generated the doc to look a it.
>
> Short option "-I
Hello Masahiko-san,
Patch applies and compiles.
One bug found, and some minor points again. Sorry for this hopefully last
iteration... I'm kind of an iterative person...
I've generated the doc to look a it.
Short option "-I" does not use a "=", it should be "-I
custom_init_commands".
Als
On Thu, Aug 31, 2017 at 4:35 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
>> [...] Personally I prefer "t" for table creation because "c" for create is
>> a generic word. We might want to have another initialization command that
>> creates something.
>
>
> Ok, good point.
>
>
> About the pat
Hello Masahiko-san,
[...] Personally I prefer "t" for table creation because "c" for create
is a generic word. We might want to have another initialization command
that creates something.
Ok, good point.
About the patch: applies, compiles, works for me. A few minor comments.
While re-read
On Wed, Aug 30, 2017 at 3:39 PM, Fabien COELHO wrote:
>
> Hello,
>
>>> Hmmm. The existing "is_no_vacuum" variable is typed *both* as int (in
>>> "main") and as bool (in "init"), called by main (yuk!). I see no reason
>>> to
>>> choose the bad one for the global:-)
>>
>>
>> Yeah, I think this might
Hello,
Hmmm. The existing "is_no_vacuum" variable is typed *both* as int (in
"main") and as bool (in "init"), called by main (yuk!). I see no reason to
choose the bad one for the global:-)
Yeah, I think this might be a good timing to re-consider int-for-bool
habits in pgbench. If we decided t
On Tue, Aug 29, 2017 at 4:47 PM, Fabien COELHO wrote:
>
> Hello,
>
> Patch applies and works.
>
>>> I would like to insist a little bit: the name was declared as an int but
>>> passed to init and used as a bool there before the patch. Conceptually
>>> what
>>> is meant is really a bool, and I see
Hello,
Patch applies and works.
I would like to insist a little bit: the name was declared as an int but
passed to init and used as a bool there before the patch. Conceptually what
is meant is really a bool, and I see no added value bar saving a few strokes
to have an int. ISTM that recent pgb
On Mon, Aug 28, 2017 at 4:59 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
> Patch applies cleanly, compiles, works for me.
Thank you for reviewing!
>
>>> At least: "Required to invoke" -> "Require to invoke".
>>
>>
>> Fixed.
>
>
> I meant the added one about -I, not the pre-existing one ab
Hello Masahiko-san,
Patch applies cleanly, compiles, works for me.
At least: "Required to invoke" -> "Require to invoke".
Fixed.
I meant the added one about -I, not the pre-existing one about -i.
About the code:
is_no_vacuum should be a bool?
We can change it but I think there is no d
On Sun, Aug 27, 2017 at 5:12 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
>> Attached latest v4 patch. Please review it.
Thank you for reviewing this patch!
>
> Patch applies, compiles.
>
> The messages/options do not seem to work properly:
>
> sh> ./pgbench -i -I t
> done.
Fixed this s
Quick precision to my previous review.
sh> ./pgbench -i -I t
done.
There should be "creating tables..."
Does not seem to have initialized the tables although it was requested...
sh> ./pgbench -i -I d
creating tables...
Probably "filling tables..." would be more appropriate.
--
Fabien.
Hello Masahiko-san,
Attached latest v4 patch. Please review it.
Patch applies, compiles.
The messages/options do not seem to work properly:
sh> ./pgbench -i -I t
done.
Does not seem to have initialized the tables although it was requested...
sh> ./pgbench -i -I d
creating tables...
1
On Tue, Aug 22, 2017 at 5:15 PM, Fabien COELHO wrote:
>
>>> Why not allow -I as a short option for --custom-initialize?
>>
>>
>> Other options for similar purpose such as --foreign-keys also have
>> only a long option. Since I think --custom-initialize option is in the
>> same context as other opt
Why not allow -I as a short option for --custom-initialize?
Other options for similar purpose such as --foreign-keys also have
only a long option. Since I think --custom-initialize option is in the
same context as other options I didn't add short option to it for now.
Because the options name
On Wed, Aug 16, 2017 at 4:55 PM, Fabien COELHO wrote:
>
> Hello Masahiko-san,
>
>> Yeah, once custom initialization patch get committed we can extend it.
>>
>> Attached updated patch. I've incorporated the all comments from Fabien
>> to it, and changed it to single letter version.
>
>
> Patch appl
Hello Masahiko-san,
Yeah, once custom initialization patch get committed we can extend it.
Attached updated patch. I've incorporated the all comments from Fabien
to it, and changed it to single letter version.
Patch applies and works.
A few comments and questions about the code and document
On Tue, Aug 15, 2017 at 2:43 AM, Fabien COELHO wrote:
>
> Hello,
>
>> I think we can use it like --custom-initialize="create_table, vacuum"
>> which is similar to what we specify a connection option to psql for
>> example.
>
>
> Even if it is allowed, do not advertise it. Or use space as a separat
Hello,
I think we can use it like --custom-initialize="create_table, vacuum"
which is similar to what we specify a connection option to psql for
example.
Even if it is allowed, do not advertise it. Or use space as a separator
and not comma. ISTM that with psql connections space is the higher
On Tue, Aug 8, 2017 at 10:50 PM, Fabien COELHO wrote:
>
> Hello Mahahiko-san,
>
> My 0.02€ about the patch & feature, which only reflect my point of view:
Thank you for reviewing the patch!
> Please add a number to patches to avoid ambiguities. This was patch was the
> second sent on the thread.
Hello Mahahiko-san,
My 0.02€ about the patch & feature, which only reflect my point of view:
Please add a number to patches to avoid ambiguities. This was patch was
the second sent on the thread.
There is no need to have both custom_init & init functions. I'll suggest
to remove function "in
Attached patch
I'll look into it.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Aug 3, 2017 at 11:31 PM, Tom Lane wrote:
> Masahiko Sawada writes:
>> If we want to create other tables and load data to them as we want we
>> can do that using psql -f. So an alternative ways is having a flexible
>> style option for example --custom-initialize = { [load, create_pkey,
>>
On Fri, Aug 4, 2017 at 3:24 AM, Fabien COELHO wrote:
>
>>> For the CREATE stuff, the script language is SQL, the command to use it
>>> is
>>> "psql"...
>>
>>
>>> The real and hard part is to fill tables with meaningful pseudo-random
>>> test data which do not violate constraints for any non trivia
For the CREATE stuff, the script language is SQL, the command to use it is
"psql"...
The real and hard part is to fill tables with meaningful pseudo-random
test data which do not violate constraints for any non trivial schema
involving foreign keys and various unique constraints.
The solut
Masahiko Sawada writes:
> If we want to create other tables and load data to them as we want we
> can do that using psql -f. So an alternative ways is having a flexible
> style option for example --custom-initialize = { [load, create_pkey,
> create_fkey, vacuum], ... }. That would solve this in a
Fabien COELHO writes:
> As for a more generic solution, the easy part are the "CREATE" stuff and
> the transaction script stuff (existing pgbench scripts).
> For the CREATE stuff, the script language is SQL, the command to use it is
> "psql"...
> The real and hard part is to fill tables with m
Hello,
My motivation of this proposal is same as what Robert has. I
understand that ad-hoc option can solve only the part of big problem
and it could be cause of mess. However It seems me that the script
especially for table initialization will not be flexible than we
expected. I mean, even if
On Thu, Aug 3, 2017 at 2:00 AM, Robert Haas wrote:
> On Wed, Aug 2, 2017 at 12:34 PM, Tom Lane wrote:
>> Of course. It's also a heck of a lot more flexible. Adding on another
>> ad-hoc option that does the minimum possible amount of work needed to
>> address one specific problem is always going
On Wed, Aug 2, 2017 at 5:50 PM, Tom Lane wrote:
> Robert Haas writes:
>> On Wed, Aug 2, 2017 at 10:17 AM, Tom Lane wrote:
>>> Or in other words, this looks to me quite a bit like the hackery
>>> that resulted in pgbench's -S and -N options, before we figured out
>>> that making it scriptable was
On Wed, Aug 2, 2017 at 12:34 PM, Tom Lane wrote:
> Of course. It's also a heck of a lot more flexible. Adding on another
> ad-hoc option that does the minimum possible amount of work needed to
> address one specific problem is always going to be less work; but after
> we repeat that process five
Robert Haas writes:
> On Wed, Aug 2, 2017 at 11:50 AM, Tom Lane wrote:
>> Well, I'm imagining that "-i" would essentially become a short form
>> of "-b initialize", as already happened for -S and -N, where the script
>> looks something like ...
> I imagine that would be useful for some use cases
On Wed, Aug 2, 2017 at 11:50 AM, Tom Lane wrote:
> Robert Haas writes:
>> On Wed, Aug 2, 2017 at 10:17 AM, Tom Lane wrote:
>>> Or in other words, this looks to me quite a bit like the hackery
>>> that resulted in pgbench's -S and -N options, before we figured out
>>> that making it scriptable wa
Robert Haas writes:
> On Wed, Aug 2, 2017 at 10:17 AM, Tom Lane wrote:
>> Or in other words, this looks to me quite a bit like the hackery
>> that resulted in pgbench's -S and -N options, before we figured out
>> that making it scriptable was a better answer.
> But it's not very clear to me how
On Wed, Aug 2, 2017 at 10:17 AM, Tom Lane wrote:
> Sure, but "no indexes at all" is hardly ever the real goal, is it?
Right.
> So the switch as proposed is only solving part of your problem.
> I'd rather see a solution that addresses a larger range of desires.
That's reasonable.
> Or in other
Robert Haas writes:
> I've actually wanted this exact thing multiple times: most recently,
> to make a non-unique btree index instead of a unique one, and to make
> a hash index instead of a btree one. I don't object to a modest
> effort at coming up with a more general mechanism here, but I also
On Wed, Aug 2, 2017 at 9:41 AM, Tom Lane wrote:
> Robert Haas writes:
>> On Tue, Aug 1, 2017 at 9:49 PM, Masahiko Sawada
>> wrote:
>>> I'd like to propose a new option -I for pgbench command which skips
>>> the creating primary keys after initialized tables.
>
>> I support adding an option for
> I think we could probably do without this ... if you want a non-default
> test setup, why do you need to use "pgbench -i" to create it?
>
> It's not that there's anything greatly wrong with this particular idea,
> it's just that pgbench has too many switches already, and omitting random
> subset
Robert Haas writes:
> On Tue, Aug 1, 2017 at 9:49 PM, Masahiko Sawada wrote:
>> I'd like to propose a new option -I for pgbench command which skips
>> the creating primary keys after initialized tables.
> I support adding an option for this, but I propose that we just make
> it a long-form optio
On Wed, Aug 2, 2017 at 10:25 PM, Robert Haas wrote:
> On Tue, Aug 1, 2017 at 9:49 PM, Masahiko Sawada wrote:
>> I'd like to propose a new option -I for pgbench command which skips
>> the creating primary keys after initialized tables. This option is
>> useful for users who want to do bench markin
On Tue, Aug 1, 2017 at 9:49 PM, Masahiko Sawada wrote:
> I'd like to propose a new option -I for pgbench command which skips
> the creating primary keys after initialized tables. This option is
> useful for users who want to do bench marking with no index or indexes
> other than btree primary inde
Hi all,
I'd like to propose a new option -I for pgbench command which skips
the creating primary keys after initialized tables. This option is
useful for users who want to do bench marking with no index or indexes
other than btree primary index. If we initialize pgbench tables at a
large number sc
64 matches
Mail list logo