Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/28#issuecomment-46936065
Thank you for nice contribution. The patch looks good to me. This patch
seems to fix outer join behaviors in terms of following cases:
* join conditions in located on either ON clause or WHERE clause
* column references included in join conditions correspond to either
row-preserving or null supplying tables.
There are tree comments:
* The patch needs rebase. It causes some conflicts against the changes of
TAJO-850.
* TestJoinQuery is changed to use parameterized unit tests. The constructor
of TestJoinQuery changes the global configuration in all workers. It affects
other unit tests. So, I'm facing following errors. When I remove the
reconfiguration in TestJoinQuery, all tests were passed.
```
Results :
Failed tests:
testGroupbyWithJson(org.apache.tajo.engine.query.TestGroupByQuery): Result
Verification expected:<...-------------------(..)
testHavingWithAggFunction(org.apache.tajo.engine.query.TestGroupByQuery):
Result Verification expected:<...-------------------(..)
testGroupByWithSameConstantKeys1(org.apache.tajo.engine.query.TestGroupByQuery):
Result Verification expected:<...---------(..)
testHavingWithNamedTarget(org.apache.tajo.engine.query.TestGroupByQuery):
Result Verification expected:<...-------------------(..)
testFilterPushDownPartitionColumnCaseWhen(org.apache.tajo.engine.query.TestJoinOnPartitionedTables):
Result Verification
expected:<[c_custkey,c_nationkey,c_name,o_custkey,?casewhen(..)
```
* I found some trivial NPE bug in outer join tests as follows. But, I cound
fix it by adding some trivial workaround code to SeqScanExec::scanAndAddCache
as follows:
```
2014-06-24 14:08:15,673 INFO:
org.apache.tajo.engine.planner.PhysicalPlannerImpl
(createBestLeftOuterJoinPlan(470)) - Left Outer Join (5) chooses [Hash Join].
2014-06-24 14:08:15,675 ERROR: org.apache.tajo.worker.Task (run(395)) -
java.lang.NullPointerException
at
org.apache.tajo.engine.planner.physical.SeqScanExec.scanAndAddCache(SeqScanExec.java:236)
at
org.apache.tajo.engine.planner.physical.SeqScanExec.init(SeqScanExec.java:176)
at
org.apache.tajo.engine.planner.physical.BinaryPhysicalExec.init(BinaryPhysicalExec.java:53)
at
org.apache.tajo.engine.planner.physical.UnaryPhysicalExec.init(UnaryPhysicalExec.java:52)
at
org.apache.tajo.engine.planner.physical.StoreTableExec.init(StoreTableExec.java:48)
at org.apache.tajo.worker.Task.run(Task.java:386)
at org.apache.tajo.worker.TaskRunner$1.run(TaskRunner.java:406)
at java.lang.Thread.run(Thread.java:744)
2014-06-24 14:08:15,675 INFO: org.apache.tajo.worker.TaskAttemptContext
(setState(115)) - Query status of ta_1403586203430_0348_000003_000000_00 is
changed to TA_FAILED
2014-06-24 14:08:15,676 INFO: org.apache.tajo.worker.Task (run(452)) -
Worker's task counter - total:1, succeeded: 0, killed: 1, failed: 1
2014-06-24 14:08:15,676 ERROR:
org.apache.tajo.master.querymaster.QueryUnitAttempt (transition(418)) -
ta_1403586203430_0348_000003_000000_00 FROM 192.168.0.205 >>
java.lang.NullPointerException
2014-06-24 14:08:15,678 INFO: org.apache.tajo.worker.TaskRunner (run(346))
- Request GetTask:
eb_1403586203430_0348_000003,container_1403586203430_0348_01_001096
```
Fixed code in SeqScanExec::scanAndAddCache():
```
if (scanner != null) {
scanner.close();
scanner = null;
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
