Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3711: Check privileges when necessary ......................................................................
Patch Set 4: (7 comments) I think we can fix the issues in the 2nd patch by modifying the Catalog.filterStringsByPattern() functions. Even though it may be correct, I don't think adding all these extra params in the getDbsMetadata is a good approach. It's really confusing and potentially error prone. Let's talk tomorrow. http://gerrit.cloudera.org:8080/#/c/3371/3//COMMIT_MSG Commit Message: PS3, Line 7: Check privileges when necessary I think this patch solves a bigger problem which is not accessing catalog objects that are not needed during HS2 API calls. PS3, Line 9: Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 : : : : I think we can rephrase that a bit. I think you can simply mention that executing HS2 metadata operations in Impala caused unnecessary accesses of catalog objects, thereby resulting in excess authorization checks. Next, briefly describe how this patch addresses this issue. http://gerrit.cloudera.org:8080/#/c/3371/3/fe/src/main/java/com/cloudera/impala/catalog/Catalog.java File fe/src/main/java/com/cloudera/impala/catalog/Catalog.java: PS3, Line 131: /** : * Returns databases that match dbPattern. See filterStringsByPattern for details of : * the pattern match semantics. : * : * dbPattern may be null (and thus matches everything). : */ Can you update the comment? PS3, Line 165: /** : * Returns a list of tables in the supplied database that match : * tablePattern. See filterStringsByPattern for details of the pattern match semantics. : * : * dbName must not be null, but tablePattern may be null (and thus matches : * everything). : * : * Table names are returned unqualified. : */ Update comment. PS3, Line 336: empty string, no strings match. : */ Update comment and everywhere else there is a change in the function signature. http://gerrit.cloudera.org:8080/#/c/3371/3/fe/src/main/java/com/cloudera/impala/service/MetadataOp.java File fe/src/main/java/com/cloudera/impala/service/MetadataOp.java: PS3, Line 239: If functionName is not null, then only function metadata will be returned. : * If tableName is null, then DbsTablesColumns.tableNames and DbsTablesColumns.columns : * will not be populated. : * If columns is null, then DbsTablesColumns.columns will not be populated. Are these comments up to date? PS3, Line 411: schemaNam nit: database names. I don't think it returns the objects. -- 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: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: Yes
