Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/17794 )
Change subject: IMPALA-10822: Allow multiple group returns for LDAP Search ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/17794/4/be/src/util/ldap-search-bind.cc File be/src/util/ldap-search-bind.cc: http://gerrit.cloudera.org:8080/#/c/17794/4/be/src/util/ldap-search-bind.cc@106 PS4, Line 106: " entries " : << "have been found."; We could add that we expected a unique result. I'm not familiar with this part of the code so it may be obvious for the users but for me it would be strange at first that the search failed although there are several results found. http://gerrit.cloudera.org:8080/#/c/17794/4/be/src/util/ldap-search-bind.cc@149 PS4, Line 149: " entries have been found."; Same as on L107. http://gerrit.cloudera.org:8080/#/c/17794/4/be/src/util/ldap-simple-bind.cc File be/src/util/ldap-simple-bind.cc: http://gerrit.cloudera.org:8080/#/c/17794/4/be/src/util/ldap-simple-bind.cc@169 PS4, Line 169: (auto & For me explicitly naming the type (etc. const string&) is cleaner and more readable but I realise that it is hugely just personal preference. -- To view, visit http://gerrit.cloudera.org:8080/17794 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27eaf717985207e36d4fa47a2af757d06544474e Gerrit-Change-Number: 17794 Gerrit-PatchSet: 4 Gerrit-Owner: Tamas Mate <tm...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 24 Aug 2021 10:53:28 +0000 Gerrit-HasComments: Yes
