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

'INSERT INTO' statement to non existence table casuses NPE.

    Details

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

      Description

      See the title. In the following example, lineitem_8 does not exist in catalog. In this case, if we use INSERT INTO statement, NPE will be caused. This is a logical validator bug.

      default> insert into lineitem_8 SELECT L_ORDERKEY, L_PARTKEY, L_SUPPKEY, L_LINENUMBER, L_QUANTITY, L_EXTENDEDPRICE, L_DISCOUNT, L_TAX, L_LINESTATUS, L_SHIPDATE, L_COMMITDATE, L_RECEIPTDATE, L_SHIPINSTRUCT, L_SHIPMODE, L_COMMENT, L_RETURNFLAG FROM LINEITEM;
      ERROR: java.lang.NullPointerException
      

        Activity

        Hide
        eminency Jongyoung Park added a comment -

        The cause is there is no part to check if the table instance is null before it's used

        Show
        eminency Jongyoung Park added a comment - The cause is there is no part to check if the table instance is null before it's used
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user eminency opened a pull request:

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

        TAJO-948: INSERT INTO' statement to non existence table casuses NPE

        The cause is there is no part to check if the table instance is null before it's used.

        Please review if the changeset is appropriate.

        Thanks.

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

        $ git pull https://github.com/eminency/tajo TAJO-948

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

        https://github.com/apache/tajo/pull/142.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 #142


        commit 0b96b4cf26bd24f108ff7e268e1c3199693cbc7f
        Author: Jongyoung Park <eminency@gmail.com>
        Date: 2014-09-16T09:57:15Z

        Checking null code is added to verify table existence


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user eminency opened a pull request: https://github.com/apache/tajo/pull/142 TAJO-948 : INSERT INTO' statement to non existence table casuses NPE The cause is there is no part to check if the table instance is null before it's used. Please review if the changeset is appropriate. Thanks. You can merge this pull request into a Git repository by running: $ git pull https://github.com/eminency/tajo TAJO-948 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/142.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 #142 commit 0b96b4cf26bd24f108ff7e268e1c3199693cbc7f Author: Jongyoung Park <eminency@gmail.com> Date: 2014-09-16T09:57:15Z Checking null code is added to verify table existence
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/142#discussion_r17604055

        — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java —
        @@ -254,6 +254,10 @@ public Expr visitInsert(Context context, Stack<Expr> stack, Insert expr) throws
        }

        TableDesc table = catalog.getTableDesc(qualifiedName);
        + if (table == null) {
        + context.state.addVerification("It seems target table doesn't exist");
        — End diff –

        The bug fix looks correct. But, I have one suggestion about the message. The other similar cases use ```relation "%s" does not exist```. Could you change the message for consistency?

        You can see the similar case at https://github.com/eminency/tajo/blob/TAJO-948/tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java#L134

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/142#discussion_r17604055 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java — @@ -254,6 +254,10 @@ public Expr visitInsert(Context context, Stack<Expr> stack, Insert expr) throws } TableDesc table = catalog.getTableDesc(qualifiedName); + if (table == null) { + context.state.addVerification("It seems target table doesn't exist"); — End diff – The bug fix looks correct. But, I have one suggestion about the message. The other similar cases use ```relation "%s" does not exist```. Could you change the message for consistency? You can see the similar case at https://github.com/eminency/tajo/blob/TAJO-948/tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java#L134
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user eminency commented on the pull request:

        https://github.com/apache/tajo/pull/142#issuecomment-55751677

        @hyunsik ,

        Your line comment sounds like a good suggestion.
        I will touch further.

        Thanks.

        Show
        githubbot ASF GitHub Bot added a comment - Github user eminency commented on the pull request: https://github.com/apache/tajo/pull/142#issuecomment-55751677 @hyunsik , Your line comment sounds like a good suggestion. I will touch further. Thanks.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user eminency commented on the pull request:

        https://github.com/apache/tajo/pull/142#issuecomment-55852967

        The message was refined.

        Show
        githubbot ASF GitHub Bot added a comment - Github user eminency commented on the pull request: https://github.com/apache/tajo/pull/142#issuecomment-55852967 The message was refined.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/142#issuecomment-55923268

        +1

        The patch looks good to me.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/142#issuecomment-55923268 +1 The patch looks good to me.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

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

        I've just committed it to master branch.

        Welcome to Tajo community. Thank you for your first contribution to Tajo project.

        Show
        hyunsik Hyunsik Choi added a comment - I've just committed it to master branch. Welcome to Tajo community. Thank you for your first contribution to Tajo project.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #362 (See https://builds.apache.org/job/Tajo-master-build/362/)
        TAJO-948: 'INSERT INTO' statement to non existence table casuses NPE. (Jongyoung Park via hyunsik) (hyunsik: rev 200177482fbce1e33de22ad3a22f7c6b4c9cb7b7)

        • tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #362 (See https://builds.apache.org/job/Tajo-master-build/362/ ) TAJO-948 : 'INSERT INTO' statement to non existence table casuses NPE. (Jongyoung Park via hyunsik) (hyunsik: rev 200177482fbce1e33de22ad3a22f7c6b4c9cb7b7) tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java CHANGES
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-CODEGEN-build #4 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/4/)
        TAJO-948: 'INSERT INTO' statement to non existence table casuses NPE. (Jongyoung Park via hyunsik) (hyunsik: rev 200177482fbce1e33de22ad3a22f7c6b4c9cb7b7)

        • tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-CODEGEN-build #4 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/4/ ) TAJO-948 : 'INSERT INTO' statement to non existence table casuses NPE. (Jongyoung Park via hyunsik) (hyunsik: rev 200177482fbce1e33de22ad3a22f7c6b4c9cb7b7) tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java CHANGES
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-block_iteration-branch-build #3 (See https://builds.apache.org/job/Tajo-block_iteration-branch-build/3/)
        TAJO-948: 'INSERT INTO' statement to non existence table casuses NPE. (Jongyoung Park via hyunsik) (hyunsik: rev 200177482fbce1e33de22ad3a22f7c6b4c9cb7b7)

        • tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-block_iteration-branch-build #3 (See https://builds.apache.org/job/Tajo-block_iteration-branch-build/3/ ) TAJO-948 : 'INSERT INTO' statement to non existence table casuses NPE. (Jongyoung Park via hyunsik) (hyunsik: rev 200177482fbce1e33de22ad3a22f7c6b4c9cb7b7) tajo-core/src/main/java/org/apache/tajo/engine/planner/PreLogicalPlanVerifier.java CHANGES

          People

          • Assignee:
            eminency Jongyoung Park
            Reporter:
            hyunsik Hyunsik Choi
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development