On Thu, May 4, 2017 at 4:21 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> plpgsql has an enum called IdentifierLookup which includes a value >> IDENTIFIER_LOOKUP_EXPR which is declared like this: >> IDENTIFIER_LOOKUP_EXPR /* In SQL expression --- special >> case */ >> It regrettably does not explain what exactly is special about it, and >> AFAICT, neither does any other comment. If I replace every usage of >> IDENTIFIER_LOOKUP_EXPR with IDENTIFIER_LOOKUP_NORMAL, the regression >> tests pass nonetheless. > > AFAICS, the only place where IDENTIFIER_LOOKUP_EXPR acts differently from > IDENTIFIER_LOOKUP_NORMAL is plpgsql_parse_word(), and what it does is to > prevent a lookup in plpgsql's namestack from occurring, so that an > identifier is reported as "not recognized" even if there is a matching > variable. In the two places that currently select IDENTIFIER_LOOKUP_EXPR, > that seems to be only an optimization, because they will act the same > anyway for all the token types that plpgsql_parse_word could return. > I don't remember if there originally was a bigger reason than that. > But it seems reasonable to be able to signal the lookup machinery that > we're in a SQL expression rather than someplace else; in principle that > could have larger implications than it does right now.
Thanks for the explanation. >> The rule, as far as I can tell from reading the code, is that >> IDENTIFIER_LOOKUP_NORMAL looks up words, double-words (e.g. x.y), and >> triple-words (e.g x.y.z), IDENTIFIER_LOOKUP_EXPR looks up only >> double-words and triple-words, and IDENTIFIER_LOOKUP_DECLARE looks up >> nothing. But it's not clear to me exactly what the motivation for >> that is. plpgsql_parse_word says: > > The doubleword and tripleword cases are slightly different: lookup can't > be turned off completely, because we need to create matching RECFIELD > datums for use later. It's still true that we don't especially care what > token is returned, but the side-effect of creating a RECFIELD entry is > important. Possibly we could shave a few more cycles by making the code > know that explicitly, but it didn't seem worth the complexity at the time. The PLPGSQL_DTYPE_* constants are another thing that's not really documented. You've mentioned that we should get rid of PLPGSQL_DTYPE_ROW in favor of, uh, whatever's better than that, but it's not clear to me what that really means, why one way is better than the other way, or what is involved. I'm not really clear on what a PLpgSQL_datum_type is in general, or what any of the specific types actually mean. I'll guess that PLPGSQL_DTYPE_VAR is a variable, but beyond that... > Feel free to improve the comments if this bugs you ... I might try to do that at some point, but I don't think my understanding of all this is clear enough to attempt it at this point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
