On Tue, 22 Sep 2009, Michael Paquier wrote:

The function doCustom is defined with a void.

I see this in pgbench.c:

  /* return false iff client should be disconnected */
  static bool
  doCustom(CState *st, instr_time *conn_time)

I think you need to increase the verbosity of the error messages when you're working on this code, because when I compile I get a slew of these errors pointing to the problem too:

pgbench.c:1009: warning: return with no value, in function returning non-void

The fix is that when there's an error, you need to do this:

    return clientDone(st, false);

To indicate to DoCustom that things have gone badly and it shouldn't try executing anything else in the script.

Your patch didn't apply cleanly either, but I think that's not your fault. There's a lot of code that looks quite similar in this area and both "git apply" and "patch" seemed confused. Probably needs more context around the diff next time.

See attached a patch of this setshell feature. If you use in a script file something like:
  /setshell param_set setshellparam.sh
pgbench reads from the shell script setshellparam.sh the first output value, verifies if it is an integer, then manages it as a pgbench parameter. I did not take into account you 4th and 5th arguments, so it is just a basic implementation.

That part looks fine so far, but what actually needs to happen here is that the rest of the line after the name of the script should be passed to the script as part of its command line.

This patch is moving in the right direction, it still needs some work. So far my list of concerns is:

 * Make the patch diff apply cleanly to HEAD (I'm past this)

* Update the return logic (already fixed in my copy, just need to update all the "return" statements with no value)

* Make "setshell" pass through the rest of the command line (I could fix this, but don't really want to)

* We need a much simpler example how this patch can be helpful and used (this I will do, I have a couple of ideas)

* The documentation needs to be updated with that example and the rest of the details on what you've added. If you plan to submit more patches in the future, you probably want to get familiar with this eventually, and I encourage you to take a shot at it. I have some other pgbench docs fixes to apply anyway soon, so I wouldn't mind taking care of this too once we're close to code that can be committed.

We're trying to close up this CommitFest, so I'm going to mark this one "returned with feedback" for now. I've got it sitting in my personal git repo how and can help out with the details. I'll be glad to work with you to get it into shape for the next CommitFest, where I think it can be a useful feature to add.

--
* Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to