Skip to content

Commit 0691183

Browse files
committed
Address feedback again
1 parent c0ad2bf commit 0691183

2 files changed

Lines changed: 24 additions & 27 deletions

File tree

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

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ public class DatabricksCallableStatement extends DatabricksPreparedStatement
3737
private static final Pattern RETURN_VALUE_SYNTAX =
3838
Pattern.compile("\\{\\s*\\?\\s*=\\s*call\\b", Pattern.CASE_INSENSITIVE);
3939

40+
/** Matches JDBC callable escape syntax: {@code {call proc(...)}}. */
41+
private static final Pattern CALL_ESCAPE_SYNTAX =
42+
Pattern.compile("\\{\\s*call\\s+([^}]*)\\}", Pattern.CASE_INSENSITIVE);
43+
4044
private static final String OUT_PARAM_NOT_SUPPORTED =
4145
"OUT and INOUT parameters are not supported. "
4246
+ "Only IN parameters are supported for callable statements. "
@@ -47,34 +51,27 @@ public class DatabricksCallableStatement extends DatabricksPreparedStatement
4751

4852
public DatabricksCallableStatement(DatabricksConnection connection, String sql)
4953
throws SQLException {
50-
super(connection, sql);
54+
// Convert {call proc(?)} to CALL proc(?) before passing to parent so that:
55+
// 1. The parent's shouldReturnResultSet matches CALL_PATTERN correctly
56+
// 2. The conversion is independent of the escapeProcessing flag
57+
// 3. Other JDBC escape sequences ({d ...}, {fn ...}, etc.) remain controlled
58+
// by setEscapeProcessing as usual
59+
super(connection, convertCallEscapeSyntax(sql));
5160
validateNoReturnValueSyntax(sql);
52-
// Enable escape processing for callable statements so {call proc(?)} is
53-
// converted to CALL proc(?). This overrides DEFAULT_ESCAPE_PROCESSING (false)
54-
// only for this statement instance — other statement types are unaffected.
55-
setEscapeProcessing(true);
56-
// Stored procedures can return result sets, so always allow executeQuery().
57-
// The parent computes shouldReturnResultSet from the raw SQL ({call ...} doesn't
58-
// match CALL_PATTERN), so we override it here.
59-
this.shouldReturnResultSet = true;
6061
LOGGER.debug("Created DatabricksCallableStatement for SQL: {}", sql);
6162
}
6263

6364
/**
64-
* Escape processing is required for callable statements to convert {@code {call proc(?)}} to
65-
* {@code CALL proc(?)}. Disabling it is rejected with a warning — callers using native {@code
66-
* CALL} syntax directly are unaffected since the escape conversion is a no-op for that form.
65+
* Converts JDBC callable escape syntax {@code {call proc(?, ?)}} to native {@code CALL proc(?,
66+
* ?)}. This is done unconditionally (not gated by escapeProcessing) because the {@code {call
67+
* ...}} form is specific to callable statements and must always be resolved. Other JDBC escape
68+
* sequences are handled independently by the standard escape processing path.
6769
*/
68-
@Override
69-
public void setEscapeProcessing(boolean enable) throws SQLException {
70-
if (!enable) {
71-
LOGGER.warn(
72-
"setEscapeProcessing(false) ignored for CallableStatement — "
73-
+ "escape processing is required to convert {call ...} to CALL syntax. "
74-
+ "If using native CALL syntax, this has no effect.");
75-
return;
70+
private static String convertCallEscapeSyntax(String sql) {
71+
if (sql == null) {
72+
return null;
7673
}
77-
super.setEscapeProcessing(true);
74+
return CALL_ESCAPE_SYNTAX.matcher(sql).replaceAll("CALL $1");
7875
}
7976

8077
private void validateNoReturnValueSyntax(String sql) throws SQLException {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -304,16 +304,16 @@ void testParameterCountNoArgs() throws Exception {
304304
}
305305

306306
@Test
307-
@DisplayName("setEscapeProcessing(false) is rejected for callable statements")
308-
void testSetEscapeProcessingFalseRejected() throws Exception {
307+
@DisplayName("{call ...} conversion is independent of escapeProcessing flag")
308+
void testCallConversionIndependentOfEscapeProcessing() throws Exception {
309309
DatabricksConnection connection = createConnection();
310310
DatabricksCallableStatement stmt = new DatabricksCallableStatement(connection, CALL_SQL);
311311

312-
// Should not throw, but should be silently ignored
313-
assertDoesNotThrow(() -> stmt.setEscapeProcessing(false));
312+
// Disable escape processing — {call ...} should still be converted
313+
stmt.setEscapeProcessing(false);
314314

315-
// Verify escape processing is still active by executing — the mock expects
316-
// the converted SQL, which only happens if escape processing is on
315+
// The mock expects CALL_SQL_AS_EXECUTED ("CALL my_proc(?, ?)") because
316+
// the {call} → CALL conversion happens at construction, not via escape processing
317317
when(client.executeStatement(
318318
eq(CALL_SQL_AS_EXECUTED),
319319
eq(new Warehouse(WAREHOUSE_ID)),

0 commit comments

Comments
 (0)