Re: [PATCHES] hint infrastructure setup (v3)
Fabien COELHO <[EMAIL PROTECTED]> writes: > I can resubmit a new patch that would provide the needed infrastructure > AND some hints on some commands as a proof of concept of what can be > achieved. I quite agree that you shouldn't do a complete implementation when it's not clear if we'll accept it or not. What I'd like to see is a demo, basically. How about one example of each of several different kinds of hints? We need to get a feeling for what can be accomplished and how messy the grammar would get. BTW, it seems like you are putting in a lot of infrastructure to recreate externally the parse-stack state that bison has internally. Maybe an interesting alternative would be to look at touching that state directly. I realize that bison doesn't consider that part of their official API, but bison is a stable tool and seems unlikely to change much in the future. We could probably get away with it... regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] hint infrastructure setup (v3)
Dear Tom, > > Well, the current status of the infrastructure is that there is no hint;-) > > Ah, now I remember why I was waiting to review that stuff: I was expecting > you to come out with a version that actually did some things :-( Well, if you wait for something from me, it is better to tell me directly. I was waiting for anything to happen to the patch (accept, discuss or reject) before submitting anything else. > What you've got at the moment is a patch that significantly uglifies the > grammar without actually doing anything useful, or even clearly pointing > the way to what else will need to happen before it does do something > useful. So it's difficult to form any reasoned judgment about whether > we want to commit to following this path or not. I can see your point. The reasonnable way out of the deadlock that I can suggest is the following: I can resubmit a new patch that would provide the needed infrastructure AND some hints on some commands as a proof of concept of what can be achieved. Then you can decide whether it is worth having this kind of thing on all commands, or not. If not, I won't have passed 3 week-ends in the parser instead if my garden for nothing. If you are okay then you apply, and I'll submit some new patches later on, one command at a time, and when I have time. Does this sound reasonnable enough? -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] hint infrastructure setup (v3)
Fabien COELHO <[EMAIL PROTECTED]> writes: >> Also, what is typical output for a hint? Can you show one? > Well, the current status of the infrastructure is that there is no hint;-) Ah, now I remember why I was waiting to review that stuff: I was expecting you to come out with a version that actually did some things :-( What you've got at the moment is a patch that significantly uglifies the grammar without actually doing anything useful, or even clearly pointing the way to what else will need to happen before it does do something useful. So it's difficult to form any reasoned judgment about whether we want to commit to following this path or not. As I said back at the beginning, I'm unconvinced that this path leads to anything useful --- it seems like an extremely painful way of accomplishing less than what a simple change to auto-show the command's syntax summary could do. To change my mind, you've got to show me some concrete results of hints that are more useful than "\h " is. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] hint infrastructure setup (v3)
Dear Bruce,
> Why did all the tags have to be renamed:
>
> + cmdGRANT: GRANT {noH;};
that's a good question.
In order to add hints, I want to attach them to the states of the parser
automaton. So as to do that, I need I put a name on every state, so I need
to refactor the prefix that lead to a state, at give it a name.
Otherwise, I would have :
xxx: GRANT {noH;} ALL
| GRANT {noH;} CREATE
;
(1) I would have to put the "after GRANT" hint twice, one after each
GRANT occurence.
(2) this would result in shift/reduce conflicts, as the parser does not
know which hint should be put. Well, it is the same code in both case,
but yacc does not look at that.
Also, I need to stop hints, otherwise the last 'pushed' hint would be
shown on any error later.
> Also, what is typical output for a hint? Can you show one?
Well, the current status of the infrastructure is that there is no hint;-)
The only hint with the patch is if you do not put an SQL command.
It may look like :
psql> creat table foo();
ERROR: syntax error at or near "creat" at character 1
creat table foo();
^
HINT: sql command such as SELECT or CREATE... ?
All other syntax errors result in no hint being displayed, thanks to
all added noH calls I put.
Also, As far as I remember, I put a new option to activate these hints.
Hints are disactivated by default. This is because some people do not
like advices and may want to turn them on or off.
--
Fabien Coelho - [EMAIL PROTECTED]
---(end of broadcast)---
TIP 8: explain analyze is your friend
Re: [PATCHES] hint infrastructure setup (v3)
Fabien COELHO wrote:
>
> Dear patchers,
>
> Well, as the discussion rages over my previous patch submissions, I've
> time to improve the code;-)
>
> I finally figured out that there is 2 errhint functions (elog.c vs
> ipc_text.c), and the one I'm calling is the fist one, so I need to put a
> format. The second appears to ignore it's arguments after the first.
>
> Anyway, please consider the following patch for inclusion to current
> head. It validates for me. I need it to be able to go on.
Why did all the tags have to be renamed:
+ cmdGRANT: GRANT {noH;};
Also, what is typical output for a hint? Can you show one?
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
