Prev: [HACKERS] rbtree code breaks GIN's adherence to maintenance_work_mem
Next: review: xml_is_well_formed
From: Stephen Frost on 31 Jul 2010 07:08
* Tom Lane (tgl(a)sss.pgh.pa.us) wrote:
> After a bit of study of the code, it appears to me that it's not too
> difficult to fix: we just have to use the expression's result type
> rather than the index column's atttypid in the subsequent processing.
> ANALYZE never actually looks at the index column contents (indeed
> cannot, since our index AMs provide no API for extracting the contents),

I don't think it'll be an issue, but just in case- is there any reason
this will cause trouble for the possible index-only quals/scans work?

> So that seems to make it not practical to back-patch.

But we could get it into 9.0..

> I thought of an ugly hack that would avoid an API/ABI break: since
> VacAttrStats.attr is a copy of the pg_attribute data, we could scribble
> on it, changing atttypid, attlen, attbyval, etc to match the index
> expression result type. This seems pretty grotty, but it would allow
> the fix to be back-patched into existing branches.

Yeah, that's rather nasty. :/

> Comments? I'm not sure which way to jump here.

For my 2c- let's get it fixed for 9.0 cleanly and encourage people who
run into this to upgrade to that once it's released. Perhaps I've
missed it, but I don't recall seeing many complaints about this.

Thanks,

Stephen
From: Stephen Frost on 31 Jul 2010 21:16
* Kevin Grittner (Kevin.Grittner(a)wicourts.gov) wrote:
> Robert Haas 07/31/10 12:33 PM >>>
> > Tom Lane wrote:
> >> Failing to store stats isn't a bug?
> >
> > Well, it kind of sounds more like you're removing a known
> > limitation than fixing a bug.
>
> It's operating as designed and documented. There is room for
> enhancement, but the only thing which could possibly justify this as
> 9.0 material is if there was a demonstrated performance regression in
> 9.0 for which this was the safest cure.

I have to disagree with this, to be honest. The fact that we've
documented what is completely unexpected and frustrating behaviour
doesn't mean we get to say it's not a bug. Not collecting stats, at
all, is a pretty bad bug, in my view. Stats are an important part of
the system which needs to work at least decently. Perhaps before it was
pretty rare that we'd have the situation described (before we brought in
tsearch2), but it's not any longer and we need to support it as we would
the other types. The only reason I'm against backpatching it to the
beginning is that it's either an ABI change or some rather grotty code,
and even then it wouldn't be hard to push me to accepting the grotty
code if we make the cleaner change for 9.0 and going forward, especially
as we have people in the wild being affected by it.

Certain other databases have done a very good job of documenting their
bugs and in some cases even calling them features. I'd rather we not go
down that path. I don't see the lack of stats collecting to be a simple
'limitation'.

Thanks,

Stephen
 | 
Pages: 1
Prev: [HACKERS] rbtree code breaks GIN's adherence to maintenance_work_mem
Next: review: xml_is_well_formed