Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9800 )
Change subject: IMPALA-6724: Incorrect exception handling in create/drop function statements ...................................................................... Patch Set 5: (10 comments) http://gerrit.cloudera.org:8080/#/c/9800/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9800/5//COMMIT_MSG@10 PS5, Line 10: reestriction nit: spelling http://gerrit.cloudera.org:8080/#/c/9800/5//COMMIT_MSG@11 PS5, Line 11: to to -> a http://gerrit.cloudera.org:8080/#/c/9800/5//COMMIT_MSG@13 PS5, Line 13: whether when? http://gerrit.cloudera.org:8080/#/c/9800/5//COMMIT_MSG@55 PS5, Line 55: lt-in function ex in the context of "use <db>"? the corresponding create entry succeeded in making something. its asymmetric that it cannot be queried with the same settings. http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/main/java/org/apache/impala/analysis/FunctionName.java File fe/src/main/java/org/apache/impala/analysis/FunctionName.java: http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/main/java/org/apache/impala/analysis/FunctionName.java@95 PS5, Line 95: * Some rules regarding how the database name set when analyze is called: ... is set ... http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/main/java/org/apache/impala/analysis/FunctionName.java@103 PS5, Line 103: the nit: a http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/main/java/org/apache/impala/analysis/FunctionName.java@104 PS5, Line 104: * set the database name to _impala_builtins. just checking, so if I have a session like this: use fooDb; select sin(42); we resolve "sin" to the builtin db before resolving to the current session db? that seems like a surprising scoping rule. perhaps I've missed something. http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@3115 PS5, Line 3115: Error("drop function sin(d does this round trip with the create in L3106? I assume it can if specified as default.sin? perhaps add an e2e test to confirm. unless I've misread it, a drop where there is no FQN and the builtin database is not specified, if the fn exists in the default db (in this case), then it will be found and dropped? http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java: http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2330 PS5, Line 2330: estUdfs.so' symbol='NoArgs'", : "User '%s' does not hav you'd get this error even for a fn that was not a built-in? http://gerrit.cloudera.org:8080/#/c/9800/5/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2343 PS5, Line 2343: nction sin(double) same question as previous comment -- To view, visit http://gerrit.cloudera.org:8080/9800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa Gerrit-Change-Number: 9800 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 27 Mar 2018 17:00:05 +0000 Gerrit-HasComments: Yes
