Daniel Farina escribió:
> Given that, perhaps a way to fix this is something like this patch-fragment:
>
> """
> {
> PGSS_TUP_V1_0 = 1,
> PGSS_TUP_V1_1,
> - PGSS_TUP_LATEST
> + PGSS_TUP_V1_2
> } pgssTupVersion;
>
> +#define PGSS_TUP_LATEST PGSS_TUP_V1_2
> """
This sounds good. I have seen other places that have the LATEST
definition as part of the enum, assigning the previous value to it. I'm
not really sure which of these is harder to miss when updating the code.
I'm happy with either.
Make sure to use the PGSS_TUP_V1_2 in the two places mentioned, in any
case.
> > Just noticed that you changed the timer to struct Instrumentation. Not
> > really sure about that change. Since you seem to be using only the
> > start time and counter, wouldn't it be better to store only those?
> > Particularly unsure about passing INSTRUMENT_ALL to InstrAlloc().
>
> Yeah, I was unsure about that too.
>
> The motivation was that I need one more piece of information in
> pgss_store (the absolute start time). I was going to widen the
> argument list, but it was looking pretty long, so instead I was
> thinking it'd be more concise to push the entire, typically extant
> Instrumentation struct pointer down.
Would it work to define your own struct to pass around?
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
