Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: Expression, SQL Parser
    • Labels:

      Description

      Timestamp literal represents timestamp constant. It has the following bnf grammar.

      <timestamp literal>    ::=   TIMESTAMP <timestamp string>
      <timestamp string>    ::=   <quote> <unquoted timestamp string> <quote>
      <unquoted timestamp string>    ::=   <unquoted date string> <space> <unquoted time string>
      <unquoted date string>    ::=   <date value>
      <unquoted time string>    ::=   <time value> [ <time zone interval> ]
      <date value>    ::=   <years value> <minus sign> <months value> <minus sign> <days value>
      <time value>    ::=   <hours value> <colon> <minutes value> <colon> <seconds value>
      <time zone interval>    ::=   <sign> <hours value> <colon> <minutes value>
      
      1. TAJO-437_2.patch
        439 kB
        Hyunsik Choi
      2. TAJO-437_3.patch
        47 kB
        Hyunsik Choi
      3. TAJO-437.patch
        436 kB
        Hyunsik Choi

        Activity

        Hide
        hyunsik Hyunsik Choi added a comment -

        This patch mainly does as follows:

        • Add timestamp literal rule to SQLParser.g4 and SQLLexer.g4,
        • Add DateValue, TimeValue, TimestampLiteral to tajo-algebra,
        • Allow SQLAnalyzer and LogicalPlanner to handle them.
        • Add an unit test for Timestamp literal.

        In addition, this patch includes some refactorings:

        • Rename NullValue to NullLiteral
          • Later, the suffix 'Literal' will be only used to represent SQL literal.
          • The suffix 'Value' will be used for an internal value representation.
        Show
        hyunsik Hyunsik Choi added a comment - This patch mainly does as follows: Add timestamp literal rule to SQLParser.g4 and SQLLexer.g4, Add DateValue, TimeValue, TimestampLiteral to tajo-algebra, Allow SQLAnalyzer and LogicalPlanner to handle them. Add an unit test for Timestamp literal. In addition, this patch includes some refactorings: Rename NullValue to NullLiteral Later, the suffix 'Literal' will be only used to represent SQL literal. The suffix 'Value' will be used for an internal value representation.
        Hide
        charsyam DaeMyung Kang added a comment -

        [Hyunsik Choi first of all, you should add license to files for build.

        tajo-algebra/src/main/java/org/apache/tajo/algebra/DateValue.java
        tajo-algebra/src/main/java/org/apache/tajo/algebra/TimeValue.java

        after adding license.

        "mvn clean install" is ok.

        Show
        charsyam DaeMyung Kang added a comment - [ Hyunsik Choi first of all, you should add license to files for build. tajo-algebra/src/main/java/org/apache/tajo/algebra/DateValue.java tajo-algebra/src/main/java/org/apache/tajo/algebra/TimeValue.java after adding license. "mvn clean install" is ok.
        Hide
        charsyam DaeMyung Kang added a comment - - edited

        Hyunsik Choi and second, serialize function in tajo-common/src/main/java/org/apache/tajo/json/DatumAdapter.java

        I think you should add break statement after

        case DATE:
        jsonObj.addProperty("value", src.asInt4());
        break; <--- need to add this;

        default:
        jsonObj.add("body", context.serialize(src));
        break; <-- add break even though it works same.

        Show
        charsyam DaeMyung Kang added a comment - - edited Hyunsik Choi and second, serialize function in tajo-common/src/main/java/org/apache/tajo/json/DatumAdapter.java I think you should add break statement after case DATE: jsonObj.addProperty("value", src.asInt4()); break; <--- need to add this; default: jsonObj.add("body", context.serialize(src)); break; <-- add break even though it works same.
        Hide
        hyunsik Hyunsik Choi added a comment -

        Thank you for the review. I uploaded the second patch which reflects your comments.

        Show
        hyunsik Hyunsik Choi added a comment - Thank you for the review. I uploaded the second patch which reflects your comments.
        Hide
        blrunner Jaehwa Jung added a comment -

        It looks great.
        But, I'm not sure why it includes 'jquery-min.js'. Could you explain about it?

        Show
        blrunner Jaehwa Jung added a comment - It looks great. But, I'm not sure why it includes 'jquery-min.js'. Could you explain about it?
        Hide
        hyunsik Hyunsik Choi added a comment -

        I uploaded the updated patch.

        Show
        hyunsik Hyunsik Choi added a comment - I uploaded the updated patch.
        Hide
        blrunner Jaehwa Jung added a comment -

        +1 for the patch.
        'mvn clean install -Phcatalog-0.12.0' finished successfully.
        Ship it.

        Show
        blrunner Jaehwa Jung added a comment - +1 for the patch. 'mvn clean install -Phcatalog-0.12.0' finished successfully. Ship it.
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed it to master. Thanks!

        Show
        hyunsik Hyunsik Choi added a comment - committed it to master. Thanks!
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-trunk-postcommit #635 (See https://builds.apache.org/job/Tajo-trunk-postcommit/635/)
        TAJO-437: Timestamp literal support. (hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=4319ded3aa974dfca35fd2630b97b8f2601507c4)

        • tajo-core/tajo-core-backend/src/test/queries/complex_union_1.sql
        • tajo-common/src/main/java/org/apache/tajo/json/PathDeserializer.java
        • tajo-algebra/src/main/java/org/apache/tajo/algebra/NullValue.java
        • tajo-common/src/main/java/org/apache/tajo/json/DateDatumAdapter.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
        • tajo-algebra/src/main/java/org/apache/tajo/algebra/TimeValue.java
        • tajo-algebra/src/main/java/org/apache/tajo/algebra/NullLiteral.java
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLDateTimeTypes.java
        • tajo-common/src/main/java/org/apache/tajo/json/TimestampDatumAdapter.java
        • tajo-algebra/src/main/java/org/apache/tajo/algebra/TimestampLiteral.java
        • tajo-algebra/src/main/java/org/apache/tajo/algebra/OpType.java
        • tajo-common/src/main/java/org/apache/tajo/json/GsonObject.java
        • tajo-common/src/main/java/org/apache/tajo/json/ClassNameDeserializer.java
        • CHANGES.txt
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/BaseAlgebraVisitor.java
        • tajo-common/src/main/java/org/apache/tajo/datum/TimestampDatum.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/HiveConverter.java
        • tajo-algebra/src/main/java/org/apache/tajo/algebra/DateValue.java
        • tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/AlgebraVisitor.java
        • tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4
        • tajo-common/src/main/java/org/apache/tajo/json/CommonGsonHelper.java
        • tajo-common/src/main/java/org/apache/tajo/json/TimeDatumAdapter.java
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/ExprTestBase.java
        • tajo-common/src/main/java/org/apache/tajo/json/PathSerializer.java
        • tajo-common/src/main/java/org/apache/tajo/json/DatumAdapter.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-trunk-postcommit #635 (See https://builds.apache.org/job/Tajo-trunk-postcommit/635/ ) TAJO-437 : Timestamp literal support. (hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=4319ded3aa974dfca35fd2630b97b8f2601507c4 ) tajo-core/tajo-core-backend/src/test/queries/complex_union_1.sql tajo-common/src/main/java/org/apache/tajo/json/PathDeserializer.java tajo-algebra/src/main/java/org/apache/tajo/algebra/NullValue.java tajo-common/src/main/java/org/apache/tajo/json/DateDatumAdapter.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java tajo-algebra/src/main/java/org/apache/tajo/algebra/TimeValue.java tajo-algebra/src/main/java/org/apache/tajo/algebra/NullLiteral.java tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLDateTimeTypes.java tajo-common/src/main/java/org/apache/tajo/json/TimestampDatumAdapter.java tajo-algebra/src/main/java/org/apache/tajo/algebra/TimestampLiteral.java tajo-algebra/src/main/java/org/apache/tajo/algebra/OpType.java tajo-common/src/main/java/org/apache/tajo/json/GsonObject.java tajo-common/src/main/java/org/apache/tajo/json/ClassNameDeserializer.java CHANGES.txt tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/BaseAlgebraVisitor.java tajo-common/src/main/java/org/apache/tajo/datum/TimestampDatum.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/HiveConverter.java tajo-algebra/src/main/java/org/apache/tajo/algebra/DateValue.java tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/AlgebraVisitor.java tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4 tajo-common/src/main/java/org/apache/tajo/json/CommonGsonHelper.java tajo-common/src/main/java/org/apache/tajo/json/TimeDatumAdapter.java tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/ExprTestBase.java tajo-common/src/main/java/org/apache/tajo/json/PathSerializer.java tajo-common/src/main/java/org/apache/tajo/json/DatumAdapter.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
        Hide
        jihoonson Jihoon Son added a comment -

        +1.
        This patch looks good to me.

        Show
        jihoonson Jihoon Son added a comment - +1. This patch looks good to me.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development