trafodion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Selva Govindarajan <selva.govindara...@esgyn.com>
Subject RE: Reserved/Nonreserved word cruft clean-up
Date Tue, 03 Jul 2018 19:06:57 GMT
It is good to make this change to avoid confusion and to make code consistent and uniform in
all places. However, just want to point out that ComResWords.cpp is still in active use. There
has been changes in this file as recent as Apr via 
https://github.com/apache/trafodion/pull/1485

Thanks
Selva
-----Original Message-----
From: Dave Birdsall <dave.birdsall@esgyn.com> 
Sent: Tuesday, July 3, 2018 11:58 AM
To: dev@trafodion.apache.org
Subject: Reserved/Nonreserved word cruft clean-up

Hi,

Trafodion today has three places where it defines whether a word is a reserved word or a non-reserved
word.

In parser/ParKeyWords.cpp, there is a table of identifiers. Each identifier has a token code
associated with it. If the token code is not IDENTIFIER, it may be a reserved or non-reserved
word. There are flags, RESWORD_, NONRESWORD_ and NONRESTOKEN_ associated with each identifier
in the table, however logic never looks at these flags; they are for documentation purposes
only. (Exception: There is legacy code that looks at them if we are in an "MP" context. That
refers to one of Trafodion's predecessor products. That code is never entered in Trafodion.)

The real work happens in parser/sqlparser.y. If a word's token has a token code other than
IDENTIFIER, then there are a set of productions that determine whether it is reserved or not.
If it is in the nonreserved_word production, then it is a non-reserved word. So apart from
those contexts where the word has a special meaning, it can be used as an ordinary identifier.
Similarly, nonreserved_func_word lists tokens that are acceptable as function names. Each
of these productions allows a word to be an ordinary identifier in particular contexts. If
a word's token is not in any of these productions, then it is truly a reserved word.

There is another module, common/ComResWords.cpp, that seems to duplicate the table in parser/ParKeyWords.cpp,
but it is quite out-of-date. It's also never called. It looks like in the past it was used
by the ToInternalIdentifier function in common/NAString.cpp to return an error if one tried
to use a reserved word as an external-format identifier. But that logic has been changed;
ToInternalIdentifier now calls a stub function that always returns FALSE when checking for
reserved words.

I'd like to simplify this logic, as it is causing problems for folks who are adding new non-reserved
words to Trafodion.

I'd like to delete common/ComResWords.cpp entirely. It's unused, obsolete and redundant. If
we decide we need ToInternalIdentifier to check for reserved words, we can have it use the
table in parser/ParKeyWords.cpp instead.

I'd also like to remove the obsolete "MP context" code in parser/ParKeyWords.cpp (and possibly
other places in the parser).

Does anyone object to my doing this? If I hear no objections by Monday, July 8, I'll file
a JIRA and commence work.

Thanks,

Dave

Mime
View raw message