Review request: https://reviews.apache.org/r/17370/

Overall: -1 at least one tests failed

Here are the results of testing the latest attachment:
https://issues.apache.org/jira/secure/attachment/12625241/HIVE-6290.1.patch.txt

ERROR: -1 due to 3 failed/errored test(s), 4958 tests executed
Failed tests:

org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_import_exported_table
org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_load_hdfs_file_with_space_in_the_name
org.apache.hadoop.hive.cli.TestNegativeMinimrCliDriver.testNegativeCliDriver_file_with_header_footer_negative

Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1033/testReport
Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1033/console

Messages:

Executing org.apache.hive.ptest.execution.PrepPhase
Executing org.apache.hive.ptest.execution.ExecutionPhase
Executing org.apache.hive.ptest.execution.ReportingPhase
Tests exited with: TestsFailedException: 3 tests failed

This message is automatically generated.

ATTACHMENT ID: 12625241

The test failures here seem unrelated. The test pass locally on my machine.

mac-swarnim:itests sk018283$ git branch
* issue/HIVE-6290
  trunk

mac-swarnim:itests sk018283$ mvn test -Dtest=TestMinimrCliDriver -Dqfile=testCliDriver_load_hdfs_file_with_space_in_the_name.q -Dtest.output.overwrite=true -Phadoop-1
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Hive Integration - QFile Tests
......
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.hadoop.hive.cli.TestMinimrCliDriver
2014-01-26 20:36:16.574 java[68243:1903] Unable to load realm info from SCDynamicStore
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 226.088 sec - in org.apache.hadoop.hive.cli.TestMinimrCliDriver

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

Hi Swarnim,

Yes those tests are failing on trunk at present. There is work going on to fix them.

Thank you!!
Brock

Hi,

I noticed there are some tabs in the file. Could you change the tabs to spaces?

Brock

Absolutely. Sorry for that. I was under the impression that the eclipse formatter would do it for me. But HIVE-6317 was the result of me realizing that it wasn't. I'll get that fixed.

Fixed and new patch added.

Updated review request: https://reviews.apache.org/r/17483/

Overall: +1 all checks pass

Here are the results of testing the latest attachment:
https://issues.apache.org/jira/secure/attachment/12625799/HIVE-6290.2.patch.txt

SUCCESS: +1 4972 tests passed

Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1096/testReport
Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1096/console

Messages:

Executing org.apache.hive.ptest.execution.PrepPhase
Executing org.apache.hive.ptest.execution.ExecutionPhase
Executing org.apache.hive.ptest.execution.ReportingPhase

This message is automatically generated.

ATTACHMENT ID: 12625799

Nice work! I see you created two RB items for this patch. Perhaps you know this already and created a new item for some other reason, but you can update an existing RB item with a the latest version of a patch.

Thank you very much for your contributions!!

Addressed comments. New patch attached.

Navis Would you also mind taking a quick look as it is related to some of the work that you have been doing with HIVE-6411.

Overall: +1 all checks pass

Here are the results of testing the latest attachment:
https://issues.apache.org/jira/secure/attachment/12629551/HIVE-6290.3.patch.txt

SUCCESS: +1 5133 tests passed

Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1389/testReport
Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1389/console

Messages:

Executing org.apache.hive.ptest.execution.PrepPhase
Executing org.apache.hive.ptest.execution.ExecutionPhase
Executing org.apache.hive.ptest.execution.ReportingPhase

This message is automatically generated.

ATTACHMENT ID: 12629551

Swarnim Kulkarni Interesting. I think I've missed how to make filter predicates (and how to write to hbase) in HIVE-6411. We can proceed with this patch as is state and then I should extend HIVE-6411 functionality onto it.

Thanks NAvis. Brock Noland I have addressed all the review comments. Anything else you want me to look at for this JIRA?

Brock NolandXuefu Zhang Unless there is something like you guys want me to look at with this patch, this should be ready to be merged.

Swarnim Kulkarni The review board (https://reviews.apache.org/r/17483/) doesn't seem having your latest code change. Do you mind updating that? You can reuse the same review request, but uploading diff with your latest patch.

Done.

Patch looks good. Minor comment on review board.

Swarnim Kulkarni Xuefu Zhang I'm really sorry to cut in. But could you consider HIVE-6411 before this? IMHO, this kind of functionality could be provided in more generic way, including full support of predicate pushdown, etc. Just consider once.

Navis Thanks for sharing your thoughts. I haven't looked at HIVE-6411, so not sure of the relationship between the two. I'll take a look and figured out what's the best. Thanks again.

Navis Out of curiosity, do you have a rough design proposal for this support over HIVE-6411? Just was curious as to how things could change. Thanks.

Maybe I can make HBaseCompositeKey$HBaseCompositeKeyFactory to implement HBaseWritableKeyFactory, which is

public interface HBaseWritableKeyFactory extends HBaseKeyFactory, HiveStoragePredicateHandler {
  boolean serialize(Object object, ObjectInspector inspector, ByteStream.Output output) throws IOException;
}

Thanks Navis. So your proposal here is that we first let HIVE-6411 in and then apply changes made in this patch on top of it or kill the patch here all together in lieu for HIVE-6411?

I've regarded that as a following issue but I've merged your patch into HIVE-6411.

Resolved via HIVE-6411.

Does this need any user doc?

User doc should go with HIVE-6411 also.

This has been fixed in 0.14 release. Please open new jira if you see any issues.