Обсуждение: pgsql: Allow GIN's extractQuery method to signal that nothing can

Log Message:
-----------
Allow GIN's extractQuery method to signal that nothing can satisfy the query.
In this case extractQuery should returns -1 as nentries. This changes
prototype of extractQuery method to use int32* instead of uint32* for
nentries argument.
Based on that gincostestimate may see two corner cases: nothing will be found
or seqscan should be used.

Per proposal at http://archives.postgresql.org/pgsql-hackers/2007-01/msg01581.php

PS tsearch_core patch should be sightly modified to support changes, but I'm
waiting a verdict about reviewing of tsearch_core patch.

Modified Files:
--------------
    pgsql/contrib/intarray:
        _int_gin.c (r1.3 -> r1.4)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/contrib/intarray/_int_gin.c.diff?r1=1.3&r2=1.4)
    pgsql/contrib/tsearch2:
        ginidx.c (r1.3 -> r1.4)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/contrib/tsearch2/ginidx.c.diff?r1=1.3&r2=1.4)
    pgsql/doc/src/sgml:
        gin.sgml (r2.7 -> r2.8)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/gin.sgml.diff?r1=2.7&r2=2.8)
    pgsql/src/backend/access/gin:
        ginarrayproc.c (r1.8 -> r1.9)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/ginarrayproc.c.diff?r1=1.8&r2=1.9)
        ginbulk.c (r1.7 -> r1.8)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/ginbulk.c.diff?r1=1.7&r2=1.8)
        ginget.c (r1.5 -> r1.6)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/ginget.c.diff?r1=1.5&r2=1.6)
        gininsert.c (r1.6 -> r1.7)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/gininsert.c.diff?r1=1.6&r2=1.7)
        ginscan.c (r1.8 -> r1.9)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/ginscan.c.diff?r1=1.8&r2=1.9)
        ginutil.c (r1.9 -> r1.10)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/ginutil.c.diff?r1=1.9&r2=1.10)
    pgsql/src/backend/utils/adt:
        selfuncs.c (r1.223 -> r1.224)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/selfuncs.c.diff?r1=1.223&r2=1.224)
    pgsql/src/include/access:
        gin.h (r1.9 -> r1.10)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/access/gin.h.diff?r1=1.9&r2=1.10)

teodor@postgresql.org (Teodor Sigaev) writes:
> Log Message:
> -----------
> Allow GIN's extractQuery method to signal that nothing can satisfy the query.
> In this case extractQuery should returns -1 as nentries. This changes
> prototype of extractQuery method to use int32* instead of uint32* for
> nentries argument.
> Based on that gincostestimate may see two corner cases: nothing will be found
> or seqscan should be used.

This patch contains some pretty ugly coding in gincostestimate that
thinks it must go out of its way to prevent full indexscans.  But
(a) it does not work (you cannot positively guarantee a plan will not
be chosen just by setting its cost high) and (b) it is unnecessary.
pg_am.amoptionalkey = false is the right way, and you already have that.
So please remove the klugery in gincostestimate.

            regards, tom lane

> (a) it does not work (you cannot positively guarantee a plan will not
> be chosen just by setting its cost high) and (b) it is unnecessary.
> pg_am.amoptionalkey = false is the right way, and you already have that.

 From docs:
 >>>>>
When amcanmulticol is false, amoptionalkey essentially says whether the access
method allows full-index scans without any restriction clause.
<<<<<

amcanmulticol doesn't resolve issue, because restriction clause might present,
but it might have not any actual values ( void tsquery, void array ) and
semantic meaning of void query might be a 'any tuple matches'. Suggested
gincostestimation's patch allows to prevent from index in some situations, I
imagine, that isn't a good solution for two reason:
  - high cost doesn't guarantee an indexscan will be choosen
  - Doesn't work with anything except Const query
But I didn't  find a better place to insert it to resolve first point.

Sorry, but now I have no idea how to produce GIN's fullindex scan without
disaster performance gap. If you insist then I'll remove whole new code in
gincostestimate...


--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Teodor Sigaev <teodor@sigaev.ru> writes:
> amcanmulticol doesn't resolve issue, because restriction clause might present,
> but it might have not any actual values ( void tsquery, void array ) and
> semantic meaning of void query might be a 'any tuple matches'. Suggested
> gincostestimation's patch allows to prevent from index in some situations, I
> imagine, that isn't a good solution for two reason:
>   - high cost doesn't guarantee an indexscan will be choosen
>   - Doesn't work with anything except Const query
> But I didn't  find a better place to insert it to resolve first point.

If you're going to support operators that could allow every tuple to
match, then I think you had better find a way to allow a full index scan
within GIN.  Because the above does not fix the problem, it's only a
very crude band-aid; you *cannot* assume that you'll always have Consts
to look at.

            regards, tom lane

> If you're going to support operators that could allow every tuple to

Right now it's a 'contains', 'contained by' operations for arrays.

> match, then I think you had better find a way to allow a full index scan
> within GIN.  Because the above does not fix the problem, it's only a
> very crude band-aid; you *cannot* assume that you'll always have Consts
> to look at.

Ok, I'll remove that.

--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/