Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17308 )

Change subject: IMPALA-10502: Handle CREATE/DROP events correctly
......................................................................


Patch Set 12:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/17308/12/fe/src/main/java/org/apache/impala/catalog/events/NoOpEventProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/NoOpEventProcessor.java:

http://gerrit.cloudera.org:8080/#/c/17308/12/fe/src/main/java/org/apache/impala/catalog/events/NoOpEventProcessor.java@20
PS12, Line 20: import org.apache.hadoop.hive.metastore.api.NotificationEvent;
             : import org.apache.impala.catalog.CatalogException;
             : import 
org.apache.impala.catalog.events.MetastoreEvents.IgnoredEvent;
             : import 
org.apache.impala.catalog.events.MetastoreEvents.MetastoreEvent;
> nit: seems unused imports
Done


http://gerrit.cloudera.org:8080/#/c/17308/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/17308/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@693
PS12, Line 693: debug
> Do we change the log level somewhere? I see these debug loggings when runni
Not sure I understand what you mean? Do you mean you don't see this logs?


http://gerrit.cloudera.org:8080/#/c/17308/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@804
PS12, Line 804:
> nit: don't need space
Done


http://gerrit.cloudera.org:8080/#/c/17308/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@805
PS12, Line 805:
> nit: don't need space
Done


http://gerrit.cloudera.org:8080/#/c/17308/12/tests/custom_cluster/test_event_processing.py
File tests/custom_cluster/test_event_processing.py:

http://gerrit.cloudera.org:8080/#/c/17308/12/tests/custom_cluster/test_event_processing.py@666
PS12, Line 666:           "insert into table {0}.{1} partition (year,month) "
> nit: Could you also add an OVERWRITE insert case? E.g. split this into
Done


http://gerrit.cloudera.org:8080/#/c/17308/12/tests/custom_cluster/test_event_processing.py@709
PS12, Line 709:           # the table is empty so the following inserts add 0 
rows
> nit: seems a wrong comment since "functional.alltypestiny" is not empty..
yes, I removed it. I am not sure what exactly is the purpose of that insert 
there. So I will skip adding the comment for now. If you have any suggestions 
please let me know.



--
To view, visit http://gerrit.cloudera.org:8080/17308
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c5e96b48abac015240f20295b3ec3b1d71f24a
Gerrit-Change-Number: 17308
Gerrit-PatchSet: 12
Gerrit-Owner: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Sourabh Goyal <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 25 Jun 2021 23:14:15 +0000
Gerrit-HasComments: Yes

Reply via email to