Re: patch for geqo tweaks

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nathan Wagner <nw+pg(at)hydaspes(dot)if(dot)org>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch for geqo tweaks
Date: 2015-11-04 17:51:52
Message-ID: 8011.1446659512@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Nathan Wagner <nw+pg(at)hydaspes(dot)if(dot)org> writes:
> I have two patches to make the geqo initialization and mutation
> slightly better.

> The first adjusts the mutation swaps to avoid having to re-pick
> ties. The second changes the initialization and shuffling algorithm
> for the gene array to use an in-place Fisher-Yates shuffling
> algorithm.

I took a quick look at this.

I'm not very impressed with the first patch: it might save a few
geqo_randint() calls, but it seems to do so at the price of making the
swap choices less random --- for instance it sure looks to me like the
last array element is now less likely to participate in swaps than
other elements. Unless you can prove that actually the swapping is
still unbiased, I'm inclined to reject this part.

As for the second part, I had to look up Fisher-Yates ;-) but after
having read Wikipedia's entry about it I think this is a good change.
The code's shorter and more efficient, and it should mathematically
provide an equally-unbiased initial shuffle. It could do with a better
comment, and I'd be inclined to handle the first element outside the loop
rather than uselessly computing geqo_randint(0,0), but those are trivial
changes.

Having said that, though, I believe that it's also probably a
*different* initial shuffle, which may well mean that GEQO gives
different plans in some cases. That doesn't bother me as long as
we only make the change in HEAD, but does anyone want to complain?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-11-04 17:56:29 Re: fortnight interval support
Previous Message Simon Riggs 2015-11-04 17:31:37 Re: Bitmap index scans use of filters on available columns