rangareddy commented on code in PR #19463:
URL: https://github.com/apache/hudi/pull/19463#discussion_r3764335324
##########
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieRealtimeRecordReaderUtils.java:
##########
@@ -273,15 +273,24 @@ public static List<String> orderFields(String
fieldNameCsv, String fieldOrderCsv
// /org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java#L188}
// Field Names -> {@link
https://github.com/apache/hive/blob/f37c5de6c32b9395d1b34fa3c02ed06d1bfbf6eb/serde/src/java
// /org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java#L229}
- String[] fieldOrdersWithDups = fieldOrderCsv.isEmpty() ? new String[0] :
fieldOrderCsv.split(",");
+ // Blank tokens are dropped rather than carried into the loop below. For
SELECT COUNT(*) on Hive before
+ // 3.0.0 the read-column ids arrive empty and Hive combines them into e.g.
",2,0,3" (HIVE-22438, see
+ // HoodieRealtimeInputFormatUtils#cleanProjectionColumnIds, which only
strips one leading comma). A blank
+ // token used to reach Integer.parseInt and fail with a bare
NumberFormatException carrying none of the
+ // projection lists.
+ String[] fieldOrdersWithDups = fieldOrderCsv.isEmpty() ? new String[0]
+ : Arrays.stream(fieldOrderCsv.split(",")).filter(id ->
!id.trim().isEmpty()).toArray(String[]::new);
Review Comment:
Declining this one, with evidence — `isBlank()` cannot be used in this
module.
`hudi-hadoop-mr/pom.xml:176-184` pins the compiler to Java 8 for this module
specifically:
```xml
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<!-- Required to support older Hive versions that still use Java 8 -->
<target>8</target>
<release>8</release>
</configuration>
```
`String.isBlank()` is Java 11 API, so `--release 8` rejects it. I applied
your suggestion and compiled to be sure rather than arguing from the pom:
```
[ERROR] .../HoodieRealtimeRecordReaderUtils.java:[282,67] cannot find symbol
[ERROR] symbol: method isBlank()
```
Consistent with that, `isBlank()` has **zero** occurrences across the
codebase while `trim().isEmpty()` has 38 — the existing idiom is a constraint,
not an oversight.
Worth noting the two are not strictly equivalent either: `trim()` strips
code points `<= U+0020`, whereas `isBlank()` uses `Character.isWhitespace`.
Irrelevant for Hive projection ids, but it means this would not be a pure
readability swap even on a Java 11 module.
Leaving the line as is.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
