You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "zhangbutao (via GitHub)" <gi...@apache.org> on 2026/10/12 16:54:34 UTC
[PR] HIVE-30156: Fix data corruption when using multi-byte line.delim with text format tables [hive]
zhangbutao opened a new pull request, #6866:
URL: https://github.com/apache/hive/pull/6866
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducible example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30156: Fix data corruption when using multi-byte line.delim with text format tables [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6866:
URL: https://github.com/apache/hive/pull/6866#issuecomment-6130179008
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6866) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6866&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6866&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6866&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6866&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6866&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6866)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30156: Fix data corruption when using multi-byte line.delim with text format tables [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6866:
URL: https://github.com/apache/hive/pull/6866#discussion_r4250573157
##########
ql/src/java/org/apache/hadoop/hive/ql/io/HiveIgnoreKeyTextOutputFormat.java:
##########
@@ -67,16 +68,11 @@ public class HiveIgnoreKeyTextOutputFormat<K extends WritableComparable, V exten
public RecordWriter getHiveRecordWriter(JobConf jc, Path outPath,
Class<? extends Writable> valueClass, boolean isCompressed,
Properties tableProperties, Progressable progress) throws IOException {
- int rowSeparator = 0;
String rowSeparatorString = tableProperties.getProperty(
serdeConstants.LINE_DELIM, "\n");
- try {
- rowSeparator = Byte.parseByte(rowSeparatorString);
- } catch (NumberFormatException e) {
- rowSeparator = rowSeparatorString.charAt(0);
- }
+ final byte[] rowSeparatorBytes =
+ rowSeparatorString.getBytes(StandardCharsets.UTF_8);
Review Comment:
This changes the historical semantics of `line.delim` when it is configured as a numeric byte value (e.g., \"10\"): previously it would be parsed to a single byte (LF), but now it will write the UTF-8 bytes for the characters '1' and '0'. This is a behavior regression for existing tables that use numeric delimiters. Consider preserving backward compatibility by detecting purely-numeric values and converting them to a single byte (as before), while still supporting multi-byte delimiters for non-numeric strings. Also consider defining what should happen if `line.delim` is the empty string; currently it results in writing no row separator bytes, which can concatenate records.
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java:
##########
@@ -141,6 +142,18 @@ public FetchOperator(FetchWork work, JobConf job, Operator<?> operator,
this.job = job;
this.work = work;
this.operator = operator;
+ // When the table uses a multi-byte line.delim, propagate it to
+ // Hadoop's TextInputFormat so that LineRecordReader splits records
+ // on the full delimiter sequence instead of the default '\n'.
+ // FetchTask bypasses HiveInputFormat, so we must set it here.
+ if (work.getTblDesc() != null) {
+ String lineDelim = work.getTblDesc().getProperties()
+ .getProperty(serdeConstants.LINE_DELIM);
+ if (lineDelim != null && !lineDelim.isEmpty()
+ && !"\n".equals(lineDelim)) {
+ job.set("textinputformat.record.delimiter", lineDelim);
+ }
+ }
Review Comment:
The delimiter-propagation logic is duplicated here and in `HiveInputFormat` (same property lookup + filtering + `job.set`). To reduce drift and ensure both paths handle edge cases consistently (e.g., numeric delimiters, empty string semantics), consider extracting a small shared helper (e.g., in `Utilities` or a common IO utility) that resolves the table `line.delim` into the exact value to set for `textinputformat.record.delimiter`.
##########
ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java:
##########
@@ -448,6 +449,16 @@ public RecordReader getRecordReader(InputSplit split, JobConf job,
if ((part != null) && (part.getTableDesc() != null)) {
Utilities.copyTableJobPropertiesToConf(part.getTableDesc(), job);
nonNative = part.getTableDesc().isNonNative();
+ // When the table uses a multi-byte line.delim, propagate it to
+ // Hadoop's TextInputFormat so that LineRecordReader splits records
+ // on the full delimiter sequence instead of the default '\n'.
+ // TableDesc.getProperties() returns the raw (unescaped) value.
+ String lineDelim = part.getTableDesc().getProperties()
+ .getProperty(serdeConstants.LINE_DELIM);
+ if (lineDelim != null && !lineDelim.isEmpty()
+ && !"\n".equals(lineDelim)) {
+ job.set("textinputformat.record.delimiter", lineDelim);
+ }
Review Comment:
If `serdeConstants.LINE_DELIM` is set to a numeric byte string (e.g., \"10\"), this will configure Hadoop’s record delimiter to the literal characters '1''0' instead of LF, which would break reading for such tables. If numeric `line.delim` values are supported/expected, apply the same backward-compatible decoding here as in the write path (convert numeric strings to the corresponding single byte delimiter) before calling `job.set(...)`.
##########
ql/src/test/org/apache/hadoop/hive/ql/io/TestMultiCharLineDelimTextFormat.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.io;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.ql.exec.FileSinkOperator.RecordWriter;
+import org.apache.hadoop.hive.serde.serdeConstants;
+import org.apache.hadoop.io.LongWritable;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.mapred.FileInputFormat;
+import org.apache.hadoop.mapred.InputSplit;
+import org.apache.hadoop.mapred.JobConf;
+import org.apache.hadoop.mapred.RecordReader;
+import org.apache.hadoop.mapred.Reporter;
+import org.apache.hadoop.mapred.TextInputFormat;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+/**
+ * Verifies that multi-byte {@code line.delim} round-trips correctly
+ * after the fix to {@link HiveIgnoreKeyTextOutputFormat} (write) and
+ * the {@code textinputformat.record.delimiter} propagation in
+ * {@link HiveInputFormat} (read), including field values that contain
+ * bare {@code \n} bytes.
+ */
+public class TestMultiCharLineDelimTextFormat {
+
+ @Rule
+ public TemporaryFolder tmpFolder = new TemporaryFolder();
+
+ private static final String LINE_DELIM = "\n";
+ private static final String FIELD_DELIM = "@_@";
+
+ @Test
+ public void testWriteThenReadMultiByteLineDelim() throws Exception {
+ // --- Write ---
+ Properties tblProps = new Properties();
+ tblProps.setProperty(serdeConstants.LINE_DELIM, LINE_DELIM);
+ tblProps.setProperty(serdeConstants.FIELD_DELIM, FIELD_DELIM);
+
+ JobConf job = new JobConf();
+ File outDir = tmpFolder.newFolder("write_test");
+ Path outPath = new Path(outDir.getAbsolutePath(), "000000_0");
+
+ HiveIgnoreKeyTextOutputFormat<?, ?> outputFormat =
+ new HiveIgnoreKeyTextOutputFormat<>();
+ RecordWriter writer = outputFormat.getHiveRecordWriter(
+ job, outPath, Text.class, false, tblProps, Reporter.NULL);
+
+ // Rows with \n inside field values. Field delimiter is already
+ // applied here; in real usage the SerDe does that.
+ String[] rows = {
+ "1001" + FIELD_DELIM + "gaoqiang" + FIELD_DELIM + "28",
+ "1002" + FIELD_DELIM + "an\nxin" + FIELD_DELIM + "27",
+ "1003" + FIELD_DELIM + "mengyu" + FIELD_DELIM + "25"
+ };
+ for (String row : rows) {
+ writer.write(new Text(row));
+ }
+ writer.close(false);
+
+ // Verify raw file content: each record ends with \002\n
+ byte[] fileBytes = Files.readAllBytes(outDir.toPath().resolve("000000_0"));
+ String fileContent = new String(fileBytes, StandardCharsets.UTF_8);
+ for (String row : rows) {
+ Assert.assertTrue("File should contain row + \\002\\n",
+ fileContent.contains(row + LINE_DELIM));
+ }
+
+ // --- Read ---
+ // Simulate what HiveInputFormat.getRecordReader does after the fix:
+ // read line.delim from table properties, then set
+ // textinputformat.record.delimiter on the JobConf.
+ JobConf readJob = new JobConf();
+ readJob.set("textinputformat.record.delimiter", LINE_DELIM);
+
+ FileInputFormat.setInputPaths(readJob, outPath);
+
+ TextInputFormat inputFormat = new TextInputFormat();
+ inputFormat.configure(readJob);
+
+ InputSplit[] splits = inputFormat.getSplits(readJob, 1);
+ Assert.assertEquals(1, splits.length);
+
+ RecordReader<LongWritable, Text> reader =
+ inputFormat.getRecordReader(splits[0], readJob, Reporter.NULL);
+
+ List<String> records = new ArrayList<>();
+ LongWritable key = reader.createKey();
+ Text value = reader.createValue();
+ while (reader.next(key, value)) {
+ records.add(value.toString());
+ }
+ reader.close();
+
+ Assert.assertEquals("Should read exactly 3 records", 3, records.size());
+ Assert.assertEquals(rows[0], records.get(0));
+ Assert.assertEquals(rows[1], records.get(1));
+ Assert.assertEquals(rows[2], records.get(2));
+ }
+
+ /**
+ * Verify that when line.delim is not set, the default \n splitting
+ * behaviour is preserved on both write and read paths.
+ */
+ @Test
+ public void testDefaultLineDelim() throws Exception {
Review Comment:
The new tests cover multi-byte delimiter strings and the default delimiter, but they don't cover the backward-compatibility cases impacted by the write/read changes (notably numeric `line.delim` values like \"10\" and the empty-string case). Adding targeted assertions for those inputs would help prevent regressions and validate the intended decoding semantics for `line.delim`.
##########
ql/src/test/org/apache/hadoop/hive/ql/io/TestMultiCharLineDelimTextFormat.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.io;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.ql.exec.FileSinkOperator.RecordWriter;
+import org.apache.hadoop.hive.serde.serdeConstants;
+import org.apache.hadoop.io.LongWritable;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.mapred.FileInputFormat;
+import org.apache.hadoop.mapred.InputSplit;
+import org.apache.hadoop.mapred.JobConf;
+import org.apache.hadoop.mapred.RecordReader;
+import org.apache.hadoop.mapred.Reporter;
+import org.apache.hadoop.mapred.TextInputFormat;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+/**
+ * Verifies that multi-byte {@code line.delim} round-trips correctly
+ * after the fix to {@link HiveIgnoreKeyTextOutputFormat} (write) and
+ * the {@code textinputformat.record.delimiter} propagation in
+ * {@link HiveInputFormat} (read), including field values that contain
+ * bare {@code \n} bytes.
+ */
+public class TestMultiCharLineDelimTextFormat {
+
+ @Rule
+ public TemporaryFolder tmpFolder = new TemporaryFolder();
+
+ private static final String LINE_DELIM = "\n";
Review Comment:
The new tests cover multi-byte delimiter strings and the default delimiter, but they don't cover the backward-compatibility cases impacted by the write/read changes (notably numeric `line.delim` values like \"10\" and the empty-string case). Adding targeted assertions for those inputs would help prevent regressions and validate the intended decoding semantics for `line.delim`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org