Обсуждение: Improving "missing FROM-clause entry" message
I'm thinking about whether we can't improve the message for "missing FROM-clause entry" to somehow account for situations where the table does exist in the query but it's referenced from an improper place, as in bug #2130 (filed a couple hours ago, not yet visible in mail list archives): SELECT ... FROM a, b LEFT JOIN c ON (c.task_id=a.task_id ... This seems to come up often enough in porting MySQL code that we ought to try to deliver a more specific error message for it. We can fairly easily modify warnAutoRange() to check whether the target name exists anywhere in the ParseState's range table. This would not detect forward references, as in SELECT ... FROM b LEFT JOIN c ON (c.task_id=a.task_id ...), a ... but I think this is acceptable since that case isn't going to occur in ported MySQL code. What I'm wondering about is how to word the error message. A minimum-change approach would be to add a HINT, but I'm thinking it'd be better to replace the message entirely: ERROR: reference to table "a" is not allowed from this locationHINT: JOIN clauses cannot refer to tables outside the JOIN Any thoughts about it? Are there any cases where the existing message wording is preferable even though a matching RTE does exist? regards, tom lane
I wrote: > I'm thinking about whether we can't improve the message for "missing > FROM-clause entry" to somehow account for situations where the table > does exist in the query but it's referenced from an improper place, > as in bug #2130 (filed a couple hours ago, not yet visible in mail list > archives): > SELECT ... FROM a, b LEFT JOIN c ON (c.task_id=a.task_id ... On further investigation, this is arguably a regression in 8.1. Every PG release back to 7.2 has responded to this query with NOTICE: adding missing FROM-clause entry for table "a" ERROR: JOIN/ON clause refers to "a", which is not part of JOIN In 8.1, where add_missing_from defaults to false, you get the first line as an ERROR and so the much-more-useful specific message doesn't appear. I think we need to do something about this. regards, tom lane
I wrote: >> I'm thinking about whether we can't improve the message for "missing >> FROM-clause entry" to somehow account for situations where the table >> does exist in the query but it's referenced from an improper place, >> ... > On further investigation, this is arguably a regression in 8.1. > Every PG release back to 7.2 has responded to this query with > NOTICE: adding missing FROM-clause entry for table "a" > ERROR: JOIN/ON clause refers to "a", which is not part of JOIN > In 8.1, where add_missing_from defaults to false, you get the first > line as an ERROR and so the much-more-useful specific message doesn't > appear. I think we need to do something about this. After some thought I've come up with possible ways to handle this. Plan A: when we are about to raise an error in warnAutoRange(), scan the rangetable to see if there are any entries anywhere that could match the specified table name (either by alias or by real table name). If so, don't use the "missing FROM-clause entry" wording, but instead say something like ERROR: invalid reference to FROM-clause entry for table "foo"HINT: The entry cannot be referenced from this part of the query. When the match is by real table name and there's an alias, a better HINT might be HINT: You probably should have used the table alias "bar". since this would do something useful for the perennial mistake select foo.* from foo f; Plan B: when we are about to raise an error in warnAutoRange(), instead just save the error info in the ParseState struct and keep going. If we get to the end of parsing without detecting any other error, report the missing-FROM error. This would let the specific error messageERROR: JOIN/ON clause refers to "a", which is not partof JOIN come out when it's applicable, but not change the behavior otherwise. Plan C: do both. This would give us the most specific error messages possible without major restructuring. A reasonable objection to either Plan A or Plan C is that it will add error strings that are not currently in the translation message files; which wouldn't matter for a HEAD-only patch, but I'd really like to back-patch this into 8.1. Plan B wouldn't change the set of possible messages. If that's not considered a show-stopper, I'd like to go with Plan C. We've certainly got plenty of evidence that this is a confusing error condition, and the more we can do to explain the problem in the error message, the less time will be wasted all around. Comments? Any thoughts about the exact wording of the proposed new messages? regards, tom lane
On Wednesday 04 January 2006 20:37, Tom Lane wrote: > A reasonable objection to either Plan A or Plan C is that it will add > error strings that are not currently in the translation message files; > which wouldn't matter for a HEAD-only patch, but I'd really like to > back-patch this into 8.1. Plan B wouldn't change the set of possible > messages. > No objections here but since I don't use a foreign lang I figure my vote doesn't really matter. I was wondering though if it would be resonable to try and get some language updates into the patch release? -- Robert Treat Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
Robert Treat <xzilla@users.sourceforge.net> writes: > No objections here but since I don't use a foreign lang I figure my vote > doesn't really matter. I was wondering though if it would be resonable to try > and get some language updates into the patch release? With the current re-release plans it'd take awfully quick response from the translators to get that done for 8.1.2. (I'm not even 100% sure I'll get the code patch done for 8.1.2, though I'll try if there's nothing more important on my plate tomorrow.) But it's reasonable to hope that the translations might catch up in 8.1.3 or later. regards, tom lane
Tom Lane wrote: > Robert Treat <xzilla@users.sourceforge.net> writes: > > No objections here but since I don't use a foreign lang I figure my vote > > doesn't really matter. I was wondering though if it would be resonable to try > > and get some language updates into the patch release? > > With the current re-release plans it'd take awfully quick response from > the translators to get that done for 8.1.2. (I'm not even 100% sure > I'll get the code patch done for 8.1.2, though I'll try if there's > nothing more important on my plate tomorrow.) But it's reasonable > to hope that the translations might catch up in 8.1.3 or later. Do we have enough time to test the patch before the minor releases? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > Do we have enough time to test the patch before the minor releases? Sure, it's not like it raises any portability issues. As long as it gives a better error message than before in some common cases, it'll be a step forward, even if we think of further improvements later. regards, tom lane