Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14145 )
Change subject: IMPALA-8579: Ignore trivial alter table/partition events. ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/14145/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/14145/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1032 PS2, Line 1032: Parameters() > Can we refactor this method to static boolean canBeSkipped(paramBefore, par canBeSkipped() needs to compare entire objects and not just the parameters, for example an alter table add column event can have the same transientDdlTime before and after the event but the difference is in the table objects. This means after masking trivial parameters, the parameter maps will be equal but the event should not be skipped. So canBeSkipped() in alter table is comparing Table objects and canBeSkipped() in alter partitions is comparing Partition objects. One common method could be setTrivialParameters() below. If we refactor canBeSkipped() into TableInvalidatingEvent we would have to work with Objects which is kind of ugly. What are your thoughts? http://gerrit.cloudera.org:8080/#/c/14145/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1049 PS2, Line 1049: en. The flag changes > Do you need a separate method for this? Not really. However, this is the piece that could be reused for alterTable and alter partition. But I do not think that is necessary anyway. I will merge this into the above code. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/14145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01a59d5170accc014f76f14eb526d96ddcf61f76 Gerrit-Change-Number: 14145 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Tue, 27 Aug 2019 22:28:55 +0000 Gerrit-HasComments: Yes
