On Thu, Sep 25, 2014 at 1:27 AM, Gregory Smith <gregsmithpgsql@gmail.com> wrote:
> On 9/24/14, 10:10 PM, Robert Haas wrote:
>> I think you're making a mountain out of a molehill. I implemented this
>> today in about three hours.
>
> I think you're greatly understating your own efficiency at shift/reducing
> parser mountains down to molehills. Fabien even guessed the LOC size of the
> resulting patch with less than a 9% error. That's some top notch software
> metrics and development work there boys; kudos all around.
Well, I blame Heikki. I used to think that this kind of thing was
really hard, and a few years ago I might have had Fabien's reaction,
but then I saw Heikki bust out a shift/reduce parser for the isolation
tester in no time, so I decided it must not be that hard. So it
proved. I copied all that hard parts from other parts of the
PostgreSQL code base - my references were the isolation tester lexer
and parser, the contrib/seg parser, and the main parser. I couldn't
do it that fast from scratch, not even close, but adapting something
that's already known to work is much easier.
> Let's get this operator support whipped into shape, then we can add the 2 to
> 3 versions of the modulo operator needed to make the major use cases work.
> (There was never going to be just one hacked in with a quick patch that
> satisfied the multiple ways you can do this)
I don't think adding more versions of the modulo operator is the right
way forward: I think we should add ? : for conditionals and some kind
of function thing like abs(x). Or maybe come up with a more
sophisticated rehashing algorithm and expose that directly as hash(x).
That's my whole reason for not wanting to adopt Fabien's approach in
the first place: I was cool with exposing C's modulo operator, but any
other modulo semantics seem like they should be built up from
general-purpose primitives.
Anyway, I think the first thing is that somebody needs to spend some
time testing, polishing, and documenting this patch, before we start
adding to it. I'm hoping someone else will volunteer - other tasks
beckon.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company