Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14720 )

Change subject: IMPALA-9104: Support retrieval of PK/FK information through 
impala-hs2-server.
......................................................................


Patch Set 9:

(2 comments)

The patch looks good to me. Both the comments below are non-blocking and can be 
done in a follow-up later if required.

http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java@888
PS8, Line 888:           if (!authzChecker_.get().hasAccess(user, 
fkPrivilegeRequest) ||
             :               !authzChecker_.get().hasAccess(user, 
pkPrivilegeRequest)) {
             :             omitList.add(fkName);
             :           }
> There are these cases:
May be I don't understand what you mean by sequence here. Can you give an 
example and add it to the method doc so that its clear.


http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:

http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/MetadataOp.java@393
PS8, Line 393:            }
             :             columns.addAll(fe.getColumns(table, columnPatternMa
> We always match all columns for getPrimaryKeys() request. See line 629. It
Actually, it looks like if the column matcher is NONE, we get the table if its 
loaded. Hence, the columns.addAll() call at the 392 can be executed with the 
matcher is NONE. In such a case, we might end up returning primaryKeys when the 
column matcher is NONE. I thnk that should be okay though. What do you think?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477
Gerrit-Change-Number: 14720
Gerrit-PatchSet: 9
Gerrit-Owner: Anurag Mantripragada <anu...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <anu...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 01:10:59 +0000
Gerrit-HasComments: Yes

Reply via email to