Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19252 )

Change subject: IMPALA-11728 Set fallback database for functions
......................................................................


Patch Set 7:

(8 comments)

The code looks good to me, some comments to make commit message clearer.

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

http://gerrit.cloudera.org:8080/#/c/19252/7//COMMIT_MSG@7
PS7, Line 7:  Set f
nit: missing :


http://gerrit.cloudera.org:8080/#/c/19252/7//COMMIT_MSG@10
PS7, Line 10: When using the function in another database, it will throw
Could add that the function is used without specifying the database. When the 
function name is fully qualified then this query option has no effect.


http://gerrit.cloudera.org:8080/#/c/19252/7//COMMIT_MSG@11
PS7, Line 11:  unknown for database
Do you mean "the function is unknown in the current database"?


http://gerrit.cloudera.org:8080/#/c/19252/7//COMMIT_MSG@13
PS7, Line 13: Add a fallback database for functions as query option. It works on
Can you add the exact name of the query option here or in the title?


http://gerrit.cloudera.org:8080/#/c/19252/7//COMMIT_MSG@15
PS7, Line 15:
Could add the exact precedence of the functions:
1. builtins
2. fallback db
3. function in current db


http://gerrit.cloudera.org:8080/#/c/19252/7//COMMIT_MSG@16
PS7, Line 16: Testing:
Some example in the commit message could make this feature very clear, e.g. 
creating a function, setting the query option, then calling the function.


http://gerrit.cloudera.org:8080/#/c/19252/7/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/19252/7/tests/query_test/test_udfs.py@649
PS7, Line 649:     drop_function_stmt = "drop function if exists 
`{0}`.fn()".format(unique_database)
This is not needed, as unique_database always starts empty.


http://gerrit.cloudera.org:8080/#/c/19252/7/tests/query_test/test_udfs.py@677
PS7, Line 677:     self.client.execute(drop_function_stmt)
No need to cleanup, a DROP DATABASE CASCADE will be ran after the test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37b7e126718fea1c50723cacbaed898b20bb55e5
Gerrit-Change-Number: 19252
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Xiaoqing Gao <[email protected]>
Gerrit-Comment-Date: Tue, 22 Nov 2022 15:56:02 +0000
Gerrit-HasComments: Yes

Reply via email to