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.4.patch
        0.6 kB
        Mark Grover
      2. HIVE-3810.3.patch
        0.8 kB
        Mark Grover
      3. HIVE-3810.2.patch
        0.7 kB
        Mark Grover
      4. HIVE-3810.1.patch
        0.7 kB
        Mark Grover

        Activity

        qiangwang created issue -
        qiangwang made changes -
        Field Original Value New Value
        Summary HiveHistory need replace '\r' with space before writing Entry.value to historyfile HiveHistory.log need to replace '\r' with space before writing Entry.value to historyfile
        qiangwang made changes -
        Description HiveHistory.log will replace '\n' with space before writing Entry.value to history file:

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

        but HiveHistoryViewer 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 HiveHistoryViewer.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', ' ');
        + val = val.replaceAll("\r|\n", " ");
        or
        - val = val.replace('\n', ' ');
        + val = val.replace('\r', ' ').replace('\n', ' ');
        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', ' ');
        + val = val.replaceAll("\r|\n", " ");
        or
        - val = val.replace('\n', ' ');
        + val = val.replace('\r', ' ').replace('\n', ' ');
        qiangwang made changes -
        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', ' ');
        + val = val.replaceAll("\r|\n", " ");
        or
        - val = val.replace('\n', ' ');
        + val = val.replace('\r', ' ').replace('\n', ' ');
        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', ' ');
        Mark Grover made changes -
        Assignee Mark Grover [ mgrover ]
        Mark Grover made changes -
        Attachment HIVE-3810.1.patch [ 12561425 ]
        Mark Grover made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Mark Grover added a comment -

        Took care of mac line endings

        Show
        Mark Grover added a comment - Took care of mac line endings
        Mark Grover made changes -
        Attachment HIVE-3810.2.patch [ 12561433 ]
        qiangwang made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        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.
        Ashutosh Chauhan made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        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.
        Mark Grover made changes -
        Attachment HIVE-3810.3.patch [ 12567791 ]
        Mark Grover made changes -
        Status Reopened [ 4 ] Patch Available [ 10002 ]
        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.
        Mark Grover made changes -
        Attachment HIVE-3810.4.patch [ 12567792 ]
        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!
        Ashutosh Chauhan made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Fix Version/s 0.12.0 [ 12324312 ]
        Resolution Fixed [ 1 ]
        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.
        Ashutosh Chauhan made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development