----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65642/#review197782 -----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java Lines 225-227 (patched) <https://reviews.apache.org/r/65642/#comment278063> Actual code change to conert the time is good, please update your comment. You current comment "sentry CreateTime is the difference, measured in milliseconds,between the current time and midnight, January 1, 1970 UTC." gives a sence that the timesamp calculated by sentry and hive are different. From what you explained to me they are same, except for the fact that one of them is stored in millisec and another in sec. You can remove this sentence and just say that timstamp is converted from millisec to seconds. - kalyan kumar kalvagadda On Feb. 17, 2018, 12:30 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65642/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2018, 12:30 a.m.) > > > Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio > Pena. > > > Repository: sentry > > > Description > ------- > > sentry CreateTime is the difference, measured in milliseconds, between the > current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. > So need to convert the time. > > The original code just cost the timestamp from long to int without converting > milliseconds to seconds. Therefore, the timestamp value is wrong when > retrieving the privilege from hive. > > The solution is to convert the time correctly. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java > b536283 > > > Diff: https://reviews.apache.org/r/65642/diff/2/ > > > Testing > ------- > > tested manually by stepping into code. I cannot test it in unit test because > Hive returns timestamp as -1L for test mode > > DDLTask.writeGrantInfo > static String writeGrantInfo(List<HivePrivilegeInfo> privileges, boolean > testMode) { > if (privileges != null && !privileges.isEmpty()) { > StringBuilder builder = new StringBuilder(); > Collections.sort(privileges, new Comparator<HivePrivilegeInfo>() { > public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) { > int compare = o1.getObject().compareTo(o2.getObject()); > if (compare == 0) { > compare = o1.getPrincipal().compareTo(o2.getPrincipal()); > } > > if (compare == 0) { > compare = o1.getPrivilege().compareTo(o2.getPrivilege()); > } > > return compare; > } > }); > Iterator var3 = privileges.iterator(); > > while(var3.hasNext()) { > HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next(); > HivePrincipal principal = privilege.getPrincipal(); > HivePrivilegeObject resource = privilege.getObject(); > HivePrincipal grantor = privilege.getGrantorPrincipal(); > appendNonNull(builder, resource.getDbname(), true); > appendNonNull(builder, resource.getObjectName()); > appendNonNull(builder, resource.getPartKeys()); > appendNonNull(builder, resource.getColumns()); > appendNonNull(builder, principal.getName()); > appendNonNull(builder, principal.getType()); > appendNonNull(builder, privilege.getPrivilege().getName()); > appendNonNull(builder, privilege.isGrantOption()); > appendNonNull(builder, testMode ? -1L : > (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not > return real timestamp > appendNonNull(builder, grantor.getName()); > } > > return builder.toString(); > } else { > return ""; > } > } > > > Thanks, > > Na Li > >
