Re: Problemas with gram.y

Hi,

I'm trying to extend the CREATE INDEX statement with a fillfactor clause. In
Gram.y, I did this:

IndexStmt: CREATE index_opt_unique INDEX index_name ON qualified_name
access_method_clause '(' index_params ')' fillfactor_clause where_clause
{
IndexStmt *n = makeNode(IndexStmt);
n->unique = $2;
n->idxname = $4;
n->relation = $6;
n->accessMethod = $7;
n->indexParams = $9;
n->fillfactor = $11;
n->whereClause = $12;
$$ = (Node *)n;
}

And the clause:

fillfactor_clause:
FILLFACTOR IntegerOnly { $$ = makeInteger($2); }
{ $$ = 0; };

I had to add a new field into IndexStmt (unsigned int fillfactor). Everything
is fine after parsing except that I can't see the integer value. For example,
in transformIndexStmt (analyze.c), I've inspected stmt->fillfactor and I've got
a strange, obviously wrong, value (137616352) after issuing this statement:

create index ix_desc on products(description) fillfactor 7;

Is thre any statement with numeric clauses? The only one that I found was
Alter/Create Sequence, but there is an ugly list there.

Could anyone help me, please?

Thanks a lot!

tmorelli(at)tmorelli(dot)com(dot)br wrote:

>Hi,
>
>I'm trying to extend the CREATE INDEX statement with a fillfactor clause. In
>Gram.y, I did this:
>
>IndexStmt: CREATE index_opt_unique INDEX index_name ON qualified_name
>access_method_clause '(' index_params ')' fillfactor_clause where_clause
> {
> IndexStmt *n = makeNode(IndexStmt);
> n->unique = $2;
> n->idxname = $4;
> n->relation = $6;
> n->accessMethod = $7;
> n->indexParams = $9;
> n->fillfactor = $11;
> n->whereClause = $12;
> $$ = (Node *)n;
> }
>
>And the clause:
>
>fillfactor_clause:
>FILLFACTOR IntegerOnly { $$ = makeInteger($2); }
> { $$ = 0; };
>
>I had to add a new field into IndexStmt (unsigned int fillfactor). Everything
>is fine after parsing except that I can't see the integer value. For example,
>in transformIndexStmt (analyze.c), I've inspected stmt->fillfactor and I've got
> a strange, obviously wrong, value (137616352) after issuing this statement:
>
>create index ix_desc on products(description) fillfactor 7;
>
>Is thre any statement with numeric clauses? The only one that I found was
>Alter/Create Sequence, but there is an ugly list there.
>
>Could anyone help me, please?
>
>Thanks a lot!
>
>
>
>

did you change nodes/copyfuncs.c and nodes/equalfuncs.c as you have to
do when you add new node fields?

cheers

andrew

On Fri, Mar 03, 2006 at 07:14:45PM -0200, tmorelli(at)tmorelli(dot)com(dot)br wrote:
> Hi,
>
> I'm trying to extend the CREATE INDEX statement with a fillfactor clause. In
> Gram.y, I did this:

<snip>

> I had to add a new field into IndexStmt (unsigned int fillfactor). Everything
> is fine after parsing except that I can't see the integer value. For example,
> in transformIndexStmt (analyze.c), I've inspected stmt->fillfactor and I've got
> a strange, obviously wrong, value (137616352) after issuing this statement:

Did you update infunc/outfuncs/copyfuns in the nodes directory? They're
used to copy the nodes and if you forget to update them you get strange
numbers instead.

Hope this helps,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

tmorelli(at)tmorelli(dot)com(dot)br writes:
> I'm trying to extend the CREATE INDEX statement with a fillfactor
> clause.

Um, are you aware that a patch for that was already submitted?
http://momjian.postgresql.org/cgi-bin/pgpatches

I find the whole idea pretty ugly myself.

regards, tom lane

On 3/3/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> tmorelli(at)tmorelli(dot)com(dot)br writes:
> > I'm trying to extend the CREATE INDEX statement with a fillfactor
> > clause.
>
> Um, are you aware that a patch for that was already submitted?
> http://momjian.postgresql.org/cgi-bin/pgpatches
>
> I find the whole idea pretty ugly myself.
>
> regards, tom lane
>

why? if i can ask? you didn't seem upset with that in the thread

--
regards,
Jaime Casanova

"What they (MySQL) lose in usability, they gain back in benchmarks, and that's
all that matters: getting the wrong answer really fast."
Randal L. Schwartz

"Jaime Casanova" <systemguards(at)gmail(dot)com> writes:
> On 3/3/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I find the whole idea pretty ugly myself.

> why? if i can ask? you didn't seem upset with that in the thread

What's bugging me about it is that the proposed syntax wedges a bunch
of index-access-method-specific parameters into what ought to be an
access-method-agnostic syntax; and furthermore does it by adding more
grammar keywords, something we have far too many of already. There are
direct measurable costs to having more keywords, and the approach does
not scale up to allowing other index AMs to have other parameters that
might not bear at all on btree.

