Reject overlapping declared and generated key columns in SimpleJdbcInsert

[junhyeong9812]

Overview

TableMetaDataContext.reconcileColumnsToUse() excludes generated key columns from the auto-discovered column list, but when columns are declared explicitly via SimpleJdbcInsert.usingColumns(...), a column also declared via usingGeneratedKeyColumns(...) was not excluded from that declared list. This leaves the generated insert SQL out of sync with the values/types arrays bound to it, causing a parameter count mismatch at execution time whenever a column is declared both ways.

Problem

usingColumns("id", "name") combined with usingGeneratedKeyColumns("id"):

protected List<String> reconcileColumnsToUse(List<String> declaredColumns, String[] generatedKeyNames) {
    if (generatedKeyNames.length > 0) {
        this.generatedKeyColumnsUsed = true;
    }
    if (!declaredColumns.isEmpty()) {
        return new ArrayList<>(declaredColumns);
    }
    ...
}

returns ["id", "name"] unfiltered when declaredColumns is non-empty, while the auto-discovery branch below it already filters out generated key names. createInsertString() independently filters generatedKeyNames when building the SQL, so the insert statement only gets a placeholder for name:

INSERT INTO customers (name) VALUES(?)

but matchInParameterValuesWithInsertColumns(...) and createInsertTypes() both iterate the unfiltered tableColumns (= the declared columns, size 2), producing values/types arrays of size 2. The placeholder count (1) and the bound value count (2) diverge, and the JDBC driver rejects execution with a parameter index error.

Fix

Apply the same generated-key filter to the declared-columns branch that the auto-discovery branch already uses:

protected List<String> reconcileColumnsToUse(List<String> declaredColumns, String[] generatedKeyNames) {
    if (generatedKeyNames.length > 0) {
        this.generatedKeyColumnsUsed = true;
    }
    Set<String> keys = CollectionUtils.newLinkedHashSet(generatedKeyNames.length);
    for (String key : generatedKeyNames) {
        keys.add(key.toUpperCase(Locale.ROOT));
    }
    if (!declaredColumns.isEmpty()) {
        List<String> columns = new ArrayList<>();
        for (String column : declaredColumns) {
            if (!keys.contains(column.toUpperCase(Locale.ROOT))) {
                columns.add(column);
            }
        }
        return columns;
    }
    ...
}

This makes tableColumns — the single source consumed by createInsertString(), createInsertTypes(), and matchInParameterValuesWithInsertColumns(...) — consistently exclude generated key columns regardless of whether they came from auto-discovery or an explicit usingColumns(...) declaration. Added regression tests covering the partial-overlap, full-overlap (declared columns consist entirely of generated keys), and case-insensitive overlap cases.

Note on impact

This changes behavior for the specific combination of declaring a column via both usingColumns(...) and usingGeneratedKeyColumns(...) — previously that combination always failed at execution time, so no working code should depend on the old behavior. That said, it is a behavior change to a protected extension point (reconcileColumnsToUse), so I want to flag it explicitly in case there's a reason to prefer a different approach (e.g. rejecting the overlap explicitly instead of silently excluding it) — happy to adjust if so.

[sbrannen]

Hi @junhyeong9812,

Thanks for discovering this and submitting a proposal to address it. 👍

After reviewing the proposal and considering the potential ramifications of the change in behavior, we've decided that we would actually prefer an explicit failure over silent exclusion here. When a column is declared via both usingColumns(...) and usingGeneratedKeyColumns(...), that's a configuration error, and we'd rather fail fast with a clear message than have the column silently disappear from the generated SQL.

An InvalidDataAccessApiUsageException should suffice, and it's already what AbstractJdbcInsert.compile() throws for other misconfigurations (such as a missing table name). So, this would be consistent with existing validation.

Could you please rework the PR along those lines?

• In reconcileColumnsToUse(), throw InvalidDataAccessApiUsageException (naming the offending column(s)) when a declared column overlaps a generated-key column, instead of excluding it.
• Update/replace the added tests to assert the exception (message included) rather than the exclusion behavior.

Once you've completed those tasks, please force-push to this branch, and I'll take another look.

Cheers,

Sam

[sbrannen]

Please note that I changed the title of this PR to match the new direction, and I've converted this to a draft PR in the interim.