knngist patch support
in [PgSql]
|
Prev: [HACKERS] knngist patch support
Next: pgsql: Remove old-style VACUUM FULL (which wasknown for a little while
From: Tom Lane on 13 Feb 2010 15:58 Robert Haas <robertmhaas(a)gmail.com> writes: > Just to be clear, I was intending this patch, at least, to be applied > now. I actually think there's a good argument that we should do at > least this much for 9.0, namely that now is probably the time when > there are the fewest outstanding patches that will be broken by this. > If we try to apply this for the first 9.1 CommitFest, then (1) it'll > have to be completely redone and (2) it'll force massive rebasing. What I think we should do is not change SearchSysCache for 9.0, but provide the SearchSysCacheN macros as syntactic sugar, and go around and change (at least most of) the call sites to use the macros. This is clearly cleaner source code, and with that method we'll still be ABI-compatible in 9.0 for anybody who doesn't fix their source right away. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 13 Feb 2010 22:40 Robert Haas <robertmhaas(a)gmail.com> writes: > ... > 2. Modify pg_amop by adding a new column amopcategory, probably either > int2 or maybe even just char. > ... > I'm not prepared to endorse doing #3 in core for 9.0, but I wonder if > it would be feasible to think about doing #1 and #2 and putting > something into contrib for #3. No, we are not touching the system catalogs for this in 9.0, much less fooling with any planner logic. If it had been submitted a few months earlier that could have happened, but we are barely 48 hours away from alpha freeze. The only part of this that I think can even be considered at this point is to do a backwards-compatible source code upgrade that will decouple the various call sites from knowledge of exactly how many key columns the syscaches allow. And even that is not for the benefit of this feature; it's mainly to minimize breakage of other patches that will be developed between now and whenever knngist does land. (IOW, I agree with your point that making the call syntax change now is the least painful time to do that.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Jeff Davis on 13 Feb 2010 19:12 On Sat, 2010-02-13 at 13:28 -0500, Tom Lane wrote: > If we didn't already have the plus/minus-for-WINDOW-RANGE example > staring us in the face, I might think that an extensible solution > wasn't needed here ... but we do so I think we really need to allow > for multiple categories in some form. Is this also an opportunity to provide the infrastructure to assign meaning to operators? Related discussion: http://archives.postgresql.org/pgsql-hackers/2009-10/msg01443.php Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 13 Feb 2010 22:26 On Sat, Feb 13, 2010 at 3:58 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas(a)gmail.com> writes: >> Just to be clear, I was intending this patch, at least, to be applied >> now. I actually think there's a good argument that we should do at >> least this much for 9.0, namely that now is probably the time when >> there are the fewest outstanding patches that will be broken by this. >> If we try to apply this for the first 9.1 CommitFest, then (1) it'll >> have to be completely redone and (2) it'll force massive rebasing. > > What I think we should do is not change SearchSysCache for 9.0, > but provide the SearchSysCacheN macros as syntactic sugar, and > go around and change (at least most of) the call sites to use the > macros. This is clearly cleaner source code, and with that method > we'll still be ABI-compatible in 9.0 for anybody who doesn't fix their > source right away. Let me just think out loud for a second about the roadmap for knngist at this point. 1. Add support for 5-key syscaches using either my patch or Teodor's or some third version that may crop up. This could possibly be split into one version that converts everything to using macros (for 9.0) and a second version that actually increases the maximum number of keys from 4 to 5 (for 9.1). 2. Modify pg_amop by adding a new column amopcategory, probably either int2 or maybe even just char. Add this key to both unique indices on pg_amop. Modify CREATE OPERATOR CLASS (and maybe ALTER OPERATOR FAMILY?) to support some new syntax to add "order-by-op" operators to opclasses/families (I'm thinking OPERATOR ORDER <strategy-number> <operator> vs. the usual OPRATOR <strategy-number> <operator>). Also add an amcanorderbyop flag to pg_am. Teach the planner to generate an index scan path when amcanorderbyop is set on the AM and a suitable order-by-op clause is present. 3. Modify GIST to use the infrastructure introduced by #2 for the new <-> operator. I'm not prepared to endorse doing #3 in core for 9.0, but I wonder if it would be feasible to think about doing #1 and #2 and putting something into contrib for #3. The last round of knngist patches had a "planner" patch which basically did #2 (with the problems previously discussed, to which we now seem to have a fairly clean and straightforward solution) and then the "itself" and "proc" patches did #3. The planner patch is actually quite small, so maybe it's not out of the question to think that it might go in, with some suitable fixing up along the lines discussed here and upthread. On the other hand, maybe I'm getting too ambitious. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 13 Feb 2010 15:50
Robert Haas <robertmhaas(a)gmail.com> writes: > On Sat, Feb 13, 2010 at 3:02 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> What would probably be the recommended solution for backwards-compatible >> source code is to convert the actual calls to new style, and then >> provide a block of macro definitions along the lines of >> >> #if CATALOG_VERSION_NO < something >> #define SearchSysCache1(...) SearchSysCache(..., 0, 0, 0) >> #define SearchSysCache2(...) SearchSysCache(..., 0, 0) >> etc >> >> So that seems okay so far. > Where would we put this block of code? *We* wouldn't. This is a solution that might be used by pg_foundry projects for instance. Some people want to distribute loadable modules for which the same source code can be compiled against multiple PG releases. They'd stick those macro definitions, conditionalized as above, into their own source file. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |