Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12919 )
Change subject: sentry: sanitize and parse privileges from Sentry ...................................................................... Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/12919/7/src/kudu/master/sentry_authz_provider.cc File src/kudu/master/sentry_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/12919/7/src/kudu/master/sentry_authz_provider.cc@319 PS7, Line 319: return { SentryAuthorizableScope::DATABASE, : SentryAuthorizableScope::TABLE, : SentryAuthorizableScope::COLUMN }; This is going to be created every time this function called, no? Why not to use 'static const ...' for the instances of the containers and return const references to those? I'm not sure in C++11 it's possible to get benefits of constexpr functions that don't have there body in a single line, and this method isn't not even marked with constexpr. http://gerrit.cloudera.org:8080/#/c/12919/7/src/kudu/master/sentry_authz_provider.cc@335 PS7, Line 335: AuthorizableScopesSet Why not to return const reference here? -- To view, visit http://gerrit.cloudera.org:8080/12919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6de6814f99abfbee4f030298b74f21f4e7c729b Gerrit-Change-Number: 12919 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 04 Apr 2019 22:00:26 +0000 Gerrit-HasComments: Yes
