Skip to content

Fix generated keys retrieval for PostgreSQL SINGLE tables (#37485)#38024

Merged
terrymanu merged 5 commits intoapache:masterfrom
aviralgarg05:fix/37485-generated-keys-postgresql-single-table
Feb 18, 2026
Merged

Fix generated keys retrieval for PostgreSQL SINGLE tables (#37485)#38024
terrymanu merged 5 commits intoapache:masterfrom
aviralgarg05:fix/37485-generated-keys-postgresql-single-table

Conversation

@aviralgarg05
Copy link
Contributor

Description

When inserting into a SINGLE table with @GeneratedValue(IDENTITY) on PostgreSQL, getGeneratedKeys() threw NumberFormatException because it always read column index 1 from the underlying ResultSet. The PostgreSQL JDBC driver returns all columns via RETURNING * when RETURN_GENERATED_KEYS is used, so column 1 may not be the generated key column (it could be a string column like event).

Fix

Used the generated key column name from GeneratedKeyContext (when available) to read the correct column via resultSet.getObject(columnName) instead of blindly reading resultSet.getObject(1).

Fixes #37485.

Changes proposed in this pull request

  • Modified ShardingSpherePreparedStatement.getGeneratedKeys() to derive columnName earlier and use it for selection in the fallback path.
  • Modified ShardingSphereStatement.getGeneratedKeys() to derive columnName earlier and use it for selection in the fallback path.
  • Ensured fallback to resultSet.getObject(1) remains if columnName is not available.

Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request. (Labels: type: bug, db: PostgreSQL, in: JDBC)
  • I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e (Verified with ./mvnw -pl jdbc test - 1148 tests passed).
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes. (Existing GeneratedKeysResultSetTest and ShardingSpherePreparedStatementTest cover these paths and passed).
  • I have updated the Release Notes of the current development version.

When inserting into a SINGLE table with @GeneratedValue(IDENTITY)
on PostgreSQL, getGeneratedKeys() threw NumberFormatException because
it always read column index 1 from the underlying ResultSet. PostgreSQL
JDBC driver returns all columns via RETURNING *, so column 1 may not
be the generated key column.

Fix: use the generated key column name from GeneratedKeyContext (when
available) to read the correct column via resultSet.getObject(columnName)
instead of blindly reading resultSet.getObject(1).

Closes apache#37485
Copy link
Member

@terrymanu terrymanu left a comment

Choose a reason for hiding this comment

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

Decision

  • Merge Verdict: Not Mergeable
  • Reviewed Scope: latest PR revision (bd4ecca, 2026-02-12), changed files
    jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java,
    jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java,
    .github/workflows/e2e-agent.yml, related issue #37485, and PR checks page.
  • Not Reviewed Scope: full GitHub Actions job logs for every failed matrix item, runtime reproduction on real MySQL/PostgreSQL instances.
  • Need Expert Review: JDBC/dialect maintainer review is needed for generated-key behavior across PostgreSQL/MySQL/MariaDB drivers.

Positive Feedback

  • The fix direction is correct for the reported PostgreSQL SINGLE-table trigger path: fallback key extraction now uses a named column instead of always index 1, which targets the issue chain from #37485.

Major Issues

  1. High: cross-dialect compatibility regression risk in generated-key fallback.
    Symptom: both fallback paths now prefer resultSet.getObject(columnName) in
    jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java:330 and
    jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java:334, plus
    jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java:374 and
    jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java:379.
    Risk: MySQL dialect explicitly defines generated-key label "GENERATED_KEY" at
    database/connector/dialect/mysql/src/main/java/org/apache/shardingsphere/database/connector/mysql/metadata/database/MySQLDatabaseMetaData.java:84
    (and MariaDB delegates at database/connector/dialect/mariadb/src/main/java/org/apache/shardingsphere/database/connector/mariadb/metadata/database/MariaDBDatabaseMetaData.java:85), so using logical key column names may break drivers that expose only generated-key
    labels in getGeneratedKeys() result sets.
    Recommended action: make retrieval dialect-aware (use effective generated-key label or safe fallback to index 1) and add regression tests for PostgreSQL + MySQL/MariaDB.
  2. High: test evidence is insufficient for changed public behavior.
    Symptom: PR changes two production classes but no src/test files; existing test coverage for prepared statement is a single scenario unrelated to this branch at
    jdbc/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatementTest.java:52.
    Risk: root-cause path and adjacent regression paths are not proven.
    Recommended action: add dedicated tests mapping one-to-one to changed paths in both getGeneratedKeys() methods (column-name path, index fallback path, MySQL-style generated-key label path).
  3. Medium: validation signal is incomplete/unresolved.
    Symptom: PR checks page currently shows failed jobs (for example, E2E - Agent / Build E2E Image, plus multiple E2E matrix failures).
    Risk: unresolved CI reduces merge confidence and conflicts with submission expectations in CODE_OF_CONDUCT.md:18 and coverage expectations in CODE_OF_CONDUCT.md:20.
    Recommended action: rerun/fix failing checks or provide maintainer-confirmed flaky classification with fresh green evidence for this commit.

Newly Introduced Issues

  1. The current fallback implementation can introduce new failures on dialects that do not expose generated keys under the logical column name (potentially MySQL/MariaDB SINGLE-table flows that previously worked via index-based extraction).

Unrelated Changes

  • Please roll back or split out .github/workflows/e2e-agent.yml:77 and .github/workflows/e2e-agent.yml:91 (commit bd4ecca “ci: validate e2e-agent image archives”).
  • This CI workflow scope is unrelated to issue #37485 (PostgreSQL generated-key retrieval bug), so it should not be bundled in this bug-fix PR.

Next-Step Suggestions

  1. Remove/split the workflow commit from this PR.
  2. Adjust fallback key extraction to be dialect-safe, not only column-name-based.
  3. Add targeted unit tests for both modified public methods and both retrieval branches.
  4. Re-run checks and post fresh CI evidence for the latest head commit.

Copy link
Member

@terrymanu terrymanu left a comment

Choose a reason for hiding this comment

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

Thanks for the fix direction. There is one blocking scope issue before merge:

  • Commit 632d613a7095b7eedb55f263fe233e457fb75ee1 introduces formatting-only changes (line wrapping/indentation), which are not required for #37485.
  • Affected files:
    • jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java
    • jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java

Please roll back or split out these formatting-only edits, and keep this PR focused on the generated-keys behavior fix only.
This will keep the review scope minimal and reduce regression/review noise.

Please update the PR, and I will do a focused re-review.

…enerated-keys-postgresql-single-table

# Conflicts:
#	jdbc/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatementTest.java
@terrymanu terrymanu merged commit 1dd149d into apache:master Feb 18, 2026
148 checks passed
@terrymanu terrymanu added this to the 5.5.3 milestone Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't insert an entity with @GeneratedValue in a SINGLE table

2 participants