Skip to content

fix: Remove spaces around partition columns#18423

Open
linliu-code wants to merge 1 commit intoapache:masterfrom
linliu-code:fix_partition_column_trailing_spaces
Open

fix: Remove spaces around partition columns#18423
linliu-code wants to merge 1 commit intoapache:masterfrom
linliu-code:fix_partition_column_trailing_spaces

Conversation

@linliu-code
Copy link
Copy Markdown
Collaborator

@linliu-code linliu-code commented Mar 30, 2026

Describe the issue this Pull Request addresses

Partition columns can be around by spaces during configuration, like \sC1, C1\s, or C1,\sC2. In those cases, NPE could be thrown since we cannot find any partition columns when spaces.

Summary and Changelog

Remove spaces around partition columns during parsing.

Impact

Avoid NPE.

Risk Level

None.

Documentation Update

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

@linliu-code linliu-code marked this pull request as ready for review March 30, 2026 22:48
@github-actions github-actions bot added the size:XS PR with lines of changes in <= 10 label Mar 30, 2026
@hudi-bot
Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

Copy link
Copy Markdown
Member

@voonhous voonhous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add tests for these?

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.25%. Comparing base (1ff0506) to head (278e556).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18423      +/-   ##
============================================
- Coverage     68.26%   68.25%   -0.01%     
+ Complexity    27761    27754       -7     
============================================
  Files          2439     2439              
  Lines        134355   134356       +1     
  Branches      16210    16210              
============================================
- Hits          91715    91710       -5     
+ Misses        35543    35542       -1     
- Partials       7097     7104       +7     
Flag Coverage Δ
common-and-other-modules 44.36% <100.00%> (+<0.01%) ⬆️
hadoop-mr-java-client 44.97% <100.00%> (-0.02%) ⬇️
spark-client-hadoop-common 48.40% <100.00%> (+<0.01%) ⬆️
spark-java-tests 48.78% <100.00%> (+<0.01%) ⬆️
spark-scala-tests 45.29% <100.00%> (-0.01%) ⬇️
utilities 38.37% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rg/apache/hudi/common/table/HoodieTableConfig.java 94.83% <100.00%> (+<0.01%) ⬆️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@yihua yihua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

LGTM — the fix is simple and correct. Trimming before the empty-string filter is the right order, and the trimmed field names flow cleanly into getPartitionFieldWithoutKeyGenPartitionType. One thing worth noting: a reviewer has already asked for a dedicated test covering the leading/trailing-space scenario (e.g. " C1", "C1 ", "C1, C2"), and the contributor's checklist item for tests is still unchecked — it'd be good to add one before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS PR with lines of changes in <= 10

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants