Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15814 )

Change subject: IMPALA-9709: Remove Impala-lzo from the development environment
......................................................................


Patch Set 5:

(2 comments)

> (2 comments)
 >
 > Looks good!
 > just a few more updates left as follows:
 >
 > codec.cc => update the message in NO_LZO_MSG
 > HdfsScanNode.java => update VALID_LEGACY_FORMATS
 > HdfsTableSink.java => update SUPPORTED_FILE_FORMATS

Sorry to take so long getting back to this.

For NO_LZO_MSG, I updated it to say that LZO is processed by an optional 
plugin. Is that what you had in mind?

I removed LZO_TEXT from VALID_LEGACY_FORMATS and SUPPORTED_FILE_FORMATS. It 
turns out that LZO_TEXT was never actually being used on those codepaths. 
Instead, LZO tables use TEXT and the HdfsFileFormat. We could have removed 
those locations before this change. I added a comment in the declaration of 
LZO_TEXT to note that it isn't used like the other HdfsFileFormats.

http://gerrit.cloudera.org:8080/#/c/15814/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15814/5//COMMIT_MSG@11
PS5, Line 11: longer loaded a plugin. LZO tables are not loaded during dataload,
> nit: as a
Reworked the sentence.


http://gerrit.cloudera.org:8080/#/c/15814/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
File 
testdata/workloads/functional-query/queries/QueryTest/show-create-table.test:

http://gerrit.cloudera.org:8080/#/c/15814/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test@a381
PS5, Line 381:
> nit: i think this test might have been put in to verify INPUTFORMAT and OUT
Good point, this test was specific to LZO. Impala doesn't support 
INPUTFORMAT/OUTPUTFORMAT, so these were only ever outputted for LZO so that the 
statement would work in Hive. We never use INPUTFORMAT/OUTPUTFORMAT for 
anything else.

It looks like we already have coverage for show create table on text, so I'm 
just going to remove this test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a4f12247d8872b7e14c9feb4b2c58cfd60d4c0e
Gerrit-Change-Number: 15814
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Mon, 15 Jun 2020 05:25:45 +0000
Gerrit-HasComments: Yes

Reply via email to