ClickHouse / ClickHouse/clickhouse-java
[jdbc-v2] ResultSet label getters return null/0 for an unknown column label instead of throwing SQLException
- Dominant language
- Java
- Stars
- 1.6k
- Forks
- 636
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 28
Description
## Description
In `jdbc-v2`, most `ResultSet` getters that take a **column label** treat an *unknown* label as a SQL `NULL` value instead of reporting an error: they set `wasNull() == true` and return `null` / `0` / `false`.
JDBC requires an `SQLException` when the label does not identify a column in the result set (`ResultSet.getString(String)`: "throws SQLException - if the columnLabel is not valid"). As it is, a typo in a column name is indistinguishable from a genuine NULL, so an application reads silent zeros/nulls instead of failing.
The getter family is also inconsistent: `getDate`, `getTime`, `getTimestamp`, `getBytes`, `getBinaryStream`, `getObject` and `findColumn` all *do* fail on the same unknown label.
### Steps to reproduce
1. Open a JDBC connection with the `jdbc-v2` driver.
2. `SELECT 'abc' AS txt, 42 AS num, CAST(NULL AS Nullable(Int32)) AS nul`
3. Call the label getters with a label that is not in the result set, e.g. `rs.getInt("no_such_column")`.
### Error Log or Exception StackTrace
```
=== sanity: known labels ===
getString("txt") -> "abc", wasNull=false
getInt("num") -> 42, wasNull=false
getInt("nul") [real SQL NULL] -> 0, wasNull=true
=== unknown label "no_such_column" ===
getString -> NO THROW, value=null, wasNull=true <-- indistinguishable from a real NULL
getBoolean -> NO THROW, value=false, wasNull=true
getByte -> NO THROW, value=0, wasNull=true
getShort -> NO THROW, value=0, wasNull=true
getInt -> NO THROW, value=0, wasNull=true
getLong -> NO THROW, value=0, wasNull=true
getFloat -> NO THROW, value=0.0, wasNull=true
getDouble -> NO THROW, value=0.0, wasNull=true
getBigDecimal -> NO THROW, value=null, wasNull=true
=== same unknown label, getters that DO fail ===
getDate -> NoSuchColumnException: Result has no column with name 'no_such_column'
getTime -> NoSuchColumnException: Result has no column with name 'no_such_column'
getTimestamp -> NoSuchColumnException: Result has no column with name 'no_such_column'
getBytes -> NoSuchColumnException: Result has no column with name 'no_such_column'
getBinaryStream -> NoSuchColumnException: Result has no column with name 'no_such_column'
getTimestamp(label, cal) -> NoSuchColumnException: Result has no column with name 'no_such_column'
getObject -> SQLException: Method: getObject("no_such_column", null) encountered an exception.
findColumn -> SQLException: Method: findColumn("no_such_column") encountered an exception.
```
### Expected Behaviour
Every label getter reports an unknown column label as an `SQLException`. Only a column that exists and holds SQL `NULL` should return `null` / `0` with `wasNull() == true`.
Note that the getters that currently fail do so with `com.clickhouse.client.api.metadata.NoSuchColumnException`, which extends `ClientException` -> `ClickHouseException` -> `RuntimeException`. That is an unchecked exception crossing the JDBC boundary, so it does not satisfy the JDBC contract either, although at least it is not silent.
## Root cause
`jdbc-v2` `ResultSetImpl` label getters guard the read with `reader.hasValue(columnLabel)` (for example `ResultSetImpl.java:314` in `getString(String)`, `:378` in `getInt(String)`, `:394` in `getLong(String)`) and fall into the "no value" branch when it returns `false`:
```java
if (reader.hasValue(columnLabel)) {
wasNull = false;
return reader.getString(columnLabel);
} else {
wasNull = true;
return null;
}
```
`AbstractBinaryFormatReader.hasValue(String)` (`client-v2 .../data_formats/internal/AbstractBinaryFormatReader.java:607`) resolves the label with `TableSchema.findColumnIndex`, which returns `-1` for an unknown column (`client-v2 .../metadata/TableSchema.java:136`); `hasValue(int)` then rejects `-1` as out of range and returns `false`.
So `hasValue(String) == false` means **either** "the column is absent" **or** "the column is present and its value is null", and the JDBC layer maps both onto SQL NULL.
The leniency in `client-v2` is deliberate — it is the behaviour requested in #2755 for the `hasValue` predicate, and it is reasonable for a predicate. The defect is in `jdbc-v2` using that predicate as its label-resolution step, where "absent" must be an error.
The getters that do fail take a different route: they resolve the label through `TableSchema.nameToColumnIndex` (`TableSchema.java:113`), which throws `NoSuchColumnException`. Hence the inconsistency inside the same class.
## Suggested fix
Resolve the label **once** in the label getters and make an unresolvable label an `SQLException`, then look the value up by index. Concretely: a single private helper that resolves label -> 1-based index and throws `SQLException` when the column does not exist, used by all label getters, with the null check done on the resolved index (`hasValue(int)`).
This also makes the getter family consistent and converts the current unchecked `NoSuchColumnException` leaks on the `getDate` / `getTime` / `getTimestamp` / `getBytes` / `getBinaryStream` paths into proper `SQLException`s.
Cases that must keep their current behaviour:
- an existing column whose value is SQL `NULL` still returns `null` / `0` with `wasNull() == true` (verified above with `CAST(NULL AS Nullable(Int32)) AS nul`);
- `hasValue(String)` in `client-v2` keeps returning `false` for a missing column (#2755) — the change belongs in `jdbc-v2`, not in `client-v2`.
Related observation, same code path: a **case-mismatched** label for an existing column (`rs.getString("TXT")` for column `txt`) is also silently read as NULL today, because the lookup is an exact-match map. After this fix it would raise an `SQLException`. Whether `jdbc-v2` should additionally match labels case-insensitively (the JDBC javadoc states column names used as input to getter methods are case insensitive) is a separate decision, and is not covered by this report.
### Code Example
```java
try (Connection conn = DriverManager.getConnection(url, props);
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("SELECT 'abc' AS txt, 42 AS num")) {
rs.next();
// Expected: SQLException. Actual: returns 0 and wasNull() == true.
int v = rs.getInt("no_such_column");
boolean wasNull = rs.wasNull();
// Expected: SQLException. Actual: returns null and wasNull() == true.
String s = rs.getString("no_such_column");
}
```
### Configuration
#### Environment
* [ ] Cloud
* clickhouse-java: `main` at `91ec4d326` (VERSION `0.11.0-rc1`), module `jdbc-v2`
* ClickHouse server: 26.8.2.7 (local Docker)
* JDK 17, Linux x86_64
## Notes
Found by automated analysis of `jdbc-v2` while working on the indexed-getter read path (#2516 / PR #3124). It is **not** introduced by that PR — it reproduces on plain `main` at the commit above, and the pre-#3124 code reaches the identical `findColumnIndex` -> `-1` path. Verified by running the getters against a live ClickHouse server, not by inspection alone.
Contributor guide
Assessment
This issue has not been assessed yet.