Uploaded image for project: 'Hive'
  1. Hive
  2. HIVE-15792

Hive should raise SemanticException when LPAD/RPAD pad character's length is 0

    Details

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

      Description

      For example SELECT LPAD('A', 2, ''); will cause an infinite loop and the running query will hang without any error.

      It would be great if this could be prevented by checking the pad character's length and if it's 0 then throw a SemanticException.

      1. HIVE-15792.002.patch
        8 kB
        Nanda kumar
      2. HIVE-15792.001.patch
        6 kB
        Nanda kumar
      3. HIVE-15792.000.patch
        4 kB
        Nanda kumar

        Activity

        Hide
        leftylev Lefty Leverenz added a comment -

        Thanks for the documentation, Nanda kumar. Looks good.

        Show
        leftylev Lefty Leverenz added a comment - Thanks for the documentation, Nanda kumar . Looks good.
        Show
        nandakumar131 Nanda kumar added a comment - Wiki has been updated https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF
        Hide
        nandakumar131 Nanda kumar added a comment -

        Sure, Thejas M Nair.
        I don't have write access to wiki. Requested access in user mailing list.
        Will update it, once the access is provided.

        Show
        nandakumar131 Nanda kumar added a comment - Sure, Thejas M Nair . I don't have write access to wiki. Requested access in user mailing list. Will update it, once the access is provided.
        Hide
        thejas Thejas M Nair added a comment -

        Nanda kumar Can you also please update the information in the wiki ?
        cc Lefty Leverenz

        Show
        thejas Thejas M Nair added a comment - Nanda kumar Can you also please update the information in the wiki ? cc Lefty Leverenz
        Hide
        nandakumar131 Nanda kumar added a comment -

        Thanks Thejas M Nair for the review and commit.

        Show
        nandakumar131 Nanda kumar added a comment - Thanks Thejas M Nair for the review and commit.
        Hide
        thejas Thejas M Nair added a comment -

        Patch committed to trunk.
        Thanks Nanda kumar!

        Show
        thejas Thejas M Nair added a comment - Patch committed to trunk. Thanks Nanda kumar !
        Hide
        thejas Thejas M Nair added a comment -

        +1 to updated patch.
        The failed tests are tracked under HIVE-15058

        Show
        thejas Thejas M Nair added a comment - +1 to updated patch. The failed tests are tracked under HIVE-15058
        Hide
        nandakumar131 Nanda kumar added a comment -

        Note: Test case failures are not related to the patch

        Show
        nandakumar131 Nanda kumar added a comment - Note: Test case failures are not related to the patch
        Hide
        hiveqa Hive QA added a comment -

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12851728/HIVE-15792.002.patch

        SUCCESS: +1 due to 2 test(s) being added or modified.

        ERROR: -1 due to 4 failed/errored test(s), 10242 tests executed
        Failed tests:

        TestDerbyConnector - did not produce a TEST-*.xml file (likely timed out) (batchId=235)
        org.apache.hadoop.hive.cli.TestEncryptedHDFSCliDriver.testCliDriver[encryption_join_with_different_encryption_keys] (batchId=159)
        org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=223)
        org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query23] (batchId=223)
        

        Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/3446/testReport
        Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/3446/console
        Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-3446/

        Messages:

        Executing org.apache.hive.ptest.execution.TestCheckPhase
        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        Tests exited with: TestsFailedException: 4 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12851728 - PreCommit-HIVE-Build

        Show
        hiveqa Hive QA added a comment - Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12851728/HIVE-15792.002.patch SUCCESS: +1 due to 2 test(s) being added or modified. ERROR: -1 due to 4 failed/errored test(s), 10242 tests executed Failed tests: TestDerbyConnector - did not produce a TEST-*.xml file (likely timed out) (batchId=235) org.apache.hadoop.hive.cli.TestEncryptedHDFSCliDriver.testCliDriver[encryption_join_with_different_encryption_keys] (batchId=159) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=223) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query23] (batchId=223) Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/3446/testReport Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/3446/console Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-3446/ Messages: Executing org.apache.hive.ptest.execution.TestCheckPhase Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 4 tests failed This message is automatically generated. ATTACHMENT ID: 12851728 - PreCommit-HIVE-Build
        Hide
        nandakumar131 Nanda kumar added a comment -

        Uploaded a new patch with fix for TestCliDriver (lpad & rpad) failure

        Show
        nandakumar131 Nanda kumar added a comment - Uploaded a new patch with fix for TestCliDriver (lpad & rpad) failure
        Hide
        thejas Thejas M Nair added a comment -

        Looks like tests need to be updated as well.

        Show
        thejas Thejas M Nair added a comment - Looks like tests need to be updated as well.
        Hide
        hiveqa Hive QA added a comment -

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12851664/HIVE-15792.001.patch

        SUCCESS: +1 due to 2 test(s) being added or modified.

        ERROR: -1 due to 5 failed/errored test(s), 10242 tests executed
        Failed tests:

        TestDerbyConnector - did not produce a TEST-*.xml file (likely timed out) (batchId=235)
        org.apache.hadoop.hive.cli.TestCliDriver.testCliDriver[udf_lpad] (batchId=33)
        org.apache.hadoop.hive.cli.TestCliDriver.testCliDriver[udf_rpad] (batchId=82)
        org.apache.hadoop.hive.cli.TestEncryptedHDFSCliDriver.testCliDriver[encryption_join_with_different_encryption_keys] (batchId=159)
        org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=223)
        

        Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/3443/testReport
        Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/3443/console
        Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-3443/

        Messages:

        Executing org.apache.hive.ptest.execution.TestCheckPhase
        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        Tests exited with: TestsFailedException: 5 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12851664 - PreCommit-HIVE-Build

        Show
        hiveqa Hive QA added a comment - Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12851664/HIVE-15792.001.patch SUCCESS: +1 due to 2 test(s) being added or modified. ERROR: -1 due to 5 failed/errored test(s), 10242 tests executed Failed tests: TestDerbyConnector - did not produce a TEST-*.xml file (likely timed out) (batchId=235) org.apache.hadoop.hive.cli.TestCliDriver.testCliDriver[udf_lpad] (batchId=33) org.apache.hadoop.hive.cli.TestCliDriver.testCliDriver[udf_rpad] (batchId=82) org.apache.hadoop.hive.cli.TestEncryptedHDFSCliDriver.testCliDriver[encryption_join_with_different_encryption_keys] (batchId=159) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=223) Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/3443/testReport Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/3443/console Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-3443/ Messages: Executing org.apache.hive.ptest.execution.TestCheckPhase Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 5 tests failed This message is automatically generated. ATTACHMENT ID: 12851664 - PreCommit-HIVE-Build
        Hide
        thejas Thejas M Nair added a comment -

        Thanks for the update. +1

        Show
        thejas Thejas M Nair added a comment - Thanks for the update. +1
        Hide
        nandakumar131 Nanda kumar added a comment -

        Thanks for the review Thejas M Nair.
        Updated the description of functions in new patch which describes the behavior on getting empty pad string as argument

        Show
        nandakumar131 Nanda kumar added a comment - Thanks for the review Thejas M Nair . Updated the description of functions in new patch which describes the behavior on getting empty pad string as argument
        Hide
        thejas Thejas M Nair added a comment -

        Nanda kumar can you also update the description (the extended part) of these functions (the @Description tag) to talk about the behavior on getting empty pad string ?

        Show
        thejas Thejas M Nair added a comment - Nanda kumar can you also update the description (the extended part) of these functions (the @Description tag) to talk about the behavior on getting empty pad string ?
        Hide
        thejas Thejas M Nair added a comment -

        +1

        Show
        thejas Thejas M Nair added a comment - +1
        Hide
        hiveqa Hive QA added a comment -

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12851087/HIVE-15792.000.patch

        SUCCESS: +1 due to 2 test(s) being added or modified.

        ERROR: -1 due to 4 failed/errored test(s), 10241 tests executed
        Failed tests:

        TestDerbyConnector - did not produce a TEST-*.xml file (likely timed out) (batchId=235)
        org.apache.hadoop.hive.cli.TestEncryptedHDFSCliDriver.testCliDriver[encryption_join_with_different_encryption_keys] (batchId=159)
        org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=223)
        org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query23] (batchId=223)
        

        Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/3435/testReport
        Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/3435/console
        Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-3435/

        Messages:

        Executing org.apache.hive.ptest.execution.TestCheckPhase
        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        Tests exited with: TestsFailedException: 4 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12851087 - PreCommit-HIVE-Build

        Show
        hiveqa Hive QA added a comment - Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12851087/HIVE-15792.000.patch SUCCESS: +1 due to 2 test(s) being added or modified. ERROR: -1 due to 4 failed/errored test(s), 10241 tests executed Failed tests: TestDerbyConnector - did not produce a TEST-*.xml file (likely timed out) (batchId=235) org.apache.hadoop.hive.cli.TestEncryptedHDFSCliDriver.testCliDriver[encryption_join_with_different_encryption_keys] (batchId=159) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=223) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query23] (batchId=223) Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/3435/testReport Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/3435/console Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-3435/ Messages: Executing org.apache.hive.ptest.execution.TestCheckPhase Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 4 tests failed This message is automatically generated. ATTACHMENT ID: 12851087 - PreCommit-HIVE-Build
        Hide
        nandakumar131 Nanda kumar added a comment -

        Thanks Thejas M Nair

        As suggested the check is added in GenericUDFBasePad, which will now return null if the pad string is empty.
        Please review the patch.

        Show
        nandakumar131 Nanda kumar added a comment - Thanks Thejas M Nair As suggested the check is added in GenericUDFBasePad , which will now return null if the pad string is empty. Please review the patch.
        Hide
        thejas Thejas M Nair added a comment -

        Thanks Nanda kumar
        I think NULL is the better choice, that way we don't return unpadded data.
        NULL makes sense with invalid input.

        Show
        thejas Thejas M Nair added a comment - Thanks Nanda kumar I think NULL is the better choice, that way we don't return unpadded data. NULL makes sense with invalid input.
        Hide
        nandakumar131 Nanda kumar added a comment -

        Following are the results from different databases for LPAD/PPAD with null
        SELECT LPAD('x', 5, '')

        Database Output
        Oracle NULL
        MySQL NULL
        Postgres <value unmodified>
        Show
        nandakumar131 Nanda kumar added a comment - Following are the results from different databases for LPAD/PPAD with null SELECT LPAD('x', 5, '') Database Output Oracle NULL MySQL NULL Postgres <value unmodified>
        Hide
        thejas Thejas M Nair added a comment -

        There are couple of options here. It would be better to follow the convention followed by some of the other databases, like postgres and mysql with this regard. Can you take a look at what they do ?
        Its cleaner to have this check in the UDF itself rather than in semantic analyzer.

        Show
        thejas Thejas M Nair added a comment - There are couple of options here. It would be better to follow the convention followed by some of the other databases, like postgres and mysql with this regard. Can you take a look at what they do ? Its cleaner to have this check in the UDF itself rather than in semantic analyzer.
        Hide
        nandakumar131 Nanda kumar added a comment -

        Thejas M Nair, Is it ok to throw SemanticException in this case or can we return the value unmodified, since there is nothing to pad?

        Show
        nandakumar131 Nanda kumar added a comment - Thejas M Nair , Is it ok to throw SemanticException in this case or can we return the value unmodified, since there is nothing to pad?

          People

          • Assignee:
            nandakumar131 Nanda kumar
            Reporter:
            zchovan Zoltan Chovan
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development