Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: Function/UDF
    • Labels:

      Description

      length(string) function returns the number of characters in string.

      int length(string)
      
      1. TAJO-308_1.patch
        4 kB
        Hyoungjun Kim
      2. TAJO-308_2.patch
        4 kB
        Hyoungjun Kim
      3. TAJO-308.patch
        4 kB
        Hyoungjun Kim

        Activity

        Hide
        hjkim Hyoungjun Kim added a comment -

        I've attached the patch. please review this patch.

        Show
        hjkim Hyoungjun Kim added a comment - I've attached the patch. please review this patch.
        Hide
        jihoonson Jihoon Son added a comment -

        This patch looks good to me, but I want to suggest a thing.
        When the input datum is NullDatum, how about return a zero instead of NullDatum?

        Show
        jihoonson Jihoon Son added a comment - This patch looks good to me, but I want to suggest a thing. When the input datum is NullDatum, how about return a zero instead of NullDatum?
        Hide
        hjkim Hyoungjun Kim added a comment -

        Thank you for the review.
        I've attached second patch that returns a zero if input datum is null.

        Show
        hjkim Hyoungjun Kim added a comment - Thank you for the review. I've attached second patch that returns a zero if input datum is null.
        Hide
        jihoonson Jihoon Son added a comment -

        +1.
        This patch passed 'mvn verify'.

        Show
        jihoonson Jihoon Son added a comment - +1. This patch passed 'mvn verify'.
        Hide
        hyunsik Hyunsik Choi added a comment -

        Please wait to commit. There is a missing requirement. Exactly, This function should
        return the length of the string str, measured in bytes.

        Show
        hyunsik Hyunsik Choi added a comment - Please wait to commit. There is a missing requirement. Exactly, This function should return the length of the string str, measured in bytes.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I'm on the road. After back home, I'll leave
        comments.

        Show
        hyunsik Hyunsik Choi added a comment - I'm on the road. After back home, I'll leave comments.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I checked the specification of length function. My above comment is wrong. I'm sorry for confusing. In the current postgresql, length function returns the number of characters regardless of whether string contains multi bytes characters or not.

        However, like other string functions, length function should return NULL when its input string is NULL. This semantic is the same on almost all SQL functions.

        Show
        hyunsik Hyunsik Choi added a comment - I checked the specification of length function. My above comment is wrong. I'm sorry for confusing. In the current postgresql, length function returns the number of characters regardless of whether string contains multi bytes characters or not. However, like other string functions, length function should return NULL when its input string is NULL. This semantic is the same on almost all SQL functions.
        Hide
        jihoonson Jihoon Son added a comment -

        Thanks for your comment, Hyunsik.
        The above comment was just my suggestion.
        I think that both ways are reasonable, but it will be better to return NULL when an input is null to maintain the compatibility of other string functions.
        So, I'm sorry to tell hyounjun, the first patch looks good for this issue.

        Show
        jihoonson Jihoon Son added a comment - Thanks for your comment, Hyunsik. The above comment was just my suggestion. I think that both ways are reasonable, but it will be better to return NULL when an input is null to maintain the compatibility of other string functions. So, I'm sorry to tell hyounjun, the first patch looks good for this issue.
        Hide
        hjkim Hyoungjun Kim added a comment -

        I've attached third patch that returns NullDatum if input is null.

        Show
        hjkim Hyoungjun Kim added a comment - I've attached third patch that returns NullDatum if input is null.
        Hide
        jihoonson Jihoon Son added a comment -

        Thanks, Hyoungjun.
        +1 for the latest patch.

        Show
        jihoonson Jihoon Son added a comment - Thanks, Hyoungjun. +1 for the latest patch.
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed it to master branch. Thanks all guys.

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

        SUCCESS: Integrated in Tajo-trunk-postcommit #569 (See https://builds.apache.org/job/Tajo-trunk-postcommit/569/)
        TAJO-308: Implement length(string) function. (hyoungjunkim via hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=1f80cd24b7d1005aa6874143c827bde72d55da84)

        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/function/string/Length.java
        • CHANGES.txt
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-trunk-postcommit #569 (See https://builds.apache.org/job/Tajo-trunk-postcommit/569/ ) TAJO-308 : Implement length(string) function. (hyoungjunkim via hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=1f80cd24b7d1005aa6874143c827bde72d55da84 ) tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/function/string/Length.java CHANGES.txt tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development