Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/20181 )
Change subject: IMPALA-12275: Read files written with DeflateCodec ...................................................................... Patch Set 3: (3 comments) Thanks for putting together this fix, I'm fine with this as-is, but a couple questions and nits. http://gerrit.cloudera.org:8080/#/c/20181/3/tests/custom_cluster/test_hive_sequence_codec_interop.py File tests/custom_cluster/test_hive_sequence_codec_interop.py: http://gerrit.cloudera.org:8080/#/c/20181/3/tests/custom_cluster/test_hive_sequence_codec_interop.py@34 PS3, Line 34: CustomClusterTestSuite Not your change, but I'm not sure why these are custom cluster tests rather than end-to-end tests. It doesn't seem like we would need to restart Impala to test this. http://gerrit.cloudera.org:8080/#/c/20181/3/tests/custom_cluster/test_hive_sequence_codec_interop.py@58 PS3, Line 58: test_hive_impala_interop Nit: Since this test function is so similar to the test_hive_text_codec_interop.py one, it might make sense to have a shared base class that provides a function used by both. http://gerrit.cloudera.org:8080/#/c/20181/3/tests/custom_cluster/test_hive_text_codec_interop.py File tests/custom_cluster/test_hive_text_codec_interop.py: http://gerrit.cloudera.org:8080/#/c/20181/3/tests/custom_cluster/test_hive_text_codec_interop.py@30 PS3, Line 30: 'deflate' Was deflate already working for text? i.e. the bug was specific to sequence files? -- To view, visit http://gerrit.cloudera.org:8080/20181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id5ec1d0345ae35597f6aade9d8b9eef2257efeba Gerrit-Change-Number: 20181 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 11 Jul 2023 18:11:40 +0000 Gerrit-HasComments: Yes
