Shriya Gupta has posted comments on this change. (
http://gerrit.cloudera.org:8080/10656 )
Change subject: Code update for KUDU-2459
......................................................................
Patch Set 5:
Hey Adar,
Thank you for the code review. From our discussion offline, I have the
addressed the improvement suggestions as below. Please let me know if this is
acceptable.
1. Decompose the table name validation logic into a separate function and add a
unit test that exercises it with both valid and invalid names. I don't think we
have a dedicated "Impala utility" module, but you can start one, perhaps
util/impala-util.{cc,h}?
-With HMS Integration landing soon, this work is not needed for current
functionality improvement
2. Extend the function to handle _all_ of the Impala identifier requirements.
For example, it'd be nice to account for Impala reserved words as well.
-This functionality is not needed as the tablename is placed in double-quotes,
an exception that Impala allows for identifier names.
3. Use a placeholder that is more obviously a placeholder.
-I have replaced the placeholder with '<placeholder>' thereby forcing the user
to be mindful about updating it.
Thanks,
Shriya
--
To view, visit http://gerrit.cloudera.org:8080/10656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9e5242318589452a0507d3973b9be3e6a186a69
Gerrit-Change-Number: 10656
Gerrit-PatchSet: 5
Gerrit-Owner: Shriya Gupta <shriya...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Shriya Gupta <shriya...@gmail.com>
Gerrit-Reviewer: Thomas Marshall <thomasmarsh...@cmu.edu>
Gerrit-Comment-Date: Tue, 31 Jul 2018 16:47:39 +0000
Gerrit-HasComments: No
