Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19052 )
Change subject: IMPALA-8592: Add support for insert events for 'LOAD DATA' statements from Impala ...................................................................... Patch Set 1: (3 comments) This is a pretty nice fix! http://gerrit.cloudera.org:8080/#/c/19052/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19052/1//COMMIT_MSG@16 PS1, Line 16: - Run existing test_load.py We also need tests to verify the INSERT events. Could you add some tests in tests/metadata/test_event_processing.py, e.g. add some cases for LOAD DATA in test_event_based_replication? http://gerrit.cloudera.org:8080/#/c/19052/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/19052/1/be/src/service/client-request-state.cc@806 PS1, Line 806: response.partition_name nit: Could you add a comment mentioning that the partition_name is an empty string for unpartitioned tables? It's the same as DataSink::ROOT_PARTITION_KEY. http://gerrit.cloudera.org:8080/#/c/19052/1/be/src/service/client-request-state.cc@809 PS1, Line 809: catalog_update.__set_header(TCatalogServiceRequestHeader()); : catalog_update.header.__set_requesting_user(effective_user()); : catalog_update.header.__set_client_ip(session()->network_address.hostname); : catalog_update.header.__set_want_minimal_response(FLAGS_use_local_catalog); : catalog_update.header.__set_redacted_sql_stmt( : query_ctx_.client_request.__isset.redacted_stmt ? : query_ctx_.client_request.redacted_stmt : query_ctx_.client_request.stmt); nit: these duplicate the code in ClientRequestState::ExecLoadDataRequestImpl(). It'd be nice to extract them into a method like GetCatalogServiceRequestHeader(), and simplify the code to catalog_update.__set_header(GetCatalogServiceRequestHeader()) -- To view, visit http://gerrit.cloudera.org:8080/19052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f1b470f40e0aaf891c9f3f327af393b2f9c74bc Gerrit-Change-Number: 19052 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-Wen Lai <yu-wen....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Yu-Wen Lai <yu-wen....@cloudera.com> Gerrit-Comment-Date: Fri, 30 Sep 2022 01:55:21 +0000 Gerrit-HasComments: Yes
