Skip to content

Commit 3bfe2ce

Browse files
authored
[ES-1527223] Fixes parameter counting for prepared statement queries according to JDBC standards (#907)
## Description <!-- Provide a brief summary of the changes made and the issue they aim to address.--> - **Enhanced `DatabricksParameterMetaData`**: - Added constructor accepting SQL string for parameter analysis - Implemented robust `countParameters(String sql)` method with comprehensive SQL parsing - Modified `getParameterCount()` to return statically-determined parameter count - **Updated `DatabricksPreparedStatement`**: - Modified to pass SQL statement to `DatabricksParameterMetaData` during initialization - Ensures consistent parameter counting across batch operations ### SQL Parser Capabilities - Accurately identifies parameter markers (`?`) while respecting SQL syntax rules - Properly handles quoted literals (single and double quotes with escape sequences) - Correctly processes SQL comments (line comments `--` and block comments `/* */`) - Supports complex SQL constructs including multi-line statements and nested structures ## Testing <!-- Describe how the changes have been tested--> Unit tests ## Additional Notes to the Reviewer <!-- Share any additional context or insights that may help the reviewer understand the changes better. This could include challenges faced, limitations, or compromises made during the development process. Also, mention any areas of the code that you would like the reviewer to focus on specifically. --> --------- Signed-off-by: Madhav Sainanee <[email protected]>
1 parent 0effaa6 commit 3bfe2ce

File tree

4 files changed

+286
-4
lines changed

4 files changed

+286
-4
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
- Fixed Bouncy Castle registration conflicts by using local provider instance instead of global security registration.
2222
- Fixed Azure U2M authentication issue.
2323
- Fixed unchecked exception thrown in delete session
24+
- Fixed ParameterMetaData.getParameterCount() to return total parameter count from SQL parsing instead of bound parameter count, aligning with JDBC standards
2425

2526
---
2627
*Note: When making changes, please add your change under the appropriate section with a brief description.*

src/main/java/com/databricks/jdbc/api/impl/DatabricksParameterMetaData.java

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,15 @@ public class DatabricksParameterMetaData implements ParameterMetaData {
1818
private static final JdbcLogger LOGGER =
1919
JdbcLoggerFactory.getLogger(DatabricksParameterMetaData.class);
2020
private final Map<Integer, ImmutableSqlParameter> parameterBindings;
21+
private final int parameterCount;
2122

2223
public DatabricksParameterMetaData() {
24+
this(null);
25+
}
26+
27+
public DatabricksParameterMetaData(String sql) {
2328
this.parameterBindings = new HashMap<>();
29+
this.parameterCount = countParameters(sql);
2430
}
2531

2632
public void put(int param, ImmutableSqlParameter value) {
@@ -37,7 +43,24 @@ public void clear() {
3743

3844
@Override
3945
public int getParameterCount() throws SQLException {
40-
return parameterBindings.size();
46+
validateParameterBindings();
47+
return parameterCount;
48+
}
49+
50+
/**
51+
* Validates that parameter bindings do not exceed the parameter count.
52+
*
53+
* @throws SQLException if there are more parameter bindings than expected parameters
54+
*/
55+
private void validateParameterBindings() throws SQLException {
56+
if (parameterBindings.size() > parameterCount) {
57+
throw new SQLException(
58+
"Number of parameter bindings ("
59+
+ parameterBindings.size()
60+
+ ") exceeds parameter count ("
61+
+ parameterCount
62+
+ ")");
63+
}
4164
}
4265

4366
@Override
@@ -111,4 +134,80 @@ private ImmutableSqlParameter getObject(int param) {
111134
}
112135
return parameterBindings.get(param);
113136
}
137+
138+
/**
139+
* Counts the number of parameter markers (?) in the SQL statement. This is currently a hacky
140+
* implementation that may need improvement for complex SQL.
141+
*
142+
* @param sql The SQL statement to analyze
143+
* @return The number of parameter markers in the SQL statement
144+
*/
145+
private int countParameters(String sql) {
146+
if (sql == null || sql.isEmpty()) {
147+
return 0;
148+
}
149+
150+
int count = 0;
151+
boolean inSingleQuote = false;
152+
boolean inDoubleQuote = false;
153+
boolean inLineComment = false;
154+
boolean inBlockComment = false;
155+
156+
for (int i = 0; i < sql.length(); i++) {
157+
char c = sql.charAt(i);
158+
char next = (i < sql.length() - 1) ? sql.charAt(i + 1) : '\0';
159+
160+
// Handle comments
161+
if (!inSingleQuote && !inDoubleQuote) {
162+
if (!inBlockComment && !inLineComment && c == '-' && next == '-') {
163+
inLineComment = true;
164+
i++; // Skip next dash
165+
continue;
166+
} else if (!inBlockComment && !inLineComment && c == '/' && next == '*') {
167+
inBlockComment = true;
168+
i++; // Skip next asterisk
169+
continue;
170+
} else if (inLineComment && (c == '\n' || c == '\r')) {
171+
inLineComment = false;
172+
} else if (inBlockComment && c == '*' && next == '/') {
173+
inBlockComment = false;
174+
i++; // Skip next slash
175+
continue;
176+
}
177+
}
178+
179+
// Skip if in comment
180+
if (inLineComment || inBlockComment) {
181+
continue;
182+
}
183+
184+
// Handle quotes with escaped quotes
185+
if (c == '\'') {
186+
if (!inDoubleQuote) {
187+
// Check for escaped single quote
188+
if (inSingleQuote && i < sql.length() - 1 && sql.charAt(i + 1) == '\'') {
189+
i++; // Skip the escaped quote
190+
} else {
191+
inSingleQuote = !inSingleQuote;
192+
}
193+
}
194+
} else if (c == '"') {
195+
if (!inSingleQuote) {
196+
// Check for escaped double quote
197+
if (inDoubleQuote && i < sql.length() - 1 && sql.charAt(i + 1) == '"') {
198+
i++; // Skip the escaped quote
199+
} else {
200+
inDoubleQuote = !inDoubleQuote;
201+
}
202+
}
203+
}
204+
205+
// Count parameter markers only when not inside quotes or comments
206+
if (c == '?' && !inSingleQuote && !inDoubleQuote) {
207+
count++;
208+
}
209+
}
210+
211+
return count;
212+
}
114213
}

src/main/java/com/databricks/jdbc/api/impl/DatabricksPreparedStatement.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public DatabricksPreparedStatement(DatabricksConnection connection, String sql)
4141
super(connection);
4242
this.sql = sql;
4343
this.interpolateParameters = connection.getConnectionContext().supportManyParameters();
44-
this.databricksParameterMetaData = new DatabricksParameterMetaData();
44+
this.databricksParameterMetaData = new DatabricksParameterMetaData(sql);
4545
this.databricksBatchParameterMetaData = new ArrayList<>();
4646
}
4747

@@ -345,14 +345,14 @@ public boolean execute() throws SQLException {
345345
public void addBatch() {
346346
LOGGER.debug("public void addBatch()");
347347
this.databricksBatchParameterMetaData.add(databricksParameterMetaData);
348-
this.databricksParameterMetaData = new DatabricksParameterMetaData();
348+
this.databricksParameterMetaData = new DatabricksParameterMetaData(sql);
349349
}
350350

351351
@Override
352352
public void clearBatch() throws DatabricksSQLException {
353353
LOGGER.debug("public void clearBatch()");
354354
checkIfClosed();
355-
this.databricksParameterMetaData = new DatabricksParameterMetaData();
355+
this.databricksParameterMetaData = new DatabricksParameterMetaData(sql);
356356
this.databricksBatchParameterMetaData = new ArrayList<>();
357357
}
358358

src/test/java/com/databricks/jdbc/api/impl/DatabricksParameterMetaDataTest.java

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,186 @@ public void testGetParameterType() throws SQLException {
6767
assertEquals(Types.VARCHAR, metaData.getParameterType(1));
6868
assertEquals(metaData.getParameterType(2), Types.INTEGER);
6969
}
70+
71+
@Test
72+
public void testConstructorWithNullSql() throws SQLException {
73+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(null);
74+
assertEquals(0, metadata.getParameterCount());
75+
}
76+
77+
@Test
78+
public void testConstructorWithEmptySql() throws SQLException {
79+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData("");
80+
assertEquals(0, metadata.getParameterCount());
81+
}
82+
83+
@Test
84+
public void testParameterCountWithSimpleQuery() throws SQLException {
85+
String sql = "SELECT * FROM table WHERE id = ? AND name = ?";
86+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
87+
assertEquals(2, metadata.getParameterCount());
88+
}
89+
90+
@Test
91+
public void testParameterCountWithNoParameters() throws SQLException {
92+
String sql = "SELECT * FROM table WHERE id = 1 AND name = 'test'";
93+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
94+
assertEquals(0, metadata.getParameterCount());
95+
}
96+
97+
@Test
98+
public void testParameterCountIgnoresParametersInSingleQuotes() throws SQLException {
99+
String sql = "SELECT * FROM table WHERE name = 'has ? in quotes' AND id = ?";
100+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
101+
assertEquals(1, metadata.getParameterCount());
102+
}
103+
104+
@Test
105+
public void testParameterCountIgnoresParametersInDoubleQuotes() throws SQLException {
106+
String sql = "SELECT * FROM table WHERE name = \"has ? in quotes\" AND id = ?";
107+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
108+
assertEquals(1, metadata.getParameterCount());
109+
}
110+
111+
@Test
112+
public void testParameterCountWithEscapedSingleQuotes() throws SQLException {
113+
String sql =
114+
"SELECT * FROM table WHERE name = 'has '' escaped quote and ? parameter' AND id = ?";
115+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
116+
assertEquals(1, metadata.getParameterCount());
117+
}
118+
119+
@Test
120+
public void testParameterCountWithEscapedDoubleQuotes() throws SQLException {
121+
String sql =
122+
"SELECT * FROM table WHERE name = \"has \"\" escaped quote and ? parameter\" AND id = ?";
123+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
124+
assertEquals(1, metadata.getParameterCount());
125+
}
126+
127+
@Test
128+
public void testParameterCountIgnoresParametersInLineComments() throws SQLException {
129+
String sql = "SELECT * FROM table WHERE id = ? -- comment with ? parameter\nAND name = ?";
130+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
131+
assertEquals(2, metadata.getParameterCount());
132+
}
133+
134+
@Test
135+
public void testParameterCountIgnoresParametersInBlockComments() throws SQLException {
136+
String sql =
137+
"SELECT * FROM table WHERE id = ? /* block comment with ? parameter */ AND name = ?";
138+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
139+
assertEquals(2, metadata.getParameterCount());
140+
}
141+
142+
@Test
143+
public void testParameterCountWithMultilineBlockComment() throws SQLException {
144+
String sql =
145+
"SELECT * FROM table WHERE id = ? /* multi-line\n comment with ? parameter\n */ AND name = ?";
146+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
147+
assertEquals(2, metadata.getParameterCount());
148+
}
149+
150+
@Test
151+
public void testParameterCountWithComplexQuery() throws SQLException {
152+
String sql =
153+
"SELECT t1.id, t2.name "
154+
+ "FROM table1 t1 "
155+
+ "JOIN table2 t2 ON t1.id = t2.id "
156+
+ "WHERE t1.status = ? "
157+
+ "AND t2.created_date > ? "
158+
+ "AND t1.description NOT LIKE '%test?value%' -- comment with ?\n"
159+
+ "ORDER BY t1.id LIMIT ?";
160+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
161+
assertEquals(3, metadata.getParameterCount());
162+
}
163+
164+
@Test
165+
public void testParameterCountWithMixedQuotesAndComments() throws SQLException {
166+
String sql =
167+
"SELECT * FROM table WHERE col1 = 'value with ? mark' AND col2 = ? /* comment ? */ AND col3 = \"another ? value\" AND col4 = ?";
168+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
169+
assertEquals(2, metadata.getParameterCount());
170+
}
171+
172+
@Test
173+
public void testParameterCountAfterBindingParameters() throws SQLException {
174+
String sql = "SELECT * FROM table WHERE id = ? AND name = ? AND status = ?";
175+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
176+
177+
// Initially should return count based on SQL parsing
178+
assertEquals(3, metadata.getParameterCount());
179+
180+
// Add some bindings - count should still be based on SQL, not bindings
181+
metadata.put(
182+
1,
183+
ImmutableSqlParameter.builder()
184+
.type(ColumnInfoTypeName.INT)
185+
.cardinal(1)
186+
.value(123)
187+
.build());
188+
189+
assertEquals(3, metadata.getParameterCount());
190+
}
191+
192+
@Test
193+
public void testParameterCountWithInsertStatement() throws SQLException {
194+
String sql = "INSERT INTO table (col1, col2, col3) VALUES (?, ?, ?)";
195+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
196+
assertEquals(3, metadata.getParameterCount());
197+
}
198+
199+
@Test
200+
public void testParameterCountWithUpdateStatement() throws SQLException {
201+
String sql = "UPDATE table SET col1 = ?, col2 = ? WHERE id = ? AND status = ?";
202+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
203+
assertEquals(4, metadata.getParameterCount());
204+
}
205+
206+
@Test
207+
public void testParameterCountWithNestedQuotes() throws SQLException {
208+
String sql =
209+
"SELECT * FROM table WHERE col1 = 'outer ''inner ? not counted'' outer' AND col2 = ?";
210+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
211+
assertEquals(1, metadata.getParameterCount());
212+
}
213+
214+
@Test
215+
public void testParameterCountWithCarriageReturnInComment() throws SQLException {
216+
String sql = "SELECT * FROM table WHERE id = ? -- comment with ? \r\nAND name = ?";
217+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
218+
assertEquals(2, metadata.getParameterCount());
219+
}
220+
221+
@Test
222+
public void testParameterCountExceedsBindingsThrowsException() {
223+
String sql = "SELECT * FROM table WHERE id = ? AND name = ?"; // 2 parameters
224+
DatabricksParameterMetaData metadata = new DatabricksParameterMetaData(sql);
225+
226+
// Add 3 parameter bindings (more than the 2 parameters in SQL)
227+
metadata.put(
228+
1,
229+
ImmutableSqlParameter.builder().type(ColumnInfoTypeName.INT).cardinal(1).value(1).build());
230+
metadata.put(
231+
2,
232+
ImmutableSqlParameter.builder()
233+
.type(ColumnInfoTypeName.STRING)
234+
.cardinal(2)
235+
.value("test")
236+
.build());
237+
metadata.put(
238+
3,
239+
ImmutableSqlParameter.builder()
240+
.type(ColumnInfoTypeName.STRING)
241+
.cardinal(3)
242+
.value("extra")
243+
.build());
244+
245+
// getParameterCount should throw SQLException due to too many bindings
246+
SQLException exception = assertThrows(SQLException.class, metadata::getParameterCount);
247+
assertTrue(
248+
exception
249+
.getMessage()
250+
.contains("Number of parameter bindings (3) exceeds parameter count (2)"));
251+
}
70252
}

0 commit comments

Comments
 (0)