[Impala-CR](cdh5-trunk) IMPALA-3711: Only check privilege for databases in getSchemas()
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3711: Only check privilege for databases in getSchemas() .. Patch Set 1: As we discussed offline, the implementation is quite confusing. We seem to be evaluating the search patterns in three different places (MetadataOp, Frontend and Catalog), depending on the catalog object type. Let's try to simplify the code and have consistent ways of getting catalog objects and checking their privileges. Thanks for clearing our mess :) -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3711: Only check privilege for databases in getSchemas()
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3711: Only check privilege for databases in getSchemas() .. Patch Set 1: The behavior of getDbsMetadata() is not changed right? -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3711: Only check privilege for databases in getSchemas()
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3711: Only check privilege for databases in getSchemas() .. Patch Set 1: > I don't think these changes are correct. I believe the HS2 API > specifies that getSchemas(), getTables(), etc, should be able to > accept a search pattern as an input parameter. You seem to remove > that functionality altogether, but this is not what we want. Could you elaborate? Our currently API for getSchamas() is: getSchemas(Frontend fe, String catalogName, String schemaName, User user) It does not take table name or column name at all as input parameter. My patch only changes the inner behavior of getSchemas(), which I believe is invisible.? right? -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3711: Only check privilege for databases in getSchemas()
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3711: Only check privilege for databases in getSchemas() .. Patch Set 1: I don't think these changes are correct. I believe the HS2 API specifies that getSchemas(), getTables(), etc, should be able to accept a search pattern as an input parameter. You seem to remove that functionality altogether, but this is not what we want. -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3711: Only check privilege for databases in getSchemas()
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3711: Only check privilege for databases in getSchemas() .. Patch Set 1: need some polish. tested by backporting this to a 5.4.10 cluster, which has many roles/privileges/tables, etc.. getScehma() performance increases at least 50x+. I did not count in detail, got to go.. will do on monday. -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3711: Only check privilege for databases in getSchemas()
Huaisi Xu has uploaded a new change for review. http://gerrit.cloudera.org:8080/3371 Change subject: IMPALA-3711: Only check privilege for databases in getSchemas() .. IMPALA-3711: Only check privilege for databases in getSchemas() Previously getSchemas() checks privileges for all databases and tables, which can lead to bad performance. Given that it only returns databases, we are save just change its privilege checking behavior to only check databases. Also apply filter on database names before checking. Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 --- M fe/src/main/java/com/cloudera/impala/service/MetadataOp.java 1 file changed, 5 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/71/3371/1 -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu
