Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: function/udf
    • Labels:

      Description

      hex("012") ---> 303132
      hex(10) ---> a

      not support float and text

      1. TAJO-346.2.patch
        6 kB
        DaeMyung Kang
      2. TAJO-346.3.patch
        6 kB
        DaeMyung Kang
      3. TAJO-346.patch
        6 kB
        DaeMyung Kang

        Activity

        Hide
        DaeMyung Kang added a comment -

        Please review this

        It follows mysql result. and it represents with lower cases.

        Show
        DaeMyung Kang added a comment - Please review this It follows mysql result. and it represents with lower cases.
        Hide
        Hyunsik Choi added a comment -

        The patch looks good for me. For non-standard parts, Tajo mostly follows PostgreSQL. In PostgreSQL, to_hex() is equivalence of hex in MySQL. You can find the function details at http://www.postgresql.org/docs/9.1/static/functions-string.html.

        For consistency, could you please rename hex() to to_hex()?

        Thanks!

        Show
        Hyunsik Choi added a comment - The patch looks good for me. For non-standard parts, Tajo mostly follows PostgreSQL. In PostgreSQL, to_hex() is equivalence of hex in MySQL. You can find the function details at http://www.postgresql.org/docs/9.1/static/functions-string.html . For consistency, could you please rename hex() to to_hex()? Thanks!
        Hide
        DaeMyung Kang added a comment -

        I changed hex function name to to_hex for consistency.

        Thanks.

        Show
        DaeMyung Kang added a comment - I changed hex function name to to_hex for consistency. Thanks.
        Hide
        Hyunsik Choi added a comment -

        I missed one more thing. This
        function internally evaluates real values (i.e., float4 and float8) and integer types.

        Tajo follow a strong typed system. Through out the system, we assume that parameters are passed to a function with specified typed values defined in the function.

        Instead, we have a plan to implement auto casting mechanism for user convenience.

        So, I would like to suggest that this function evaluates only int8 type values.

        Show
        Hyunsik Choi added a comment - I missed one more thing. This function internally evaluates real values (i.e., float4 and float8) and integer types. Tajo follow a strong typed system. Through out the system, we assume that parameters are passed to a function with specified typed values defined in the function. Instead, we have a plan to implement auto casting mechanism for user convenience. So, I would like to suggest that this function evaluates only int8 type values.
        Hide
        DaeMyung Kang added a comment -

        Hyunsik Choi yes. I agree with you. in postgres, to_hex just supports integer type. so I will patch soon. Thank you for your advice.

        Show
        DaeMyung Kang added a comment - Hyunsik Choi yes. I agree with you. in postgres, to_hex just supports integer type. so I will patch soon. Thank you for your advice.
        Hide
        DaeMyung Kang added a comment -

        Hyunsik Choi hi, I have a question. in postgres spec, to_hex can accept Integer and BigInt.

        so should I support bigInt also?, I think it is not difficult to support. Thank you.

        Show
        DaeMyung Kang added a comment - Hyunsik Choi hi, I have a question. in postgres spec, to_hex can accept Integer and BigInt. so should I support bigInt also?, I think it is not difficult to support. Thank you.
        Hide
        Hyunsik Choi added a comment -

        I think that this function should only support bigint. I'll complete auto casting mechanism sooner. Thank you!

        Show
        Hyunsik Choi added a comment - I think that this function should only support bigint. I'll complete auto casting mechanism sooner. Thank you!
        Hide
        DaeMyung Kang added a comment -

        I patched SQLAnalyzer.java to separate INT4 and INT8, please review this part seriously. Thanks.

        Show
        DaeMyung Kang added a comment - I patched SQLAnalyzer.java to separate INT4 and INT8, please review this part seriously. Thanks.
        Hide
        Hyunsik Choi added a comment -

        +1

        The patch looks good to me.

        Show
        Hyunsik Choi added a comment - +1 The patch looks good to me.
        Hide
        Hyunsik Choi added a comment -

        For the name consistency, I renamed HexFunc to ToHex. Then, I committed it to master branch. Thank you for your contribution.

        Show
        Hyunsik Choi added a comment - For the name consistency, I renamed HexFunc to ToHex. Then, I committed it to master branch. Thank you for your contribution.
        Hide
        DaeMyung Kang added a comment -

        I updated description because of float and text

        Show
        DaeMyung Kang added a comment - I updated description because of float and text
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Tajo-trunk-postcommit #581 (See https://builds.apache.org/job/Tajo-trunk-postcommit/581/)
        TAJO-346: Implement hex function. (DaeMyung Kang via hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=778c01f8f71ea6dd4573e83c5b62ccefcd39b337)

        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/function/string/ToHex.java
        • CHANGES.txt
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Tajo-trunk-postcommit #581 (See https://builds.apache.org/job/Tajo-trunk-postcommit/581/ ) TAJO-346 : Implement hex function. (DaeMyung Kang via hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=778c01f8f71ea6dd4573e83c5b62ccefcd39b337 ) tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/function/string/ToHex.java CHANGES.txt tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development