Aggarwal-Raghav commented on PR #5872: URL: https://github.com/apache/hive/pull/5872#issuecomment-3024407867
Hi @zabetak, apologies for late reply, just wanted to let you know that I'm working on this PR and found some new issues like [HIVE-29053](https://issues.apache.org/jira/browse/HIVE-29053) along the way when coming up with UT. Writing UT for this PR is not straightforward and **need your opinion on this**. There are 2 things to note: 1. In case of NPE, its just log the error and continue as intended, which means using `TestHiveHookEventProtoPartialBuilder` or `TestHiveProtoLoggingHook` won't cut it IMO (tried with mockito as well, didn't work). 2. Writes/flush of proto data are not consistent based on ScheduledThreadPoolExecutor https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/hooks/HiveProtoLoggingHook.java#L277, one way to force flush is to stop the HS2. What I/m thinking is, writing a itest/hive-unit integration test as below code. Because of [2], I'm not able to make qfile work as the proto data is written post test completition. ``` CREATE TABLE tbl1 (id INT, name STRING); INSERT INTO tbl1 VALUES (1, 'Alice'), (2, 'Bob'); SELECT * FROM tbl1; msck repair table sys.proto_hive_query_data; select count(*) from sys.proto_hive_query_data where lower(trim(regexp_replace(get_json_object(otherinfo[1].value, '$.queryText'), '\n|\\\s+', ' '))) = "select * from tbl1"; ``` Because of NPE the QUERY Object will not be part of proto data so without fix the output should be 0, with fix it should be 1. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
