Skip to content

Fix ResultSet#findColumn - return actual column's index instead of 0#2376

Merged
chernser merged 1 commit into
ClickHouse:mainfrom
gravity182:fix-rs-column-index
May 20, 2025
Merged

Fix ResultSet#findColumn - return actual column's index instead of 0#2376
chernser merged 1 commit into
ClickHouse:mainfrom
gravity182:fix-rs-column-index

Conversation

@gravity182

@gravity182 gravity182 commented May 19, 2025

Copy link
Copy Markdown
Contributor

Summary

Now findColumn returns actual column's index.

Closes #2375.
Closes #1378.
Closes #2498.

The reason this method returns 0 is the following: this method obtains an instance of ClickHouseColumn to return its index. However, its private field columnIndex always equals 0 for any write/read path. This field is set in the constructor initially and never updated to reflect the actual column's index - https://github.com/gravity182/clickhouse-java/blob/a3f80b6476c6832a0f8e09c88b65f55e86b4e270/clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseColumn.java#L670.

Interestingly, this method worked correctly in jdbc-v1. See the implementation - https://github.com/gravity182/clickhouse-java/blob/3ce1b09161e15bfd13695d7fb7bdd198b4672e00/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/ClickHouseResultSet.java#L219-L225. As you can see, it simply counted from 1 until the required column is found.

Having said that, we can proceed with this solution fixing only the findColumn method or fix this problem on the deeper level, setting ClickHouseColumn#columnIndex field to a correct column's index. There probably might be other methods affected by this problem as well.

Checklist

Delete items not relevant to your PR:

@CLAassistant

CLAassistant commented May 19, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chernser

Copy link
Copy Markdown
Contributor

Thank you, @gravity182!
Excellent PR!

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

Labels

None yet

Projects

None yet

3 participants