Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-850

OUTER JOIN does not properly handle a NULL.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.9.0
    • Component/s: None
    • Labels:
      None

      Description

      See the title. In the case of the following query NULL value is evaluated at the SCAN time.

      select
        c_custkey,
        coalesce(orders.o_totalprice, 999.0)
      from
        customer left outer join orders on c_name = o_orderstatus
      
      select
        c_custkey,
      from
        customer left outer join orders on c_name = o_orderstatus
      where o_orderdate is null
      

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #253 (See https://builds.apache.org/job/Tajo-master-build/253/)
        TAJO-850: OUTER JOIN does not properly handle a NULL. (Hyoungjun Kim via hyunsik) (hyunsik: rev be21bc706d9ebade71e976ecb966149cf1272385)

        • tajo-core/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithNull1.sql
        • tajo-core/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithNull2.sql
        • tajo-core/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithNull1.result
        • tajo-core/src/test/resources/queries/TestJoinQuery/oj_table2_ddl.sql
        • tajo-core/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithNull3.sql
        • tajo-core/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithNull3.result
        • tajo-core/src/test/resources/queries/TestJoinQuery/oj_table1_ddl.sql
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/engine/eval/EvalTreeUtil.java
        • tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
        • tajo-core/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java
        • tajo-core/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithNull2.result
        • tajo-core/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #253 (See https://builds.apache.org/job/Tajo-master-build/253/ ) TAJO-850 : OUTER JOIN does not properly handle a NULL. (Hyoungjun Kim via hyunsik) (hyunsik: rev be21bc706d9ebade71e976ecb966149cf1272385) tajo-core/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithNull1.sql tajo-core/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithNull2.sql tajo-core/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithNull1.result tajo-core/src/test/resources/queries/TestJoinQuery/oj_table2_ddl.sql tajo-core/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithNull3.sql tajo-core/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithNull3.result tajo-core/src/test/resources/queries/TestJoinQuery/oj_table1_ddl.sql CHANGES tajo-core/src/main/java/org/apache/tajo/engine/eval/EvalTreeUtil.java tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java tajo-core/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithNull2.result tajo-core/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed it to master branch. Thanks!

        Show
        hyunsik Hyunsik Choi added a comment - committed it to master branch. Thanks!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/22

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/22
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/22#issuecomment-46927666

        +1

        Although the unit test seems to be failed, the failure is caused by the long log message. Travis causes failure if the log size > 4MB.

        I verified 'mvn clean install'. The patch fixes outer join condition bugs and includes enough unit tests to verify the bug fix.

        In addition, I revised some trivial things as follows:

        • Remove unnecessary plan log in GlobalEngine.java
        • Remove commented out lines in TajoTestingCluster and ProjectionPushDownRule
        • Rename EvalTreeUtil::findOuterJoinConditionEvals to EvalTreeUtil::findOuterJoinSensitiveEvals because they are subexpressions of join condition.

        I'll commit it shortly. Thank you for your great contribution!

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/22#issuecomment-46927666 +1 Although the unit test seems to be failed, the failure is caused by the long log message. Travis causes failure if the log size > 4MB. I verified 'mvn clean install'. The patch fixes outer join condition bugs and includes enough unit tests to verify the bug fix. In addition, I revised some trivial things as follows: Remove unnecessary plan log in GlobalEngine.java Remove commented out lines in TajoTestingCluster and ProjectionPushDownRule Rename EvalTreeUtil::findOuterJoinConditionEvals to EvalTreeUtil::findOuterJoinSensitiveEvals because they are subexpressions of join condition. I'll commit it shortly. Thank you for your great contribution!
        Hide
        hyunsik Hyunsik Choi added a comment -

        Thank you for sharing the documents.

        Show
        hyunsik Hyunsik Choi added a comment - Thank you for sharing the documents.
        Hide
        hjkim Hyoungjun Kim added a comment -

        Pull requested via the github. Please review this patch.

        Show
        hjkim Hyoungjun Kim added a comment - Pull requested via the github. Please review this patch.
        Hide
        hjkim Hyoungjun Kim added a comment -

        I rebased and removed some Korean comments.

        Show
        hjkim Hyoungjun Kim added a comment - I rebased and removed some Korean comments.
        Hide
        hjkim Hyoungjun Kim added a comment -

        I found the following documents for the OUTER JOIN's filter push down.
        http://www.ibm.com/developerworks/data/library/techarticle/purcell/0112purcell.html
        https://cwiki.apache.org/confluence/display/Hive/OuterJoinBehavior

        There are many logic to fix fully outer join capability, so I will fix only for filters in WHERE clause in this issues.
        I'll create other ISSUE for OUTER JOIN's ON clause push down.

        Show
        hjkim Hyoungjun Kim added a comment - I found the following documents for the OUTER JOIN's filter push down. http://www.ibm.com/developerworks/data/library/techarticle/purcell/0112purcell.html https://cwiki.apache.org/confluence/display/Hive/OuterJoinBehavior There are many logic to fix fully outer join capability, so I will fix only for filters in WHERE clause in this issues. I'll create other ISSUE for OUTER JOIN's ON clause push down.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/22#issuecomment-44242203

        This is conflict to the latest revision. Could you rebase it?

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/22#issuecomment-44242203 This is conflict to the latest revision. Could you rebase it?
        Hide
        sirpkt Keuntae Park added a comment -

        It works well !!

        However, how about change the name of method 'checkIfCaseWhenWithOuterJoinBeEvaluated' ?
        Because, it no longer checks 'case when' only. It additionally checks 'is null' and 'coalesce'.

        One suggestion is 'checkIfShouldBeEvaluatedAtTopMostJoin' or making separate method for each 'is null' and 'coalesce'.

        Show
        sirpkt Keuntae Park added a comment - It works well !! However, how about change the name of method 'checkIfCaseWhenWithOuterJoinBeEvaluated' ? Because, it no longer checks 'case when' only. It additionally checks 'is null' and 'coalesce'. One suggestion is 'checkIfShouldBeEvaluatedAtTopMostJoin' or making separate method for each 'is null' and 'coalesce'.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user babokim opened a pull request:

        https://github.com/apache/tajo/pull/22

        TAJO-850: OUTER JOIN does not properly handle a NULL.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/babokim/tajo TAJO-850

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/22.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #22


        commit 922e4b9d820d7f1330097f16392e3c0c65f2e23b
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-20T17:30:38Z

        TAJO-830

        commit e8e8efe933269abd147dfb0504f7b57e65a08183
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-20T17:37:21Z

        Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo

        commit d8b4fa9b046c8a04cbd6fcd7a5b6724068ff8b1b
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-20T17:42:14Z

        Tajo 830: Some filter conditions with a SUBQUERY are removed by optimizer. => Remove unused test code

        commit 3f6b6d0918645de5b27ef748f30c3beb733d6894
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-25T05:10:57Z

        TAJO-830:Some filter conditions with a SUBQUERY are removed by optimizer(remove the korean comment)

        commit e87b72725b384496b7e7667e85a5032270ac21af
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-26T13:18:40Z

        TAJO-850: OUTER JOIN does not properly handle a NULL.

        commit 9215829f52130ce7c8b31fd49c794865a4924e15
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-27T01:09:41Z

        Merge branch 'TAJO-830' of https://github.com/babokim/tajo

        Conflicts:
        tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java

        commit eda038cc492dba0e8a8e914bfaf95158aea31807
        Author: 김형준 <babokim@babokim-macbook-pro.local>
        Date: 2014-05-27T01:34:10Z

        Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo

        Conflicts:
        tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java
        tajo-core/src/main/java/org/apache/tajo/engine/planner/logical/GroupbyNode.java
        tajo-core/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user babokim opened a pull request: https://github.com/apache/tajo/pull/22 TAJO-850 : OUTER JOIN does not properly handle a NULL. You can merge this pull request into a Git repository by running: $ git pull https://github.com/babokim/tajo TAJO-850 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/22.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22 commit 922e4b9d820d7f1330097f16392e3c0c65f2e23b Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-20T17:30:38Z TAJO-830 commit e8e8efe933269abd147dfb0504f7b57e65a08183 Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-20T17:37:21Z Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo commit d8b4fa9b046c8a04cbd6fcd7a5b6724068ff8b1b Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-20T17:42:14Z Tajo 830: Some filter conditions with a SUBQUERY are removed by optimizer. => Remove unused test code commit 3f6b6d0918645de5b27ef748f30c3beb733d6894 Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-25T05:10:57Z TAJO-830 :Some filter conditions with a SUBQUERY are removed by optimizer(remove the korean comment) commit e87b72725b384496b7e7667e85a5032270ac21af Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-26T13:18:40Z TAJO-850 : OUTER JOIN does not properly handle a NULL. commit 9215829f52130ce7c8b31fd49c794865a4924e15 Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-27T01:09:41Z Merge branch ' TAJO-830 ' of https://github.com/babokim/tajo Conflicts: tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java commit eda038cc492dba0e8a8e914bfaf95158aea31807 Author: 김형준 <babokim@babokim-macbook-pro.local> Date: 2014-05-27T01:34:10Z Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo Conflicts: tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java tajo-core/src/main/java/org/apache/tajo/engine/planner/logical/GroupbyNode.java tajo-core/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java

          People

          • Assignee:
            hjkim Hyoungjun Kim
            Reporter:
            hjkim Hyoungjun Kim
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development