Re: [HACKERS] [Fwd: Index Advisor]

From: "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>
To: PGSQL-Patches <pgsql-patches(at)postgresql(dot)org>
Cc: "PGSQL Hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [Fwd: Index Advisor]
Date: 2007-01-09 13:07:48
Message-ID: 65937bea0701090507h56534eabs985cac248dab99ab@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

On 1/9/07, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> wrote:
>
>
> I have another idea for making the hooks a bit more cleaner; I will try
> that and run it through you guys later today.
>
>
Please find attached the latest version of the patch. It applies cleanly on
REL8_2_STABLE.

The following restrictions are applied before generating an index candidate
(this is not mentioned in the README):

.) It should be a base relation; We do not generate advisory for a view or
Set Returning Function or something else.
.) It should not be a system table; like pg_class etc.
.) It should not be a temporary table. (May be we can allow these; opinions
please!!)
.) We do not recommend indexes on system columns; like oid, ctid etc.
.) The relation should have at least two pages.
.) The relation should have at least two tuples.

The last two restrictions put the onus on the user to keep the table
ANALYZEd or VACUUMed.

I have moved the calls to index_adviser() from two other places to the
end of planner(). IMO, that is the best place to place a call to the
Adviser; instead of duplicating code in every caller of planner().
index_adviser() makes sure that it doesn't get called recursively.

This change however costs us the loss of ability to append suggested
plan to the existing plan, if being called by the EXPLAIN command. Instead,
it now uses the newly written function explain_getPlanString() in
explain.cto get the string representation of the plan, and then emits
it as elog(
LOG,...).

The only kludge left now is the code enclosed in '#if GLOBAL_CAND_LIST'
in plancat.c. We need to decide whether we need the 'if' part or the 'else'
part! I already see a strong objection to the 'else' option, since it is
very close to the core of the optimizer! Opinions needed.

After adding the CREATE TABLE advise_index(...) script to
src/test/regress/sql/create_table.sql and enabling the GUC in the conf
file, 'make installcheck' runs fine, with a few acceptable diffs. Moreover,
post the 'make' run, we can see there are a few advises for the tables
involved in the test run. Four of them are ob serial columns of
hash-index-testing tables, so they don't make much point; but the rest three
of them are on big tables, and one of them is a multi-column-index
suggestion.

Now that there's just one call to the Index Adviser (from planner()) we can
now move forward in making it a plugin.

Best regards,

--
gurjeet[(dot)singh](at)EnterpriseDB(dot)com
singh(dot)gurjeet(at){ gmail | hotmail | yahoo }.com

Attachment Content-Type Size
pg_index_adviser-REL8_2_STABLE-v23.patch.gz application/x-gzip 32.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2007-01-09 14:21:21 Re: COPY with no WAL, in certain circumstances
Previous Message Andrew Dunstan 2007-01-09 12:56:09 Re: 8.3 pending patch queue

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2007-01-09 14:21:21 Re: COPY with no WAL, in certain circumstances
Previous Message Magnus Hagander 2007-01-09 11:29:55 gendef fixes