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

Tajo should check if a file format given in DDL is supported.

    Details

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

      Description

      Currently Tajo just show NullPointException when can't understand file format
      ```c
      default> create external table table1 ( id int, name text, score float, type text, mytime int, mytime2 date) using TEXT1 location 'file:/Users/charsyam/tajo/table';
      ERROR: java.lang.NullPointerException
      ```
      It is better to return using verficationstate

      1. TAJO-1249_2.patch
        4 kB
        Hyunsik Choi
      2. TAJO-1249.patch
        4 kB
        Hyunsik Choi

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #519 (See https://builds.apache.org/job/Tajo-master-build/519/)
        TAJO-1249: Tajo should check if a file format given in DDL is supported. (DaeMyung Kang via hyunsik) (hyunsik: rev 09cad22ee2f7c30a0f28ebe50e00612777e69c86)

        • tajo-core/src/test/java/org/apache/tajo/engine/eval/TestPredicates.java
        • tajo-core/src/test/resources/queries/TestQueryValidation/invalid_store_format.sql
        • tajo-plan/src/main/java/org/apache/tajo/plan/verifier/PreLogicalPlanVerifier.java
        • CHANGES
        • tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #519 (See https://builds.apache.org/job/Tajo-master-build/519/ ) TAJO-1249 : Tajo should check if a file format given in DDL is supported. (DaeMyung Kang via hyunsik) (hyunsik: rev 09cad22ee2f7c30a0f28ebe50e00612777e69c86) tajo-core/src/test/java/org/apache/tajo/engine/eval/TestPredicates.java tajo-core/src/test/resources/queries/TestQueryValidation/invalid_store_format.sql tajo-plan/src/main/java/org/apache/tajo/plan/verifier/PreLogicalPlanVerifier.java CHANGES tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #159 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/159/)
        TAJO-1249: Tajo should check if a file format given in DDL is supported. (DaeMyung Kang via hyunsik) (hyunsik: rev 09cad22ee2f7c30a0f28ebe50e00612777e69c86)

        • tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java
        • tajo-core/src/test/resources/queries/TestQueryValidation/invalid_store_format.sql
        • tajo-core/src/test/java/org/apache/tajo/engine/eval/TestPredicates.java
        • tajo-plan/src/main/java/org/apache/tajo/plan/verifier/PreLogicalPlanVerifier.java
        • CHANGES
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #159 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/159/ ) TAJO-1249 : Tajo should check if a file format given in DDL is supported. (DaeMyung Kang via hyunsik) (hyunsik: rev 09cad22ee2f7c30a0f28ebe50e00612777e69c86) tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java tajo-core/src/test/resources/queries/TestQueryValidation/invalid_store_format.sql tajo-core/src/test/java/org/apache/tajo/engine/eval/TestPredicates.java tajo-plan/src/main/java/org/apache/tajo/plan/verifier/PreLogicalPlanVerifier.java CHANGES
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed to master branch. Thank you DaeMyung!

        Show
        hyunsik Hyunsik Choi added a comment - committed to master branch. Thank you DaeMyung!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

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

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12688996/TAJO-1249_2.patch
        against master revision release-0.9.0-rc0-106-g3c833e2.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 223 new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 186 release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-core tajo-plan.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/552//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/552//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/552//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/552//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/552//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688996/TAJO-1249_2.patch against master revision release-0.9.0-rc0-106-g3c833e2. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 223 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 186 release audit warnings. +1 core tests. The patch passed unit tests in tajo-core tajo-plan. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/552//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/552//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/552//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/552//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/552//console This message is automatically generated.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12688966/TAJO-1249.patch
        against master revision release-0.9.0-rc0-106-g3c833e2.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 223 new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 543 release audit warnings.

        -1 core tests. The patch failed these unit tests in tajo-core tajo-plan:
        org.apache.tajo.engine.query.TestJoinBroadcast
        org.apache.tajo.engine.query.TestJoinQuery

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/551//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/551//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/551//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/551//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/551//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688966/TAJO-1249.patch against master revision release-0.9.0-rc0-106-g3c833e2. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 223 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 543 release audit warnings. -1 core tests. The patch failed these unit tests in tajo-core tajo-plan: org.apache.tajo.engine.query.TestJoinBroadcast org.apache.tajo.engine.query.TestJoinQuery Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/551//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/551//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/551//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/551//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-plan.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/551//console This message is automatically generated.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/313#issuecomment-68018425

        +1
        assertXXX is usually used to ensure a specified case. So, assertSupportedStoreType is proper. But, it's a trivial. I'll rename it and then commit it shortly. Thank you for your contribution!

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/313#issuecomment-68018425 +1 assertXXX is usually used to ensure a specified case. So, assertSupportedStoreType is proper. But, it's a trivial. I'll rename it and then commit it shortly. Thank you for your contribution!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/313#issuecomment-67767305

        @hyunsik Oh, Thanks. It was all my mistake. Thanks.

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/313#issuecomment-67767305 @hyunsik Oh, Thanks. It was all my mistake. Thanks.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/313#issuecomment-67759687

        Hi @charsyam,

        Thank you for your contribution. I'd like to suggest one thing about github usage. You don't need to close and create new pull request to update the patch. You just commit and push to the same branch that you used to pull request. Then, the pull request will be automatically updated.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/313#issuecomment-67759687 Hi @charsyam, Thank you for your contribution. I'd like to suggest one thing about github usage. You don't need to close and create new pull request to update the patch. You just commit and push to the same branch that you used to pull request. Then, the pull request will be automatically updated.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/313#issuecomment-67656651

        @blrunner I applied your reviews. and fix testcases.

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/313#issuecomment-67656651 @blrunner I applied your reviews. and fix testcases.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user charsyam opened a pull request:

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

        TAJO-1249: Tajo Should Check File Format is allowed.

        Currently Tajo just show NullPointException when can't understand file format
        ```c
        default> create external table table1 ( id int, name text, score float, type text, mytime int, mytime2 date) using TEXT1 location 'file:/Users/charsyam/tajo/table';
        ERROR: java.lang.NullPointerException
        ```
        It is better to return using verficationstate

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

        $ git pull https://github.com/charsyam/tajo feature/TAJO-1249

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

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


        commit ad6ea76a77515899cd83d1124b19b09afd1467e2
        Author: clark.kang <clark.kang@kakao.com>
        Date: 2014-12-19T15:58:42Z

        TAJO-1249


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user charsyam opened a pull request: https://github.com/apache/tajo/pull/313 TAJO-1249 : Tajo Should Check File Format is allowed. Currently Tajo just show NullPointException when can't understand file format ```c default> create external table table1 ( id int, name text, score float, type text, mytime int, mytime2 date) using TEXT1 location 'file:/Users/charsyam/tajo/table'; ERROR: java.lang.NullPointerException ``` It is better to return using verficationstate You can merge this pull request into a Git repository by running: $ git pull https://github.com/charsyam/tajo feature/ TAJO-1249 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/313.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 #313 commit ad6ea76a77515899cd83d1124b19b09afd1467e2 Author: clark.kang <clark.kang@kakao.com> Date: 2014-12-19T15:58:42Z TAJO-1249
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam closed the pull request at:

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

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

        Github user blrunner commented on the pull request:

        https://github.com/apache/tajo/pull/305#issuecomment-67439800

        I give new opinion again. I think you'd better use PreLogicalPlanVerifier instead of LogicalPlanVerifier. PreLogicalPlanVerifier provide query validation before creating local plan.

        Show
        githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/305#issuecomment-67439800 I give new opinion again. I think you'd better use PreLogicalPlanVerifier instead of LogicalPlanVerifier. PreLogicalPlanVerifier provide query validation before creating local plan.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user blrunner commented on the pull request:

        https://github.com/apache/tajo/pull/305#issuecomment-67436403

        Hi @charsyam

        Thank you for your contribution.
        LogicalPlanVerifier provide logical plan validation. I think you'd better update LogicalPlanVerifier::visitCreateTable.

        Cheers
        Jaehwa

        Show
        githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/305#issuecomment-67436403 Hi @charsyam Thank you for your contribution. LogicalPlanVerifier provide logical plan validation. I think you'd better update LogicalPlanVerifier::visitCreateTable. Cheers Jaehwa
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user charsyam opened a pull request:

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

        TAJO-1249: Tajo Should Check File Format is allowed.

        Currently Tajo just show NullPointException when can't understand file format
        ```c
        default> create external table table1 ( id int, name text, score float, type text, mytime int, mytime2 date) using TEXT1 location 'file:/Users/charsyam/tajo/table';
        ERROR: java.lang.NullPointerException
        ```
        after patch
        ```c
        default> create table table1 ( id int, name text, score float, type text, mytime int) USING csv1;
        ERROR: csv1 is not supported storage type.
        ```

        I added code in LogicalPlanner.java because I think it is more clear and easy to understand.

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

        $ git pull https://github.com/charsyam/tajo feature/TAJO-1249

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

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


        commit d227691369e71460055cbdde083d476518fba27a
        Author: clark.kang <clark.kang@kakao.com>
        Date: 2014-12-17T15:19:31Z

        TAJO-1249


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user charsyam opened a pull request: https://github.com/apache/tajo/pull/305 TAJO-1249 : Tajo Should Check File Format is allowed. Currently Tajo just show NullPointException when can't understand file format ```c default> create external table table1 ( id int, name text, score float, type text, mytime int, mytime2 date) using TEXT1 location 'file:/Users/charsyam/tajo/table'; ERROR: java.lang.NullPointerException ``` after patch ```c default> create table table1 ( id int, name text, score float, type text, mytime int) USING csv1; ERROR: csv1 is not supported storage type. ``` I added code in LogicalPlanner.java because I think it is more clear and easy to understand. You can merge this pull request into a Git repository by running: $ git pull https://github.com/charsyam/tajo feature/ TAJO-1249 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/305.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 #305 commit d227691369e71460055cbdde083d476518fba27a Author: clark.kang <clark.kang@kakao.com> Date: 2014-12-17T15:19:31Z TAJO-1249

          People

          • Assignee:
            charsyam DaeMyung Kang
            Reporter:
            charsyam DaeMyung Kang
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development