Details

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

      Description

      Plz assign to me.

      1. TAJO-377_1.patch
        4 kB
        Seungun Choe
      2. TAJO-377_2.patch
        4 kB
        Seungun Choe
      3. TAJO-377_3.patch
        4 kB
        Seungun Choe
      4. TAJO-377_4.patch
        4 kB
        Seungun Choe
      5. TAJO-377_5.patch
        4 kB
        Seungun Choe
      6. TAJO-377.patch
        1 kB
        Seungun Choe

        Issue Links

          Activity

          Hide
          blrunner Jaehwa Jung added a comment -

          I assigned to you.
          Thank you for your contribution.

          Show
          blrunner Jaehwa Jung added a comment - I assigned to you. Thank you for your contribution.
          Hide
          seungunchoe Seungun Choe added a comment -

          I'm happy to solve this issue.
          This issue is so simple.
          But Tajo have some dependency issues, so too much delayed.
          Please confirm my patch.

          Show
          seungunchoe Seungun Choe added a comment - I'm happy to solve this issue. This issue is so simple. But Tajo have some dependency issues, so too much delayed. Please confirm my patch.
          Hide
          seungunchoe Seungun Choe added a comment -

          TAJO-377.patch does not import Concat.java file.

          Show
          seungunchoe Seungun Choe added a comment - TAJO-377 .patch does not import Concat.java file.
          Hide
          blrunner Jaehwa Jung added a comment - - edited

          Hi, Seungun Choe.

          Unfortunately, this patch failed to pass 'mvn clean install' as follows:

          Tests in error: 
            testConcat(org.apache.tajo.engine.function.TestStringOperatorsAndFunctions): function concat(text) does not exist
          

          And I recommend to use StringBuilder at Concat.
          Please refer my comments at TAJO-378.

          Show
          blrunner Jaehwa Jung added a comment - - edited Hi, Seungun Choe . Unfortunately, this patch failed to pass 'mvn clean install' as follows: Tests in error: testConcat(org.apache.tajo.engine.function.TestStringOperatorsAndFunctions): function concat(text) does not exist And I recommend to use StringBuilder at Concat . Please refer my comments at TAJO-378 .
          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/12632444/TAJO-377_2.patch
          against master revision f4a9fd1.

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/178//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/178//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/178//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/12632444/TAJO-377_2.patch against master revision f4a9fd1. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 184 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core/tajo-core-backend. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/178//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/178//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/178//console This message is automatically generated.
          Hide
          seungunchoe Seungun Choe added a comment -

          I modified from String type to Stringbuffer type.
          Type means concat function's return values intermediate step.

          Show
          seungunchoe Seungun Choe added a comment - I modified from String type to Stringbuffer type. Type means concat function's return values intermediate step.
          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/12632741/TAJO-377_4.patch
          against master revision e0b6f7a.

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/184//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/184//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/184//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/12632741/TAJO-377_4.patch against master revision e0b6f7a. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 184 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core/tajo-core-backend. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/184//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/184//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/184//console This message is automatically generated.
          Hide
          hyunsik Hyunsik Choi added a comment -

          I'm sorry for late review. The patch looks good to me.

          There is one suggestion. StringBuffer is synchronized, but StringBuilder is not. This function is not invoked concurrently. Consequently, StringBuffer will cause some unnecessary overheads while processing lots of rows. Could you replace StringBuffer with StringBuilder? Then, I'll review it quickly.

          Show
          hyunsik Hyunsik Choi added a comment - I'm sorry for late review. The patch looks good to me. There is one suggestion. StringBuffer is synchronized, but StringBuilder is not. This function is not invoked concurrently. Consequently, StringBuffer will cause some unnecessary overheads while processing lots of rows. Could you replace StringBuffer with StringBuilder? Then, I'll review it quickly.
          Hide
          seungunchoe Seungun Choe added a comment -

          Here is new patch file that is replaced StringBuffer with StringBuilder.

          Show
          seungunchoe Seungun Choe added a comment - Here is new patch file that is replaced StringBuffer with StringBuilder.
          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/12636278/TAJO-377_5.patch
          against master revision 7283c58.

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/247//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/247//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/247//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/12636278/TAJO-377_5.patch against master revision 7283c58. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 184 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core/tajo-core-backend. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/247//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/247//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/247//console This message is automatically generated.
          Hide
          blrunner Jaehwa Jung added a comment -

          +1

          Thank you for your contribution.
          I'll commit it at once.

          Show
          blrunner Jaehwa Jung added a comment - +1 Thank you for your contribution. I'll commit it at once.
          Hide
          blrunner Jaehwa Jung added a comment -

          I've just committed it to the master branch and the 0.8.0 branch.

          Show
          blrunner Jaehwa Jung added a comment - I've just committed it to the master branch and the 0.8.0 branch.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-master-build #141 (See https://builds.apache.org/job/Tajo-master-build/141/)
          TAJO-377: Implement concat function (Seungun Choe via jaehwa) (jhjung: rev 4e6d3cbab8fe714d70969d7f0e13d6c1a938547c)

          • 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/function/string/Concat.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #141 (See https://builds.apache.org/job/Tajo-master-build/141/ ) TAJO-377 : Implement concat function (Seungun Choe via jaehwa) (jhjung: rev 4e6d3cbab8fe714d70969d7f0e13d6c1a938547c) 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/function/string/Concat.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-0.8.0-build #46 (See https://builds.apache.org/job/Tajo-0.8.0-build/46/)
          TAJO-377: Implement concat function (Seungun Choe via jaehwa) (jhjung: rev 3e68d271d2255309c58496a88969c385f746ff58)

          • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/function/string/Concat.java
          • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
          • CHANGES.txt
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-0.8.0-build #46 (See https://builds.apache.org/job/Tajo-0.8.0-build/46/ ) TAJO-377 : Implement concat function (Seungun Choe via jaehwa) (jhjung: rev 3e68d271d2255309c58496a88969c385f746ff58) tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/function/string/Concat.java tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java CHANGES.txt

            People

            • Assignee:
              seungunchoe Seungun Choe
              Reporter:
              seungunchoe Seungun Choe
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development