On Thu, 5 Nov 2020 18:16:04 -0300 Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> On 2020-Nov-05, Tomas Vondra wrote: > > > On 11/5/20 6:17 PM, Alvaro Herrera wrote: > > > > There are recent changes in vacuum for temp tables (commit > > > 94bc27b57680?) that would maybe make this stable enough, without > > > having to have the CIC there. At least, I tried it locally a few > > > times and it appears to work well. This won't work for older > > > releases though, just master. This is patch 0001 attached here. > > > > IIUC you're suggesting to use a temporary table in the test? > > Unfortunately, this does not work on older releases, and IMHO the > > test should be backpatched too. IMHO the CIC "hack" is acceptable, > > unless there's a better solution that I'm not aware of. > > Oh, sure, the CIC hack is acceptable for the older branches. I'm just > saying that you can use a temp table everywhere, and keep CIC in the > old branches and no CIC in master. > > > This got me thinking though - wouldn't it be better to handle too > > large values by treating the range as "degenerate" (i.e. store NULL > > and consider it as matching all queries), instead of failing the > > CREATE INDEX or DML? I find the current behavior rather annoying, > > because it depends on the other rows in the page range, not just on > > the one row the user deals with. Perhaps this might even be > > considered an information leak about the other data. Of course, not > > something this patch should deal with. > > Hmm. Regarding text I remember thinking we could just truncate values > (as we do for LIKE, as I recall). I suppose that strategy would work > even for bytea. > > > > > > +/* > > > > + * This enables de-toasting of index entries. Needed until > > > > VACUUM is > > > > + * smart enough to rebuild indexes from scratch. > > > > + */ > > > > > > ... because, surely, we're now never working on having VACUUM > > > rebuild indexes from scratch. In fact, I wonder if we need the > > > #define at all. I propose to remove all those #ifdef lines in > > > your patch. > > > > That's a verbatim copy of a comment from indextuple.c. IMHO we > > should keep it the same in both places. > > Sure, if you want to. > > > > The fix looks good to me. I just added a comment in 0003. > > > > Thanks. Any opinions on fixing this in existing clusters? Any > > better ideas than just giving users the SQL query to list > > possibly-affected indexes? > > None here. > OK, pushed and backpatched all the way back to 9.5. I decided not to use the temporary table - I'd still need to use the CIC trick on older releases, and there were enough differences already. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
