Thread: 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 gota 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@tmorelli.com.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@tmorelli.com.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@svana.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@tmorelli.com.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@sss.pgh.pa.us> wrote: > tmorelli@tmorelli.com.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@gmail.com> writes:
> On 3/3/06, Tom Lane <tgl@sss.pgh.pa.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@svana.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@svana.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@sss.pgh.pa.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@sss.pgh.pa.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. +
Tom, sorry, but the address that you wrote tells that there isn´t any patch to apply. Is this patch Itagaki's one? How could I pick it? By the way, don´t worry about the whole idea. It's an experiment that shall be improved in the future, I hope. Best regards, Eduardo Morelli ------------------------------------------------ Tom Lane wrote: >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
etmorelli wrote: > Tom, > > sorry, but the address that you wrote tells that there isn?t any patch to > apply. Is this patch Itagaki's one? How could I pick it? > > By the way, don?t worry about the whole idea. It's an experiment that shall > be improved in the future, I hope. > > Best regards, > > Eduardo Morelli > > > > ------------------------------------------------ > Tom Lane wrote: > > > >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. The patch has moved to: http://momjian.postgresql.org/cgi-bin/pgpatches_hold -- 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. +