I don't object to the concept of providing some way of adjusting index
fill factors, but I'm not at all happy with doing it like this. I'd
like to see a design that has some extensibility to it.

regards, tom lane

On Sat, Mar 04, 2006 at 01:16:55AM -0500, Tom Lane wrote:
> What's bugging me about it is that the proposed syntax wedges a bunch
> of index-access-method-specific parameters into what ought to be an
> access-method-agnostic syntax; and furthermore does it by adding more
> grammar keywords, something we have far too many of already. There are
> direct measurable costs to having more keywords, and the approach does
> not scale up to allowing other index AMs to have other parameters that
> might not bear at all on btree.

I think the current method is based on compatability with other
databases. However, a more generic approach would be to (re)use the
"definition" or "def_list" productions. This would allow any of the
following (examples):

CREATE INDEX foo ON bar (x) WITH (fillfactor = 70, option = blah);
CREATE INDEX foo ON bar (x) WITH fillfactor = 70, option = blah;
CREATE INDEX foo ON bar (x) (fillfactor = 70, option = blah);
CREATE INDEX foo ON bar (x) fillfactor = 70, option = blah;

All without creating any new keywords. You could also place them
before/after the USING clause although there would be some grammer
conflicts there.

Would this be better?
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> On Sat, Mar 04, 2006 at 01:16:55AM -0500, Tom Lane wrote:
>> What's bugging me about it is that the proposed syntax wedges a bunch
>> of index-access-method-specific parameters into what ought to be an
>> access-method-agnostic syntax; and furthermore does it by adding more
>> grammar keywords, something we have far too many of already.

> I think the current method is based on compatability with other
> databases. However, a more generic approach would be to (re)use the
> "definition" or "def_list" productions. This would allow any of the
> following (examples):

> CREATE INDEX foo ON bar (x) WITH (fillfactor = 70, option = blah);
> CREATE INDEX foo ON bar (x) WITH fillfactor = 70, option = blah;
> CREATE INDEX foo ON bar (x) (fillfactor = 70, option = blah);
> CREATE INDEX foo ON bar (x) fillfactor = 70, option = blah;

Yeah, something along this line is what I'd like to see; probably the
first form since that creates the least hazard of foreclosing other
additions to the syntax later. I'm not wedded to WITH as the keyword,
but I think there should be some keyword (ie not choice #3), and I
definitely want parens around the list else it has to be the last thing
in the command.

The main thing that would have to be resolved before you could proceed
very far with this is how does pg_dump extract the index's options?
My guess is that we'd want to extend pg_index entries to store the list
of options. This could probably be done in very much the same way that
ALTER USER SET or ALTER DATABASE SET GUC options are stored, ie, an
array containing name/value pairs.

Another point is that we should take some thought now for the prospect
that the set of options might change across time. I'd be inclined to
argue that if an unknown option appears in a CREATE INDEX command, it's
better to emit a warning and then drop the option, rather than error out
--- otherwise, reloading dumps into newer PG versions will be difficult.
This will impact the details of the index AM API, because the AM will
need a way to filter the options before they get stored in pg_index.
Possibly adding a "precheck" AM function call is the answer here.

Anyway the bottom line is that we need to put in some infrastructure
that can handle multiple index parameters, not a one-off solution that
only handles PCTFREE.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> > CREATE INDEX foo ON bar (x) WITH (fillfactor = 70, option = blah);
>
> Yeah, something along this line is what I'd like to see; probably the
> first form since that creates the least hazard of foreclosing other
> additions to the syntax later.

> Anyway the bottom line is that we need to put in some infrastructure
> that can handle multiple index parameters, not a one-off solution that
> only handles PCTFREE.

Ok, I'll rewrite my PCTFREE patch, and change the word PCTFREE to FILLFACTOR.
There is no benefit of compatibility with Oracle now.

Current all index access methods (btree, hash and gist) have conception of
fillfactors, but static bitmap index or something may not have it.
I see that we should give priority to the design.

---
ITAGAKI Takahiro
NTT Cyber Space Laboratories

Patch withdrawn by author for reworking.

---------------------------------------------------------------------------

ITAGAKI Takahiro wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > > CREATE INDEX foo ON bar (x) WITH (fillfactor = 70, option = blah);
> >
> > Yeah, something along this line is what I'd like to see; probably the
> > first form since that creates the least hazard of foreclosing other
> > additions to the syntax later.
>
> > Anyway the bottom line is that we need to put in some infrastructure
> > that can handle multiple index parameters, not a one-off solution that
> > only handles PCTFREE.
>
> Ok, I'll rewrite my PCTFREE patch, and change the word PCTFREE to FILLFACTOR.
> There is no benefit of compatibility with Oracle now.
>
> Current all index access methods (btree, hash and gist) have conception of
> fillfactors, but static bitmap index or something may not have it.
> I see that we should give priority to the design.
>
> ---
> ITAGAKI Takahiro
> NTT Cyber Space Laboratories
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
>

--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com

+ If your life is a hard drive, Christ can be your backup. +