Re: [PATCHES] GIN improvements

From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: Jeff Davis <pgsql(at)j-davis(dot)com>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCHES] GIN improvements
Date: 2009-01-16 12:39:36
Message-ID: 49708008.8070109@sigaev.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

New version. Changes:
- synced with current CVS
- added all your changes
- autovacuum will run if fast update mode is turned on and
trigger of fresh tuple is fired
- gincostestimate now tries to calculate cost of scan of pending pages.
gincostestimate set disable_cost if it believe that tidbitmap will
become lossy. So, tidbitmap has new method - estimation of
maximum number of tuples with guaranteed non-lossy mode.

> START_CRIT_SECTION();
> ...
> l = PageAddItem(...);
> if (l == InvalidOffsetNumber)
> elog(ERROR, "failed to add item to index page in \"%s\"",
> RelationGetRelationName(index));
>
> It's no use using ERROR, because it will turn into PANIC, which is
I did that similar to other GIN/GiST places. BTW, BTree directly emits PANIC if
PageAddItem fails

>
> 4. Heikki mentioned:
> http://archives.postgresql.org/pgsql-hackers/2008-11/msg01832.php
>
> "To make things worse, a query will fail if all the matching
> fast-inserted tuples don't fit in the non-lossy tid bitmap."
>
> That issue still remains, correct? Is there a resolution to that?

Now gincostestimate can forbid index scan by disable_cost (see Changes). Of
course, it doesn't prevent failure in case of large update (for example), but it
prevents in most cases. BTW, because of sequential scan of pending list cost of
scan grows up fast and index scan becomes non-optimal.

>
> 5. I attached a newer version merged with HEAD.
Thank you

> 6. You defined:
>
> #define GinPageHasFullRow(page) ( GinPageGetOpaque(page)->flags &
> GIN_LIST_FULLROW )
>
Fixed

> 7. I don't understand this chunk of code:
>
> How can (!ItemPointerEquals(&pos->item, &item)) ever happen?
>
> And how can (scanGetCandidate(scan, pos) == false) ever happen? Should
> that be an Assert() instead?
>
> If those can happen during normal operation, then we need a better error
> message there.

It should be assert, but assert enabled and disabled code will be different :(.
In both cases, scanGetCandidate() should be called, but in assert enabled code
we need to check return value and pos->item.

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/

Attachment Content-Type Size
fast_insert_gin-0.20.gz application/x-tar 22.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2009-01-16 13:40:32 Re: libpq WSACleanup is not needed
Previous Message Magnus Hagander 2009-01-16 12:24:20 Re: FWD: Re: Updated backslash consistency patch

Browse pgsql-patches by date

  From Date Subject
Next Message Jeff Davis 2009-01-19 07:08:25 Re: [PATCHES] GIN improvements
Previous Message Hiroshi Saito 2009-01-11 07:23:32 Re: Solve a problem of LC_TIME of windows.