You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by traflm <gi...@git.apache.org> on 2018/03/20 03:31:54 UTC
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
GitHub user traflm opened a pull request:
https://github.com/apache/trafodion/pull/1485
[TRAFODION-2999] make UUID non reserved keyword
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/traflm/trafodion TRAFODION-2999
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1485.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1485
----
----
---
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1485#discussion_r178998606
--- Diff: core/sql/common/ComResWords.cpp ---
@@ -395,8 +395,7 @@ const ComResWord ComResWords::resWords_[] = {
ComResWord("USAGE", ANS_|RESWORD_),
ComResWord("USER", ANS_|RESWORD_),
ComResWord("USING", ANS_|RESWORD_),
- ComResWord("UUID", ANS_|RESWORD_),
- ComResWord("VALUE", ANS_|RESWORD_),
+ ComResWord("VALUE", ANS_|RESWORD_|MPWORD_),
--- End diff --
Thanks Dave, let me remove this line.
---
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1485#discussion_r177260101
--- Diff: core/sql/common/ComResWords.cpp ---
@@ -395,8 +395,6 @@ const ComResWord ComResWords::resWords_[] = {
ComResWord("USAGE", ANS_|RESWORD_),
ComResWord("USER", ANS_|RESWORD_),
ComResWord("USING", ANS_|RESWORD_),
- ComResWord("UUID", ANS_|RESWORD_),
- ComResWord("VALUE", ANS_|RESWORD_),
--- End diff --
Yes, @DaveBirdsall , Let me revert the changes to 'VALUE'.
The reason is we found 'VALUE' is also a popular string for column/table names and there is no strict reason to keep it as reserved word.
And we changed it in a commercial branch and test and no side effect. So I want to change it together with 'UUID' ( UUID for sure is very popular in the naming of table/columns )
But for clarity, let me change it, unless you change your mind and allow this :-)
---
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1485#discussion_r177148164
--- Diff: core/sql/common/ComResWords.cpp ---
@@ -395,8 +395,6 @@ const ComResWord ComResWords::resWords_[] = {
ComResWord("USAGE", ANS_|RESWORD_),
ComResWord("USER", ANS_|RESWORD_),
ComResWord("USING", ANS_|RESWORD_),
- ComResWord("UUID", ANS_|RESWORD_),
- ComResWord("VALUE", ANS_|RESWORD_),
--- End diff --
Looks like we accidentally removed VALUE from the reserved words list? This is incorrect.
---
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1485#discussion_r177566197
--- Diff: core/sql/common/ComResWords.cpp ---
@@ -395,8 +395,7 @@ const ComResWord ComResWords::resWords_[] = {
ComResWord("USAGE", ANS_|RESWORD_),
ComResWord("USER", ANS_|RESWORD_),
ComResWord("USING", ANS_|RESWORD_),
- ComResWord("UUID", ANS_|RESWORD_),
- ComResWord("VALUE", ANS_|RESWORD_),
+ ComResWord("VALUE", ANS_|RESWORD_|MPWORD_),
--- End diff --
I'm not sure what "MPWORD_" will do here but it is probably harmless.
---
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1485#discussion_r177269894
--- Diff: core/sql/common/ComResWords.cpp ---
@@ -395,8 +395,6 @@ const ComResWord ComResWords::resWords_[] = {
ComResWord("USAGE", ANS_|RESWORD_),
ComResWord("USER", ANS_|RESWORD_),
ComResWord("USING", ANS_|RESWORD_),
- ComResWord("UUID", ANS_|RESWORD_),
- ComResWord("VALUE", ANS_|RESWORD_),
--- End diff --
It is better to leave VALUE reserved, I think. It is reserved in ISO SQL-2003 (which is the latest copy I have available to me); we may want to implement a feature in the future that uses this keyword. If we do not reserve it, that may be difficult to do.
---
[GitHub] trafodion pull request #1485: [TRAFODION-2999] make UUID non reserved keywor...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1485
---