Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11693 )
Change subject: Fix thrift operator< implementations ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/11693/2/src/kudu/sentry/thrift_operators-test.cc File src/kudu/sentry/thrift_operators-test.cc: http://gerrit.cloudera.org:8080/#/c/11693/2/src/kudu/sentry/thrift_operators-test.cc@46 PS2, Line 46: > Can you add a test for TSentryAuthorizable and TSentryRole as well? Done http://gerrit.cloudera.org:8080/#/c/11693/2/src/kudu/sentry/thrift_operators.cc File src/kudu/sentry/thrift_operators.cc: http://gerrit.cloudera.org:8080/#/c/11693/2/src/kudu/sentry/thrift_operators.cc@37 PS2, Line 37: other.__isset.field > This will evaluate to false if the field is set for 'this' but unset for 'o Yes that's desired. unset fields compare less than set fields in this implementation, so if the left side (this) is set and the right side (other) is unset, then the left is greater than the right, and thus operator< should return false. -- To view, visit http://gerrit.cloudera.org:8080/11693 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1aacf602c603b05433c357a4a236ba0b9e521392 Gerrit-Change-Number: 11693 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <andrew.w...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 16 Oct 2018 21:25:45 +0000 Gerrit-HasComments: Yes
