> On Feb. 21, 2018, 5:13 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 137-140 (patched) > > <https://reviews.apache.org/r/65704/diff/2/?file=1962959#file1962959line137> > > > > What if oldDbName and newDbname are nulls but oldTablename and > > newTableName are not null and different? Should we consider it as a table > > rename? > > > > Would the following conditions be simpler? > > > > if (oldTableName != null && newTableName != null) { > > if (oldTableName.equalsIgnoreCase(newTableName) { > > return true; > > } > > } > > > > if (oldDbName != null && newDbname != null) { > > if (oldDbName.equalsIgnoreCase(newDbname) { > > return true; > > } > > } > > Na Li wrote: > I am following the logic in Notification.processAlterTable. We need to > keep the same logic in this function and Notification.processAlterTable
If we need to keep the same logic, would make sense to have a shared method that checks if a table is renamed? Also, the Notification method checks for path changes as well (not just rename), so does it mean we sync with these changes as well? Notice that locations are useful for HDFS sync only not grant/revoke privileges. - Sergio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/#review197936 ----------------------------------------------------------- On Feb. 21, 2018, 4:52 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65704/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2018, 4:52 p.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Sergio Pena. > > > Repository: sentry > > > Description > ------- > > wait for HMS sync at alter table, which including table rename and changing > table columns > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > 24d7763 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 4cd00e6 > > > Diff: https://reviews.apache.org/r/65704/diff/2/ > > > Testing > ------- > > unit tests succeeded > > > Thanks, > > Na Li > >
