Re: extend pgbench expressions with functions

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: extend pgbench expressions with functions
Date: 2016-02-12 15:10:33
Message-ID: CA+TgmoaZ-SjchiWy7nYUKUDNLusCHs9vYj=4HryJzpmuacVT=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Feb 12, 2016 at 5:06 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> I think that this option is too much bother for the small internal purpose
> at hand.

Yeah, I'm really frustrated with this whole thread. There's been a
huge amount of discussion on this patch, but I don't really feel like
it's converging towards something I could actually commit.

For example, I just realized that this patch allows values to be
either a double or an integer and extends the operators to handle
double values. But variables can still only be integers. Please name
any programming language that has such a restriction. The only ones I
can think of are command shells, and those at least flatten everything
to string rather than integer so that you can store the value without
loss of precision - just with loss of type-safety. I think designing
this in this way is quite short-sighted. I don't think variables
should be explicitly typed but they should be able to store a value of
any type that expression evaluation can generate.

Also, as I said back in November, there's really two completely
separate enhancements in here. One of them is to support a new data
type (doubles) and the other is to support functions. Those should
really be separate patches. If neither of you are willing to split
this patch, I'm not willing to commit it. Going over half of this
patch at a time and getting it committed is going to be a lot of work
for me, but I'm willing to do it. I'm not willing to go over the
whole thing at once - that's going to take more time than I have, and
produce what will in my opinion be an inferior commit history. If
somebody else is willing to commit the whole thing as one patch, I'm
not going to object, but I won't do it myself.

I would not worry too much about the list thing at this point. I'm
sure something simple is fine for that. I actually think the patch is
now in decent shape as far as code-cleanliness is concerned, but I'm
not sure we've really looked hard enough at the design. For example,
I find implementing operators as functions in disguise not to be one
of PostgreSQL's most awesome design decisions, and here we are copying
that into pgbench for, well, no benefit that I can see, really. Maybe
it's a good idea and maybe it's a bad idea, but how do we know? That
stuff deserves more discussion. Code cleanup is good, so I do think
it's good that a lot of effort has been put in there, but I don't
think more code cleanup is what's going to get this patch over the
finish line.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Yury Zhuravlev 2016-02-12 15:11:14 Re: GinPageIs* don't actually return a boolean
Previous Message Andres Freund 2016-02-12 14:56:45 Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)