From: Teodor Sigaev <teodor@sigaev.ru>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCHES] GIN improvements
Date: Fri, 16 Jan 2009 15:39:36 +0300
Message-ID: <49708008.8070109@sigaev.ru> (raw)
In-Reply-To: <1229910966.2285.76.camel@jdavis>
References: <483FD205.4090904@sigaev.ru>
<486BC501.6060605@sigaev.ru>
<27880.1215810535@sss.pgh.pa.us>
<487A6E0C.3090600@sigaev.ru>
<8993.1216843701@sss.pgh.pa.us>
<4888A594.6010102@sigaev.ru>
<490B07BA.6030109@sigaev.ru>
<492F04D9.5070404@enterprisedb.com>
<4942A137.3000502@sigaev.ru>
<1229910966.2285.76.camel@jdavis>
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@sigaev.ru
WWW: http://www.sigaev.ru/Attachments:
fast_insert_gin-0.20.gz (application/x-tar, 22.9K) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: teodor@sigaev.ru, pgsql@j-davis.com, heikki.linnakangas@enterprisedb.com, tgl@sss.pgh.pa.us, pgsql-hackers@postgresql.org
Subject: Re: [PATCHES] GIN improvements
In-Reply-To: <49708008.8070109@sigaev.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox