Hi Fabien and Alvaro, I found that I have already reviewed this thread before, so I tried to apply the patch, but part of the chunk failed, because of the unused line below which was already removed in the recent related commit. > PGResult *res; I removed the line and fixed the other trailing whitespaces. See the attached latest patch. The attached patch applies, builds cleanly, and passes the regression tests.
On Saturday, November 24, 2018 5:58PM (GMT+9), Fabien Coelho wrote: > About the patch you committed, a post-commit review: > > - the state and function renamings are indeed a good thing. > > - I'm not fond of "now = func(..., now)", I'd have just passed a > reference. > > - I'd put out the meta commands, but keep the SQL case and the state > assignment in the initial function, so that all state changes are in > one function… which was the initial aim of the submission. > Kind of a compromise:-) I have confirmed the following changes: 1. > - I'm not fond of "now = func(..., now)", I'd have just passed a > reference. 1.1. advanceConnectionState(): Removed > now = doExecuteCommand(thread, st, now); 1.2. executeMetaCommand(): direct reference to state Before: >- st->state = CSTATE_ABORTED; >- return now; After: >+ return CSTATE_ABORTED; 2. SQL_COMMAND type is executed in initial function advanceConnectionState(), while META_COMMAND is executed in the subroutine executeMetaCommand(). This seems reasonable to me. 3. The function name also changed, which describes the subroutine better. -static instr_time doExecuteCommand(TState *thread, CState *st, - instr_time now); +static ConnectionStateEnum executeMetaCommand(TState *thread, CState *st, instr_time *now); No problems on my part as I find the changes logical. This also needs a confirmation from Alvaro. Regards, Kirk Jamison
pgbench-state-change-followup-2.patch
Description: pgbench-state-change-followup-2.patch
