Re: pgbench more operators & functions - Mailing list pgsql-hackers

Fabien,

* Fabien COELHO (coelho@cri.ensmp.fr) wrote:
> >>bitwise: <<, >>, &, |, ^/#, ~
> >>comparisons: =/==, <>/!=, <, <=, >, >=
> >>logical: and/&&, or/||, xor/^^, not, !
> >
> >I'm not sure that we want to introduce operators '&&', '||' as logical
> >'and' and 'or' when those have specific meaning in PG which is different
> >(array overlaps and concatenation, specifically).  I can certainly see
> >how it could be useful to be able to perform string concatenation in a
> >\set command in pgbench, for example..
> >
> >Also, are we sure that we want to have both '=' and '==' for comparison?
>
> The reason I added C operators is that Pg SQL has '!=' and a few
> others, so once some are there I do not see why others should not be
> there as well for consistency.
>
> Now I agree that having operators which behave differently from psql
> to pgbench is a bad idea.
>
> In the attached patched I only included pg operators, plus "xor"
> which I feel is missing and does not seem to harm.

I'm pretty sure we should hold off on adding 'xor' until it's actually
in PG proper, otherwise we run a very serious risk that whatever we do
to add it in PG will be different from what you're suggesting we do here
for pgbench.

> >[...] I certainly understand how the if() function could be useful
>
> Indeed, some kind of "if" is needed, for instance to implement
> "tpc-b" correctly.

That's an interesting point..  Have you thought about ripping out the
built-in TPC-B-like functionality of pgbench and making that into a
script instead?  Doing so would likely point out places where we need to
add functionality or cases which aren't handled very well.  We'd also be
able to rip out all that code from pgbench and make it into a general
utility instead of "general utility + other stuff."

> >, but I'm not entirely sure that the if(expression, true-result,
> >false-result) is the best approach.  In particular, I recall cases
> >with other languages where that gets to be quite ugly when there
> >are multiple levels of if/else using that approach.
>
> I think that pgbench is not a programming language, but an
> expression language, which means that you have to provide a result,
> so you can only have balanced if/then/else, which simplifies things
> a bit compared to "full" language.
>
> The SQL syntax for CASE is on the very heavy side and would be quite
> complicated to implement in pgbench, so I rejected that and selected
> the simplest possible function for the job.

I'm not quite sure that I follow why you feel that CASE would be too
difficult to implement here..?

> Maybe the "if" function could be named something fancier to avoid
> possible future clash on an eventual "if" keyword? Note that excel
> has an IF function. Maybe "conditional"... However, as I do not
> envision pgbench growing to a full language, I would be fine keeping
> "if" as it is.

Excel would be one of the ugly if-nesting cases that I was thinking
of, actually.  I'm certainly not thrilled with the idea of modelling
anything off of it.

> >The table should really have a row per operator, which is what we
> >do in pretty much all of the core documention. [...]
>
> Ok for one line per operator.

Thanks!

> >The question which was brought up about having a line-continuation
> >(eg: '\') character would be useful,
>
> :-) I submitted a patch for that, which got rejected and resulted in
> Tom making pgbench share psql lexer. This is a good thing, although
> the continuation extension was lost in the process. Also this means
> that now such logic would probably be shared with psql, not
> necessarily a bad thing either, but clearly material for another
> patch.

Sure, that makes sense to do independently.

> >which really makes me wonder if we shouldn't try to come up with a
> >way to create functions in a pgbench script that can later be used
> >in a \set command.
>
> I do not think that pgbench script is a good target to define
> functions, because the script is implicitely in a very large loop
> which is executing it over and over again. I do not think that it
> would make much sense to redefine functions on each transaction...
> So my opinion is that without a compeling use case, I would avoid
> such a feature, which would need some careful thinking on the design
> side, and the implementation of which is non trivial.

If we're going to replace the built-in TPC-B functionality then we're
going to need a way to pass in an 'init' script of some kind which only
gets run once, that could certainly be where functions could be
defined.  As for the use-case for functions, well, we need an if()
construct, as discussed, and having a need for loops doesn't seem too
far off from needing conditional control structures.

> >With such an approach, we could have proper control structures for
> >conditionals (if/else), loops (while/for), etc, and not complicate
> >the single-statement set of operators with those constructs.
>
> If you want control, you can already use a DO & PL/pgsql script,
> although it is interpreted server-side. Yet again, I'm not convince
> that pgbench is material for such features, and it would take a
> compeling use case for me to add such a thing. Moreover, the
> currently simple internal data structures and executor would have to
> be profoundly reworked and extended to handle a full language and
> functions.

Doing it server-side would certainly not be the same as having it
client-side.  Perhaps it'd be difficult to rework pgbench to support it,
but I do think it's something we should at least be considering and
making sure we don't wall ourselves off from implementing in the future.
Most scripting languages do support functions of one form or another.

> >Lastly, we should really add some regression tests to pgbench.  We
> >already have the start of a TAP test script which it looks like it
> >wouldn't be too hard to add on to.
>
> I agree that pgbench should be tested systematically. I think that
> this is not limited to these functions and operators but a more
> general and desirable feature, thus is really material for another
> patch.

I don't agree with this- at a minimum, this patch should add regression
tests for the features that it's adding.  Other tests should be added
but we do not need to wait for some full test suite to be dropped into
pgbench before adding any regression tests.

Thanks!

Stephen