Re: [HACKERS] pgbench: Skipping the creating primary keys afterinitialization - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: [HACKERS] pgbench: Skipping the creating primary keys afterinitialization
Date
Msg-id alpine.DEB.2.20.1708271008550.18068@lancre
Whole thread Raw
In response to Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: [HACKERS] pgbench: Skipping the creating primary keys afterinitialization
Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization
List pgsql-hackers
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... 100000 of 100000 tuples (100%) done (elapsed 0.08 s, remaining 0.00 s) done.

It seems that "d" triggered table creation... In fact it seems that the
work is done correctly, but the messages are not in the right place.

Also another issue:
 sh> ./pgbench -i --foreign-keys creating tables... 100000 of 100000 tuples (100%) done (elapsed 0.09 s, remaining 0.00
s)vacuum... set primary keys... done.
 

Foreign keys do not seem to have been set... Please check that all really 
work as expected.


About the documentation:

If a native English speaker could review the text, it would be great.

At least: "Required to invoke" -> "Require to invoke".


About the code:

is_no_vacuum should be a bool?

I'm really hesitating about the out of order processing of options. If the user writes
  sh> pgbench -i --no-vacuum -I v  done.

Then does it make sense to ignore the last thing the user asked for? ISTM 
that processing options in order and keeping the last resulting spec is 
more natural. Appending contradictory options can happen easily when 
scripting, and usual what is meant is the last one.

Again, as pointed out in the previous review, I do not like much 
checkCustomCmds implementation: switch/case, fprintf and return on error 
which will trigger another fprintf and error above... ISTM that you should 
either take into account previous comments or explain why you disagree 
with them, but not send the same code without addressing them in any way.

-- 
Fabien.



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: [HACKERS] type cache for concat functions
Next
From: Fabien COELHO
Date:
Subject: Re: [HACKERS] pgbench: Skipping the creating primary keys afterinitialization