Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: SQL Parser
    • Labels:
      None

      Description

      This patch adds the nested record definition to CREATE TABLE BNF. The changed BNF is as follows:

      predefined_type
        : character_string_type
        | national_character_string_type
        | binary_large_object_string_type
        | numeric_type
        | boolean_type
        | datetime_type
        | bit_type
        | binary_type
        | network_type
        | record_type
        ;
      
      record_type
        : RECORD table_elements
        ;
      
      table_elements
        : LEFT_PAREN field_element (COMMA field_element)* RIGHT_PAREN
        ;
      

      Example statements as follows:

      CREATE TABLE T1 (
        A TEXT, 
        B INT4, 
        C RECORD (
          D TEXT, 
          E INT8, 
          F RECORD (
            G INT1, H FLOAT4
          )
        ), 
        Z FLOAT8
      );
      

      Updates

      • I changed the type name 'struct' to 'record' according to TAJO-809.
      1. TAJO-1353.patch
        24 kB
        Hyunsik Choi

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user hyunsik opened a pull request:

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

          TAJO-1353: CREATE TABLE should support the nested record definition.

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

          $ git pull https://github.com/hyunsik/tajo TAJO-1353

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

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


          commit ea6aeb38075fb3684ec33dacdf6d7769ef6c7e5f
          Author: Hyunsik Choi <hyunsik@apache.org>
          Date: 2015-02-21T01:56:06Z

          TAJO-1353: CREATE TABLE should support the nested record definition.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user hyunsik opened a pull request: https://github.com/apache/tajo/pull/385 TAJO-1353 : CREATE TABLE should support the nested record definition. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hyunsik/tajo TAJO-1353 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/385.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 #385 commit ea6aeb38075fb3684ec33dacdf6d7769ef6c7e5f Author: Hyunsik Choi <hyunsik@apache.org> Date: 2015-02-21T01:56:06Z TAJO-1353 : CREATE TABLE should support the nested record definition.
          Hide
          hyunsik Hyunsik Choi added a comment -

          The grammar is my proposal. I open to any suggestion about the DDL grammar.

          Show
          hyunsik Hyunsik Choi added a comment - The grammar is my proposal. I open to any suggestion about the DDL grammar.
          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/12699988/TAJO-1353.patch
          against master revision release-0.9.0-rc0-174-g95d9cc4.

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

          +1 tests included. The patch appears to include 5 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 188 new Findbugs (version 2.0.3) warnings.

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

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

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/586//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/586//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/586//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/586//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/586//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/12699988/TAJO-1353.patch against master revision release-0.9.0-rc0-174-g95d9cc4. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 5 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 188 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 123 release audit warnings. +1 core tests. The patch passed unit tests in tajo-algebra tajo-common tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/586//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/586//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/586//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/586//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/586//console This message is automatically generated.
          Hide
          jihoonson Jihoon Son added a comment -

          +1 for the proposal!
          It looks nice.

          Show
          jihoonson Jihoon Son added a comment - +1 for the proposal! It looks nice.
          Hide
          hyunsik Hyunsik Choi added a comment -

          Hi Jihoon Son,

          Thank you for the quick response. Did you vote +1 for the patch? If so, I'll commit it shortly.

          Show
          hyunsik Hyunsik Choi added a comment - Hi Jihoon Son , Thank you for the quick response. Did you vote +1 for the patch? If so, I'll commit it shortly.
          Hide
          jihoonson Jihoon Son added a comment -

          Hyunsik Choi, sorry for confusing you.
          I just looked over the patch, and would like to review the changes of SQLAnalyzer more carefully because the patch contains not only the changes related to the nested record, but also some refactoring codes.
          I'll finish my review, soon.

          Show
          jihoonson Jihoon Son added a comment - Hyunsik Choi , sorry for confusing you. I just looked over the patch, and would like to review the changes of SQLAnalyzer more carefully because the patch contains not only the changes related to the nested record, but also some refactoring codes. I'll finish my review, soon.
          Hide
          hyunsik Hyunsik Choi added a comment - - edited

          I missed to mention that I just cleaned up the type related code SQLAnalyzer. The change just simplified the logic and null checks.

          Also, I don't want to hasten the review. Please take your time. Anyway, I'm going to do the next step.

          Show
          hyunsik Hyunsik Choi added a comment - - edited I missed to mention that I just cleaned up the type related code SQLAnalyzer. The change just simplified the logic and null checks. Also, I don't want to hasten the review. Please take your time. Anyway, I'm going to do the next step.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/385#issuecomment-75354657

          +1
          The patch LGTM.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/385#issuecomment-75354657 +1 The patch LGTM.
          Hide
          jihoonson Jihoon Son added a comment -

          Sorry for my mistake at assignee.
          Anyway, as you said, the refactoring was just the simple cleanup of the logic and null checks.
          Thanks for your contribution!

          Show
          jihoonson Jihoon Son added a comment - Sorry for my mistake at assignee. Anyway, as you said, the refactoring was just the simple cleanup of the logic and null checks. Thanks for your contribution!
          Hide
          hyunsik Hyunsik Choi added a comment -

          Thank you for the quick review. I'll commit it shortly.

          Show
          hyunsik Hyunsik Choi added a comment - Thank you for the quick review. I'll commit it shortly.
          Hide
          hyunsik Hyunsik Choi added a comment -

          I just changed the issue title.

          Show
          hyunsik Hyunsik Choi added a comment - I just changed the issue title.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

          I just committed the patch to master branch.

          Show
          hyunsik Hyunsik Choi added a comment - I just committed the patch to master branch.
          Hide
          hudson Hudson added a comment -

          ABORTED: Integrated in Tajo-master-CODEGEN-build #225 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/225/)
          TAJO-1353: CREATE TABLE should support the nested record definition. (hyunsik: rev 08f30345047ddf62dc51c33b9c350f08bf918a5c)

          • tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_2.result
          • tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_1.sql
          • tajo-common/src/main/proto/DataTypes.proto
          • tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
          • tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4
          • CHANGES
          • tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_2.sql
          • tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4
          • tajo-algebra/src/main/java/org/apache/tajo/algebra/DataTypeExpr.java
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_1.result
          • tajo-algebra/src/main/java/org/apache/tajo/algebra/ColumnDefinition.java
          • tajo-core/src/test/java/org/apache/tajo/engine/parser/TestSQLAnalyzer.java
          Show
          hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-CODEGEN-build #225 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/225/ ) TAJO-1353 : CREATE TABLE should support the nested record definition. (hyunsik: rev 08f30345047ddf62dc51c33b9c350f08bf918a5c) tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_2.result tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_1.sql tajo-common/src/main/proto/DataTypes.proto tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4 CHANGES tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_2.sql tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 tajo-algebra/src/main/java/org/apache/tajo/algebra/DataTypeExpr.java tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_1.result tajo-algebra/src/main/java/org/apache/tajo/algebra/ColumnDefinition.java tajo-core/src/test/java/org/apache/tajo/engine/parser/TestSQLAnalyzer.java
          Hide
          hudson Hudson added a comment -

          ABORTED: Integrated in Tajo-master-build #586 (See https://builds.apache.org/job/Tajo-master-build/586/)
          TAJO-1353: CREATE TABLE should support the nested record definition. (hyunsik: rev 08f30345047ddf62dc51c33b9c350f08bf918a5c)

          • tajo-algebra/src/main/java/org/apache/tajo/algebra/DataTypeExpr.java
          • CHANGES
          • tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_1.sql
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_2.result
          • tajo-algebra/src/main/java/org/apache/tajo/algebra/ColumnDefinition.java
          • tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
          • tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_2.sql
          • tajo-core/src/test/java/org/apache/tajo/engine/parser/TestSQLAnalyzer.java
          • tajo-common/src/main/proto/DataTypes.proto
          • tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_1.result
          • tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4
          • tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4
          Show
          hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-build #586 (See https://builds.apache.org/job/Tajo-master-build/586/ ) TAJO-1353 : CREATE TABLE should support the nested record definition. (hyunsik: rev 08f30345047ddf62dc51c33b9c350f08bf918a5c) tajo-algebra/src/main/java/org/apache/tajo/algebra/DataTypeExpr.java CHANGES tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_1.sql tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_2.result tajo-algebra/src/main/java/org/apache/tajo/algebra/ColumnDefinition.java tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java tajo-core/src/test/resources/queries/TestSQLAnalyzer/create_table_nested_2.sql tajo-core/src/test/java/org/apache/tajo/engine/parser/TestSQLAnalyzer.java tajo-common/src/main/proto/DataTypes.proto tajo-core/src/test/resources/results/TestSQLAnalyzer/create_table_nested_1.result tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development