> On May 8, 2018, 8:57 p.m., Na Li wrote: > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java > > Line 31 (original), 33 (patched) > > <https://reviews.apache.org/r/67006/diff/1/?file=2017908#file2017908line33> > > > > Can you add comment saying TPrivilegeEntity.equals() is implemented to > > compare the values of the instances and return true if their values are > > equal? Otherwise, it is dangerous to use object as key if its equals just > > compares the instances. > > kalyan kumar kalvagadda wrote: > Lina, This is hashmap. When put() method is used to store (Key, Value) > pair, HashMap implementation calls hashcode on Key object to calculate a hash > that is used to find a bucket where Entry object will be stored. > > > hashCode for TPrivilegeEntity is implemented.
When two key instances have the same hashcode, the equals() will be called to see if the input key is equal to the key in the hashmap. Comparing the hashcode only determines the bucket to search, not to check if two instances are equal. The default implementation of Object.equals() is to compare the instance address, which is not desirable. That is why I prefer you to add a comment saying the TPrivilegeEntity values are used for comparison, not the instance address. The default equals() method on java.lang.Object compares memory addresses, which means that all objects are different from each other (only two references to the same object will return true ). https://stackoverflow.com/questions/9277813/the-behaviour-of-equals-method-in-java - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67006/#review202684 ----------------------------------------------------------- On May 8, 2018, 1:49 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67006/ > ----------------------------------------------------------- > > (Updated May 8, 2018, 1:49 p.m.) > > > Review request for sentry, Na Li and Sergio Pena. > > > Bugs: SENTRY-2173 > https://issues.apache.org/jira/browse/SENTRY-2173 > > > Repository: sentry > > > Description > ------- > > PrivilegeInfo should be updated to hold the permissions added to the users. > > PrivilegeInfo currently holds roles <=> privileges mapping. This data > structure should be made generic so that it could hold the permissions of > either roles/users on a given object. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java > 3b3a82e8fce7986bd046e96192105952aa6fa796 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java > 2ad74403b279fc2feafda361c92bf6a3d259d0eb > > > Diff: https://reviews.apache.org/r/67006/diff/1/ > > > Testing > ------- > > As there is not functional change. I juat made sure that all the existing > tests pass. > > > Thanks, > > kalyan kumar kalvagadda > >
