Hi,
On 2018-03-01 14:09:02 +0100, Fabien COELHO wrote:
> > A bit concerned that we're turning pgbench into a kitchen sink.
>
> I do not understand "kitchen sink" expression in this context, and your
> general concerns about pgbench in various comments in your message.
We're adding a lot of stuff to pgbench that only a few people
use. There's a lot of duplication with similar parts of code in other
parts of the codebase. pgbench in my opinion is a tool to facilitate
postgres development, not a goal in itself.
It's a bad measure, but the code growth shows my concerns somewhat:
master: 5660 +817
REL_10_STABLE: 4843 +266
REL9_6_STABLE: 4577 +424
REL9_5_STABLE: 4153 +464
REL9_4_STABLE: 3689 +562
REL9_3_STABLE: 3127 +338
REL9_2_STABLE: 2789 +96
REL9_1_STABLE: 2693
> So this setting-variable-from-query patch goes with having boolean
> expressions (already committed), having conditions (\if in the queue),
> improving the available functions (eg hashes, in the queue)... so that
> existing, data-dependent, realistic benchmarks can be implemented, and
> benefit for the great performance data collection provided by the tool.
I agree that they're useful in a few cases, but they have to consider
that they need to be reviewed and maintained an the project is quite
resource constrained in that regard.
> > - pgbench - test whether a variable exists
> >
> > NR. Submitted first time mid February.
> >
> > Quoting from author:
> > "Note that it is not really that useful for benchmarking, although it
> > does not harm."
> >
> > I'd reject or boot.
>
> As already said, the motivation is that it is a preparation for a (much)
> larger patch which would move pgbench expressions to fe utils and use them
> in "psql".
You could submit it together with that. But I don't see in the first
place why we need to add the feature with duplicate code, just so we can
unify. We can gain it via the unification, no?
Greetings,
Andres Freund
