Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11040 )

Change subject: hms precheck tool
......................................................................


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG@11
PS7, Line 11: are not unique when compared with Hive's
            : case-insensitive identifier rules
Are there any other funny cases like already existing Kuud tables named like 
'default.MegaTable' when HMS would have 'MegaTable' in its default database?


http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2605
PS7, Line 2605:   KuduSchemaBuilder schema_builder;
              :   
schema_builder.AddColumn("key")->Type(client::KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
              :   KuduSchema schema;
              :   ASSERT_OK(schema_builder.Build(&schema));
I don't think this is relevant if using CreateKuduTable()


http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2626
PS7, Line 2626: ASSERT_FALSE(s.ok());
Maybe, it's worth asserting on specific type of error (RunTimeError, etc.)?


http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2642
PS7, Line 2642: RunActionStdoutNone
wrap into NO_FATALS()


http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2643
PS7, Line 2643: RunActionStdoutNone
ditto


http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2646
PS7, Line 2646: RunActionStdoutNone
ditto


http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2657
PS7, Line 2657: vector<string>({
              :       "A.B!",
              :       "FUZZ",
              :       "a.b",
              :       "a.b!",
              :       "foo.bar",
              :       "foo.bar2",
              :       "foo.bar3",
              :       "fuzz",
              :   }
nit: the expected value comes first -- that way it's easier to read the error 
message (if any).



--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:45:01 +0000
Gerrit-HasComments: Yes

Reply via email to