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
