From c99f8519f00e4a5b7428d997253a50ed72aac9b2 Mon Sep 17 00:00:00 2001 From: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com> Date: Sun, 19 Jul 2026 09:11:44 +0900 Subject: [PATCH 1/5] Add name-based column modification to CsvSchema.Builder (#699) --- .../jackson/dataformat/csv/CsvSchema.java | 48 +++++++++++++++++++ .../dataformat/csv/schema/CsvSchemaTest.java | 42 ++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java index 0df3b8de4..6b8c660bf 100644 --- a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java +++ b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java @@ -629,6 +629,39 @@ public Builder removeColumn(int index) { return this; } + /** + * 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. + * + * @since 2.23 + */ + public Builder renameColumn(String oldName, String newName) { + return renameColumn(_columnIndex(oldName), newName); + } + + /** + * Method for replacing an existing column, located by its current name + * instead of by index. If no column with given {@code name} exists, + * an {@link IllegalArgumentException} is thrown. + * + * @since 2.23 + */ + public Builder replaceColumn(String name, Column c) { + return replaceColumn(_columnIndex(name), c); + } + + /** + * Method for removing an existing column, located by its name instead + * of by index. If no column with given {@code name} exists, an + * {@link IllegalArgumentException} is thrown. + * + * @since 2.23 + */ + public Builder removeColumn(String name) { + return removeColumn(_columnIndex(name)); + } + /** * Helper method called to drop the last collected column name if * it is empty: called if {link CsvParser.Feature#ALLOW_TRAILING_COMMA} @@ -869,6 +902,21 @@ protected void _checkIndex(int index) { throw new IllegalArgumentException("Illegal index "+index+"; only got "+_columns.size()+" columns"); } } + + /** + * Helper method for finding index of the column with given name; throws + * {@link IllegalArgumentException} if no column with the name exists. + * + * @since 2.23 + */ + protected int _columnIndex(String name) { + for (int i = 0, end = _columns.size(); i < end; ++i) { + if (_columns.get(i).getName().equals(name)) { + return i; + } + } + throw new IllegalArgumentException("No column with name '"+name+"' (have "+_columns.size()+" columns)"); + } } /* diff --git a/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java b/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java index ba16d5a8d..b07623874 100644 --- a/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java +++ b/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java @@ -253,4 +253,46 @@ public void testSchemaWithExplicitIndices() _verifyLinks(pointSchema); } + + // For [dataformats-text#699]: modify columns in Builder by name + @Test + public void testModifyColumnsByName() + { + CsvSchema.Builder b = CsvSchema.builder() + .addColumn("a", CsvSchema.ColumnType.STRING) + .addColumn("b", CsvSchema.ColumnType.NUMBER) + .addColumn("c", CsvSchema.ColumnType.BOOLEAN); + + // rename by name maps to same slot as rename by index + b.renameColumn("b", "b2"); + // replace by name preserves position, changes type + b.replaceColumn("c", new Column(2, "c2", CsvSchema.ColumnType.STRING)); + + CsvSchema schema = b.build(); + assertEquals(3, schema.size()); + assertEquals("a", schema.column(0).getName()); + assertEquals("b2", schema.column(1).getName()); + assertEquals(CsvSchema.ColumnType.NUMBER, schema.column(1).getType()); + assertEquals("c2", schema.column(2).getName()); + assertEquals(CsvSchema.ColumnType.STRING, schema.column(2).getType()); + + // remove by name drops just that column + CsvSchema shrunk = schema.rebuild().removeColumn("a").build(); + assertEquals(2, shrunk.size()); + assertEquals("b2", shrunk.column(0).getName()); + assertEquals("c2", shrunk.column(1).getName()); + } + + // For [dataformats-text#699]: unknown name should fail fast + @Test + public void testModifyUnknownColumnByName() + { + CsvSchema.Builder b = CsvSchema.builder().addColumn("a"); + try { + b.removeColumn("missing"); + fail("Should not pass"); + } catch (IllegalArgumentException e) { + verifyException(e, "No column with name 'missing'"); + } + } } From 4037df1223cc874f7731c91c8179847ec3c780a4 Mon Sep 17 00:00:00 2001 From: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:07:30 +0900 Subject: [PATCH 2/5] Use @throws/@param javadoc tags for name-based Builder methods Address review feedback: convert the prose IllegalArgumentException note into a proper @throws tag (and add @param tags) on renameColumn, replaceColumn and removeColumn. --- .../jackson/dataformat/csv/CsvSchema.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java index 6b8c660bf..3f4a7da81 100644 --- a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java +++ b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java @@ -631,8 +631,12 @@ public Builder removeColumn(int index) { /** * 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. + * instead of by index. + * + * @param oldName Current name of the column to rename + * @param newName New name to assign to the column + * + * @throws IllegalArgumentException if no column with given {@code oldName} exists * * @since 2.23 */ @@ -642,8 +646,12 @@ public Builder renameColumn(String oldName, String newName) { /** * Method for replacing an existing column, located by its current name - * instead of by index. If no column with given {@code name} exists, - * an {@link IllegalArgumentException} is thrown. + * instead of by index. + * + * @param name Name of the column to replace + * @param c Column definition to use as replacement + * + * @throws IllegalArgumentException if no column with given {@code name} exists * * @since 2.23 */ @@ -653,8 +661,11 @@ public Builder replaceColumn(String name, Column c) { /** * Method for removing an existing column, located by its name instead - * of by index. If no column with given {@code name} exists, an - * {@link IllegalArgumentException} is thrown. + * of by index. + * + * @param name Name of the column to remove + * + * @throws IllegalArgumentException if no column with given {@code name} exists * * @since 2.23 */ From 0440170d47a7c9d86c1343902973fc05bd54a01a Mon Sep 17 00:00:00 2001 From: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com> Date: Wed, 22 Jul 2026 22:04:27 +0900 Subject: [PATCH 3/5] Use @throws/@param javadoc tags on _columnIndex helper too --- .../com/fasterxml/jackson/dataformat/csv/CsvSchema.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java index 3f4a7da81..7bc47479b 100644 --- a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java +++ b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java @@ -915,8 +915,11 @@ protected void _checkIndex(int index) { } /** - * Helper method for finding index of the column with given name; throws - * {@link IllegalArgumentException} if no column with the name exists. + * Helper method for finding index of the column with given name. + * + * @param name Name of the column to find + * + * @throws IllegalArgumentException if no column with given {@code name} exists * * @since 2.23 */ From 0c2ae6412edba4d20fe54cec58961092cd88a60f Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 23 Jul 2026 20:19:44 -0700 Subject: [PATCH 4/5] Add release notes --- release-notes/CREDITS-2.x | 10 ++++++++++ release-notes/VERSION-2.x | 3 +++ 2 files changed, 13 insertions(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index edbb27d1e..0bae748a8 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -337,3 +337,13 @@ seonwoojung (@seonwooj0810) * Fixed #696: (toml) Write non-finite floating-point values as TOML tokens (`nan`/`inf`/`-inf`) instead of Java tokens (`NaN`/`Infinity`) (2.23.0) + +James Howe (@OrangeDog) + +* Requested #699: (csv) Ability to modify columns in `CsvSchema.Builder` by name + (2.23.0) + +seonwoojung (@seonwooj0810) + +* Fixed #699: (csv) Ability to modify columns in `CsvSchema.Builder` by name + (2.23.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index ea9c5bc9f..b79f75a84 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -20,6 +20,9 @@ Active Maintainers: (`nan`/`inf`/`-inf`) instead of Java tokens (`NaN`/`Infinity`) (reported by @EverNife) (fix by @seonwooj0810) +#699: (csv) Ability to modify columns in `CsvSchema.Builder` by name + (requested by @OrangeDog) + (fix by @seonwooj0810) 2.22.2 (not yet released) From 50783d01fe195a958f58c272089df8b2d9d5d9f8 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 23 Jul 2026 20:31:03 -0700 Subject: [PATCH 5/5] Minor fixes --- .../jackson/dataformat/csv/CsvSchema.java | 104 ++++++++++++++++-- .../dataformat/csv/schema/CsvSchemaTest.java | 48 +++++++- 2 files changed, 141 insertions(+), 11 deletions(-) diff --git a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java index 7bc47479b..fdb1373fe 100644 --- a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java +++ b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvSchema.java @@ -647,6 +647,11 @@ public Builder renameColumn(String oldName, String newName) { /** * Method for replacing an existing column, located by its current name * instead of by index. + *

+ * NOTE: index of the replacement column {@code c} is ignored (as with + * {@link #replaceColumn(int, Column)}): columns are renumbered to match + * their actual position when {@link #build()} is called. Caller hence does + * not need to know index of the column being replaced. * * @param name Name of the column to replace * @param c Column definition to use as replacement @@ -695,12 +700,41 @@ public Builder setColumnType(int index, ColumnType type) { return this; } + /** + * Method for changing type of an existing column, located by its name + * instead of by index. + * + * @param name Name of the column to change type of + * @param type Type to assign to the column + * + * @throws IllegalArgumentException if no column with given {@code name} exists + * + * @since 2.23 + */ + public Builder setColumnType(String name, ColumnType type) { + return setColumnType(_columnIndex(name), type); + } + public Builder removeArrayElementSeparator(int index) { _checkIndex(index); _columns.set(index, _columns.get(index).withArrayElementSeparator("")); return this; } + /** + * Method for removing array element separator of an existing column, + * located by its name instead of by index. + * + * @param name Name of the column to remove array element separator of + * + * @throws IllegalArgumentException if no column with given {@code name} exists + * + * @since 2.23 + */ + public Builder removeArrayElementSeparator(String name) { + return removeArrayElementSeparator(_columnIndex(name)); + } + /** * @since 2.7 */ @@ -710,6 +744,25 @@ public Builder setArrayElementSeparator(int index, String sep) { return this; } + /** + * Method for setting array element separator of an existing column, + * located by its name instead of by index. + *

+ * NOTE: not to be confused with single-argument + * {@link #setArrayElementSeparator(String)}, which sets the schema-wide + * default separator instead of that of a single column. + * + * @param name Name of the column to set array element separator of + * @param sep Array element separator to assign to the column + * + * @throws IllegalArgumentException if no column with given {@code name} exists + * + * @since 2.23 + */ + public Builder setArrayElementSeparator(String name, String sep) { + return setArrayElementSeparator(_columnIndex(name), sep); + } + public Builder setAnyPropertyName(String name) { _anyPropertyName = name; return this; @@ -738,12 +791,31 @@ public Iterator getColumns() { * @since 2.9 */ public boolean hasColumn(String name) { + return columnIndex(name) >= 0; + } + + /** + * Method for finding index of the column with given name, if any. + *

+ * NOTE: this method requires linear scan over existing columns + * so it may be more efficient to use other types of lookups if + * available (for example, {@link CsvSchema#columnIndex(String)} has a + * hash lookup to use). + * + * @param name Name of column to find + * + * @return Index of the first column with given name, if one exists; + * {@code -1} if not + * + * @since 2.23 + */ + public int columnIndex(String name) { for (int i = 0, end = _columns.size(); i < end; ++i) { if (_columns.get(i).getName().equals(name)) { - return true; + return i; } } - return false; + return -1; } /** @@ -915,21 +987,39 @@ protected void _checkIndex(int index) { } /** - * Helper method for finding index of the column with given name. + * Helper method for finding index of the column with given name, for + * use by name-based mutators; same as {@link #columnIndex(String)} except + * that a missing column is reported as an exception instead of {@code -1}. * * @param name Name of the column to find * + * @return Index of the first column with given name + * * @throws IllegalArgumentException if no column with given {@code name} exists * * @since 2.23 */ protected int _columnIndex(String name) { + int ix = columnIndex(name); + if (ix < 0) { + throw new IllegalArgumentException("No column '"+name+"' in CsvSchema.Builder (known columns: " + +_columnNames()+")"); + } + return ix; + } + + /** + * Helper method for constructing List of names of currently included + * columns, for use in exception messages. + * + * @since 2.23 + */ + private List _columnNames() { + List names = new ArrayList<>(_columns.size()); for (int i = 0, end = _columns.size(); i < end; ++i) { - if (_columns.get(i).getName().equals(name)) { - return i; - } + names.add(_columns.get(i).getName()); } - throw new IllegalArgumentException("No column with name '"+name+"' (have "+_columns.size()+" columns)"); + return names; } } diff --git a/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java b/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java index b07623874..7e8732b30 100644 --- a/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java +++ b/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/CsvSchemaTest.java @@ -265,8 +265,10 @@ public void testModifyColumnsByName() // rename by name maps to same slot as rename by index b.renameColumn("b", "b2"); - // replace by name preserves position, changes type - b.replaceColumn("c", new Column(2, "c2", CsvSchema.ColumnType.STRING)); + // replace by name preserves position, changes type; note that index of + // the replacement Column is deliberately "wrong" (0, not 2) to verify it + // is ignored -- the whole point being caller need not know the index + b.replaceColumn("c", new Column(0, "c2", CsvSchema.ColumnType.STRING)); CsvSchema schema = b.build(); assertEquals(3, schema.size()); @@ -275,6 +277,9 @@ public void testModifyColumnsByName() assertEquals(CsvSchema.ColumnType.NUMBER, schema.column(1).getType()); assertEquals("c2", schema.column(2).getName()); assertEquals(CsvSchema.ColumnType.STRING, schema.column(2).getType()); + // ... and index gets renumbered to match actual position + assertEquals(2, schema.column(2).getIndex()); + assertEquals(2, schema.columnIndex("c2")); // remove by name drops just that column CsvSchema shrunk = schema.rebuild().removeColumn("a").build(); @@ -283,7 +288,41 @@ public void testModifyColumnsByName() assertEquals("c2", shrunk.column(1).getName()); } - // For [dataformats-text#699]: unknown name should fail fast + // For [dataformats-text#699]: type and array-element-separator by name too + @Test + public void testModifyColumnSettingsByName() + { + CsvSchema.Builder b = CsvSchema.builder() + .addColumn("a", CsvSchema.ColumnType.STRING) + .addColumn("tags", CsvSchema.ColumnType.ARRAY); + + b.setColumnType("a", CsvSchema.ColumnType.NUMBER); + b.setArrayElementSeparator("tags", ";"); + + CsvSchema schema = b.build(); + assertEquals(CsvSchema.ColumnType.NUMBER, schema.column(0).getType()); + assertEquals(";", schema.column(1).getArrayElementSeparator()); + + // and removal of separator, likewise by name + assertEquals("", schema.rebuild().removeArrayElementSeparator("tags") + .build().column(1).getArrayElementSeparator()); + } + + // For [dataformats-text#699]: non-throwing index lookup from Builder + @Test + public void testColumnIndexLookupFromBuilder() + { + CsvSchema.Builder b = CsvSchema.builder().addColumn("a").addColumn("b"); + assertEquals(0, b.columnIndex("a")); + assertEquals(1, b.columnIndex("b")); + assertEquals(-1, b.columnIndex("missing")); + // and `hasColumn()` remains consistent with it + assertTrue(b.hasColumn("a")); + assertFalse(b.hasColumn("missing")); + } + + // For [dataformats-text#699]: unknown name should fail fast, and the failure + // should name the columns that DO exist (as `CsvSchema.withColumn(String,...)` does) @Test public void testModifyUnknownColumnByName() { @@ -292,7 +331,8 @@ public void testModifyUnknownColumnByName() b.removeColumn("missing"); fail("Should not pass"); } catch (IllegalArgumentException e) { - verifyException(e, "No column with name 'missing'"); + verifyException(e, "No column 'missing' in CsvSchema.Builder"); + verifyException(e, "known columns: [a]"); } } }