Skip to content

Conversation

@chernser
Copy link
Contributor

@chernser chernser commented Apr 22, 2025

Summary

According to JDBC there are some method from Statement interface that cannot be called on instance of PreparedStatement. It is done to protect internal state. Imaging having a PrepredStatement("SELECT * FROM t WHERE v > ?") and executing executeQuery("SELECT * FROM t1"). In this case internal result set will be pointing to a result of second query not a prepared one.
Sometimes it is convenient to create only one object of PreparedStatement and execute some SET statements. But it leads to errors and confusing internal state as it is described.

Closes: #2339

Checklist

Delete items not relevant to your PR:

@chernser chernser added this to the 0.8.5 milestone Apr 22, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR disables calling certain Statement methods on PreparedStatement objects to prevent accidental misuse and internal state corruption. Key changes include:

  • Adding tests to verify that forbidden Statement methods throw SQLException.
  • Introducing new SQL state codes and updating exception messages.
  • Refactoring method names and access modifiers for consistency in StatementImpl and PreparedStatementImpl.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java Added tests to check that disallowed Statement methods throw exceptions.
jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ExceptionUtils.java Updated error codes along with inline comments for better clarity.
jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java Refactored method names to their "Impl" variants and updated enum visibility.
jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java Overridden forbidden Statement methods to throw clear SQLException messages.

public static final String SQL_STATE_INVALID_SCHEMA = "3F000";
public static final String SQL_STATE_INVALID_TX_STATE = "25000";
public static final String SQL_STATE_DATA_EXCEPTION = "22000";
// Used only when feature is no supported
Copy link

Copilot AI Apr 22, 2025

Choose a reason for hiding this comment

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

Fix the grammatical error in the comment: change 'no supported' to 'not supported'.

Suggested change
// Used only when feature is no supported
// Used only when feature is not supported

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
43.6% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@chernser chernser merged commit 49d5322 into main Apr 24, 2025
24 of 25 checks passed
@chernser chernser deleted the jdbc_fix_prepared_statement branch April 24, 2025 17:26
@chernser chernser removed this from the 0.8.5 milestone Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[jdbc-v2] Disallow to call Statement related method on PreparedStatement

4 participants