Hive
  1. Hive
  2. HIVE-3810

HiveHistory.log need to replace '\r' with space before writing Entry.value to historyfile

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0
    • Component/s: Logging
    • Labels:
      None

      Description

      HiveHistory.log will replace '\n' with space before writing Entry.value to history file:

      val = val.replace('\n', ' ');

      but HiveHistory.parseHiveHistory use BufferedReader.readLine which takes '\n', '\r', '\r\n' as line delimiter to parse history file

      if val contains '\r', there is a high possibility that HiveHistory.parseLine will fail, in which case usually RecordTypes.valueOf(recType) will throw exception 'java.lang.IllegalArgumentException'

      HiveHistory.log need to replace '\r' with space as well:

      val = val.replace('\n', ' ');

      changed to

      val = val.replaceAll("\r|\n", " ");

      or

      val = val.replace('\r', ' ').replace('\n', ' ');

      1. HIVE-3810.1.patch
        0.7 kB
        Mark Grover
      2. HIVE-3810.2.patch
        0.7 kB
        Mark Grover
      3. HIVE-3810.3.patch
        0.8 kB
        Mark Grover
      4. HIVE-3810.4.patch
        0.6 kB
        Mark Grover

        Activity

        Hide
        Mark Grover added a comment -

        Took care of mac line endings

        Show
        Mark Grover added a comment - Took care of mac line endings
        Hide
        Mark Grover added a comment -

        qiangwang I see the status got resolved to fix. Did this get committed?

        Show
        Mark Grover added a comment - qiangwang I see the status got resolved to fix. Did this get committed?
        Hide
        Ashutosh Chauhan added a comment -

        I dont see this in svn commit log. Reopening for review.

        Show
        Ashutosh Chauhan added a comment - I dont see this in svn commit log. Reopening for review.
        Hide
        Mark Grover added a comment -
        Show
        Mark Grover added a comment - Review at https://reviews.apache.org/r/9129/
        Hide
        Edward Capriolo added a comment -

        Doesn't java.io.File have constants to detect the line terminiator per OS. can we possibly use that?

        Show
        Edward Capriolo added a comment - Doesn't java.io.File have constants to detect the line terminiator per OS. can we possibly use that?
        Hide
        Mark Grover added a comment -

        Thanks, Edward for the review!
        Your comment made me think about using System.getProperty("line.separator") instead.

        Let me post a new patch with that. I am not entirely sure how to test it (apart from running the unit tests). If you have any ideas, please let me know.

        Show
        Mark Grover added a comment - Thanks, Edward for the review! Your comment made me think about using System.getProperty("line.separator") instead. Let me post a new patch with that. I am not entirely sure how to test it (apart from running the unit tests). If you have any ideas, please let me know.
        Hide
        Mark Grover added a comment -

        My previous patch (#3) was accidently based off of patch 2 instead of being based off of trunk. Corrected that in patch 4.

        Show
        Mark Grover added a comment - My previous patch (#3) was accidently based off of patch 2 instead of being based off of trunk. Corrected that in patch 4.
        Hide
        Ashutosh Chauhan added a comment -

        +1

        Show
        Ashutosh Chauhan added a comment - +1
        Hide
        Ashutosh Chauhan added a comment -

        Committed to trunk. Thanks, Mark!

        Show
        Ashutosh Chauhan added a comment - Committed to trunk. Thanks, Mark!
        Hide
        Hudson added a comment -

        Integrated in Hive-trunk-hadoop1-ptest #47 (See https://builds.apache.org/job/Hive-trunk-hadoop1-ptest/47/)
        HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (Revision 1500991)

        Result = FAILURE
        hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991
        Files :

        • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Show
        Hudson added a comment - Integrated in Hive-trunk-hadoop1-ptest #47 (See https://builds.apache.org/job/Hive-trunk-hadoop1-ptest/47/ ) HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (Revision 1500991) Result = FAILURE hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991 Files : /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Hide
        Hudson added a comment -

        Integrated in Hive-trunk-hadoop2 #279 (See https://builds.apache.org/job/Hive-trunk-hadoop2/279/)
        HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (Revision 1500991)

        Result = ABORTED
        hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991
        Files :

        • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Show
        Hudson added a comment - Integrated in Hive-trunk-hadoop2 #279 (See https://builds.apache.org/job/Hive-trunk-hadoop2/279/ ) HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (Revision 1500991) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991 Files : /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Hide
        Hudson added a comment -

        Integrated in Hive-trunk-h0.21 #2188 (See https://builds.apache.org/job/Hive-trunk-h0.21/2188/)
        HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (Revision 1500991)

        Result = ABORTED
        hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991
        Files :

        • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Show
        Hudson added a comment - Integrated in Hive-trunk-h0.21 #2188 (See https://builds.apache.org/job/Hive-trunk-h0.21/2188/ ) HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (Revision 1500991) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991 Files : /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hive-trunk-hadoop2-ptest #14 (See https://builds.apache.org/job/Hive-trunk-hadoop2-ptest/14/)
        HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991)

        • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hive-trunk-hadoop2-ptest #14 (See https://builds.apache.org/job/Hive-trunk-hadoop2-ptest/14/ ) HIVE-3810 : HiveHistory.log need to replace \r with space before writing Entry.value to historyfile (Mark Grover via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1500991 ) /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
        Hide
        Ashutosh Chauhan added a comment -

        This issue has been fixed and released as part of 0.12 release. If you find further issues, please create a new jira and link it to this one.

        Show
        Ashutosh Chauhan added a comment - This issue has been fixed and released as part of 0.12 release. If you find further issues, please create a new jira and link it to this one.

          People

          • Assignee:
            Mark Grover
            Reporter:
            qiangwang
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development