Skip to content

Add name-based column modification to CsvSchema.Builder (#699) - #703

Merged
cowtowncoder merged 6 commits into
FasterXML:2.xfrom
seonwooj0810:fix/issue-699-csv-builder-modify-by-name
Jul 24, 2026
Merged

Add name-based column modification to CsvSchema.Builder (#699)#703
cowtowncoder merged 6 commits into
FasterXML:2.xfrom
seonwooj0810:fix/issue-699-csv-builder-modify-by-name

Conversation

@seonwooj0810

Copy link
Copy Markdown
Contributor

Fixes #699

CsvSchema.Builder only lets you modify columns by index (renameColumn(int, ...), replaceColumn(int, ...), removeColumn(int)), so modifying a column by name meant building the schema, scanning for the index, and rebuilding — and the index could shift in between. This adds name-based overloads so the fluent builder can locate a column by name directly.

Changes

  • renameColumn(String oldName, String newName)
  • replaceColumn(String name, Column c)
  • removeColumn(String name)

Each resolves the name to an index via a new _columnIndex(String) helper (linear scan, mirroring the existing hasColumn(String)) and delegates to the corresponding index-based method, so existing behavior is unchanged. An unknown name throws IllegalArgumentException, consistent with the existing _checkIndex out-of-range handling.

Test evidence

Added to CsvSchemaTest:

  • testModifyColumnsByName — rename/replace/remove by name preserve column positions and only touch the targeted column.
  • testModifyUnknownColumnByName — an unknown name throws IllegalArgumentException.

./mvnw -pl csv test -Dtest=CsvSchemaTestTests run: 11, Failures: 0, Errors: 0.

Verification done: (1) no in-flight PR (searched open/all PRs for CsvSchema renameColumn removeColumn); (2) no self-claim (issue has no comments); (3) code-focused (CsvSchema.java + test); (4) confirmed the Builder had only index-based modifiers on 2.x; (6) not a spring-projects/* repo, no triage gate.

/**
* Method for renaming an existing column, located by its current name
* instead of by index. If no column with given {@code oldName} exists,
* an {@link IllegalArgumentException} is thrown.

@OrangeDog OrangeDog Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't it be

@throws IllegalArgumentException if no column with given {@code oldName} exists

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — you're right. Converted the prose note into a proper @throws IllegalArgumentException tag (and added @param tags) on renameColumn, replaceColumn and removeColumn in 4037df1.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(and _columnIndex)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point — done in 0440170. Converted _columnIndex's prose exception note into a proper @throws IllegalArgumentException tag and added a @param name tag, matching the public name-based methods.

seonwooj0810 and others added 4 commits July 22, 2026 10:07
Address review feedback: convert the prose IllegalArgumentException note
into a proper @throws tag (and add @PARAM tags) on renameColumn,
replaceColumn and removeColumn.
@cowtowncoder cowtowncoder changed the title Add name-based column modification to CsvSchema.Builder (#699) Add name-based column modification to CsvSchema.Builder (#699) Jul 24, 2026
@cowtowncoder cowtowncoder added this to the 2.23.0 milestone Jul 24, 2026
@cowtowncoder
cowtowncoder merged commit be3f574 into FasterXML:2.x Jul 24, 2026
4 checks passed
@cowtowncoder

Copy link
Copy Markdown
Member

Added in 2.x (for 2.23.0) and 3.x (3.3.0).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants