hudi-agent commented on code in PR #19157:
URL: https://github.com/apache/hudi/pull/19157#discussion_r3521721936
##########
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieRealtimeInputFormatUtils.java:
##########
@@ -137,9 +137,7 @@ public static void cleanProjectionColumnIds(Configuration
conf) {
String columnIds =
conf.get(ColumnProjectionUtils.READ_COLUMN_IDS_CONF_STR);
if (!columnIds.isEmpty() && columnIds.charAt(0) == ',') {
conf.set(ColumnProjectionUtils.READ_COLUMN_IDS_CONF_STR,
columnIds.substring(1));
- if (LOG.isDebugEnabled()) {
- LOG.debug("The projection Ids: {" + columnIds + "} start with ','.
First comma is removed");
- }
+ LOG.debug("The projection Ids: {{}} start with ','. First comma is
removed", columnIds);
Review Comment:
🤖 nit: `{{}}` is a non-obvious SLF4J idiom — `{{` is a literal `{`, followed
by `{}` as the placeholder, giving `{VALUE}`. A future reader might think it's
a typo or try to "fix" it. Could you use something like `"The projection Ids:
'[{}]' start with ','..."` or just drop the decorative braces entirely?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/hive/HoodieCombineHiveInputFormat.java:
##########
@@ -559,7 +558,7 @@ private List<CombineFileSplit>
sampleSplits(List<CombineFileSplit> splits) {
retLists.add(split);
long splitgLength = split.getLength();
if (size + splitgLength >= targetSize) {
- LOG.info("Sample alias " + entry.getValue() + " using " + (i + 1) +
"splits");
+ LOG.info("Sample alias {} using {}splits", entry.getValue(), (i +
1));
Review Comment:
🤖 Worth noting this missing space is pre-existing — the original was `"using
" + (i + 1) + "splits"`, and the PR description explicitly calls out preserving
message text verbatim (including this quirk) to keep the change a pure
refactor. Adding the space would be a real output change, so it's probably
cleaner to do it as a separate commit/PR rather than fold it into the logging
cleanup.
##########
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/hive/HoodieCombineHiveInputFormat.java:
##########
@@ -559,7 +558,7 @@ private List<CombineFileSplit>
sampleSplits(List<CombineFileSplit> splits) {
retLists.add(split);
long splitgLength = split.getLength();
if (size + splitgLength >= targetSize) {
- LOG.info("Sample alias " + entry.getValue() + " using " + (i + 1) +
"splits");
+ LOG.info("Sample alias {} using {}splits", entry.getValue(), (i +
1));
Review Comment:
🤖 nit: since you're already touching this line, could you add a space before
`splits`? As written it would log `using 5splits` with no separator.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
