Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #2192: Added automatic spacing between parameter names for prepared SQL #2196

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 75 additions & 20 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -7389,7 +7389,7 @@ void endRequestInternal() throws SQLException {
String replaceParameterMarkers(String sqlSrc, int[] paramPositions, Parameter[] params,
boolean isReturnValueSyntax) {
final int MAX_PARAM_NAME_LEN = 6;
char[] sqlDst = new char[sqlSrc.length() + params.length * (MAX_PARAM_NAME_LEN + OUT.length)];
char[] sqlDst = new char[sqlSrc.length() + (params.length * (MAX_PARAM_NAME_LEN + OUT.length)) + (params.length * 2)];
int dstBegin = 0;
int srcBegin = 0;
int nParam = 0;
Expand All @@ -7403,8 +7403,8 @@ String replaceParameterMarkers(String sqlSrc, int[] paramPositions, Parameter[]
if (sqlSrc.length() == srcEnd)
break;

dstBegin += makeParamName(nParam++, sqlDst, dstBegin);
srcBegin = srcEnd + 1;
dstBegin += makeParamName(nParam++, sqlDst, dstBegin, true);
srcBegin = srcEnd + 1 <= sqlSrc.length() - 1 && sqlSrc.charAt(srcEnd + 1) == ' ' ? srcEnd + 2 : srcEnd + 1;

if (params[paramIndex++].isOutput() && (!isReturnValueSyntax || paramIndex > 1)) {
System.arraycopy(OUT, 0, sqlDst, dstBegin, OUT.length);
Expand All @@ -7423,33 +7423,88 @@ String replaceParameterMarkers(String sqlSrc, int[] paramPositions, Parameter[]
* @param name
* the parameter name
* @param offset
* the offset
* @param isPreparedSQL
* if the param is for build preparedSQL
* @return int
*/
static int makeParamName(int nParam, char[] name, int offset) {
name[offset + 0] = '@';
name[offset + 1] = 'P';
static int makeParamName(int nParam, char[] name, int offset, boolean isPreparedSQL) {
buildParamInitial(name, offset, isPreparedSQL);
if (nParam < 10) {
name[offset + 2] = (char) ('0' + nParam);
return 3;
return buildParamLt10(nParam, name, offset, isPreparedSQL);
} else {
if (nParam < 100) {
int nBase = 2;
while (true) { // make a char[] representation of the param number 2.26
if (nParam < nBase * 10) {
name[offset + 2] = (char) ('0' + (nBase - 1));
name[offset + 3] = (char) ('0' + (nParam - ((nBase - 1) * 10)));
return 4;
}
nBase++;
}
return buildParamLt100(nParam, name, offset, isPreparedSQL);
} else {
String sParam = "" + nParam;
sParam.getChars(0, sParam.length(), name, offset + 2);
return 2 + sParam.length();
return buildParamMt100(nParam, name, offset, isPreparedSQL);
}
}
}

private static void buildParamInitial(char[] name, int offset, boolean isPreparedSQL) {
int preparedSQLOffset = 0;
if (isPreparedSQL) {
name[offset + 0] = ' ';
preparedSQLOffset++;
}
name[offset + preparedSQLOffset + 0] = '@';
name[offset + preparedSQLOffset + 1] = 'P';
}

private static int buildParamLt10(int nParam, char[] name, int offset, boolean isPreparedSQL) {
int preparedSQLOffset = 0;

if (isPreparedSQL) {
preparedSQLOffset++;
}

name[offset + preparedSQLOffset + 2] = (char) ('0' + nParam);

if (isPreparedSQL) {
name[offset + 4] = ' ';
return 5;
}

return 3;
}

private static int buildParamLt100(int nParam, char[] name, int offset, boolean isPreparedSQL) {
int nBase = 2;
int preparedSQLOffset = 0;

if (isPreparedSQL) {
preparedSQLOffset = 1;
}

while (true) { // make a char[] representation of the param number 2.26
if (nParam < nBase * 10) {
name[offset + preparedSQLOffset + 2] = (char) ('0' + (nBase - 1));
name[offset + preparedSQLOffset + 3] = (char) ('0' + (nParam - ((nBase - 1) * 10)));

if (isPreparedSQL) {
name[offset + 5] = ' ';
preparedSQLOffset++;
}

return 4 + preparedSQLOffset;
}
nBase++;
}
}

private static int buildParamMt100(int nParam, char[] name, int offset, boolean isPreparedSQL) {
int preparedSQLOffset = 0;
String sParam = Integer.toString(nParam);

if (isPreparedSQL) {
preparedSQLOffset++;
sParam = nParam + " ";
}

sParam.getChars(0, sParam.length(), name, offset + preparedSQLOffset + 2);
return 2 + sParam.length() + preparedSQLOffset;
}

/**
* Notify any interested parties (e.g. pooling managers) of a ConnectionEvent activity on the connection. Calling
* notifyPooledConnection with null event will place this connection back in the pool. Calling
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ private String buildParamTypeDefinitions(Parameter[] params, boolean renewDefini
if (i > 0)
sb.append(',');

int l = SQLServerConnection.makeParamName(i, cParamName, 0);
int l = SQLServerConnection.makeParamName(i, cParamName, 0, false);
String parameterName = String.valueOf(cParamName, 0, l);
sb.append(parameterName);
sb.append(' ');
Expand Down Expand Up @@ -750,7 +750,7 @@ void sendParamsByRPC(TDSWriter tdsWriter, Parameter[] params) throws SQLServerEx
for (int index = 0; index < params.length; index++) {
if (JDBCType.TVP == params[index].getJdbcType()) {
cParamName = new char[10];
int paramNameLen = SQLServerConnection.makeParamName(index, cParamName, 0);
int paramNameLen = SQLServerConnection.makeParamName(index, cParamName, 0, false);
tdsWriter.writeByte((byte) paramNameLen);
tdsWriter.writeString(new String(cParamName, 0, paramNameLen));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class PreparedStatementTest extends AbstractTest {
final static String tableName = RandomUtil.getIdentifier("tableTestStatementPoolingInternal1");
final static String tableName2 = RandomUtil.getIdentifier("tableTestStatementPoolingInternal2");
final static String tableName3 = RandomUtil.getIdentifier("tableTestPreparedStatementWithSpPrepare");
final static String tableName4 = RandomUtil.getIdentifier("tableTestPreparedStatementWithMultipleParams");

@BeforeAll
public static void setupTests() throws Exception {
Expand Down Expand Up @@ -117,6 +118,94 @@ public void testPreparedStatementWithSpPrepare() throws SQLException {
}
}

@Test
public void testPreparedStatementParamNameSpacingWithMultipleParams() throws SQLException {
int paramNameCount = 105;

StringBuilder insertSql = new StringBuilder("insert into " + AbstractSQLGenerator.escapeIdentifier(tableName4)
+ "values(");

for (int i = 1; i <= paramNameCount; i++) {
insertSql.append(i % 10);

if (i != paramNameCount) {
insertSql.append(",");
} else {
insertSql.append(")");
}
}

StringBuilder createTableSql = new StringBuilder("create table " + AbstractSQLGenerator.escapeIdentifier(tableName4) + "(");

for (int i = 1; i <= paramNameCount; i++) {
createTableSql.append("c" + i + " char(1)");

if (i != paramNameCount) {
createTableSql.append(",");
} else {
createTableSql.append(")");
}
}

try (SQLServerConnection con = (SQLServerConnection) getConnection()) {
executeSQL(con, createTableSql.toString());
executeSQL(con, insertSql.toString());

// There are no typos in the queries eg. The 'c1=?and' is not a typo. We are testing the spacing, or lack of spacing.
// The driver should automatically space out the params eg. 'c1=?and' becomes 'c1= ? and' in the final string we send to the server.

// Testing with less than 10 params
String sql1 = "select * from " + AbstractSQLGenerator.escapeIdentifier(tableName4) + " where c1=?and c2=?";

// Testing with number of params between 10 and 100
String sql2 = "select * from " + AbstractSQLGenerator.escapeIdentifier(tableName4)
+ " where c1=?and c2=? and c3=?and c4=? and c5=? and c6=? and c7=? and c8=? and c9=? and c10=? and c11=? and c12=?";

// Testing with more than 100 params
StringBuilder sql3 = new StringBuilder("select * from " + AbstractSQLGenerator.escapeIdentifier(tableName4)
+ " where c1=?and ");

for (int i = 2; i <= paramNameCount; i++) {
sql3.append("c" + i + "=?");

if (i != paramNameCount) {
sql3.append(" and ");
}
}

try (SQLServerPreparedStatement ps = (SQLServerPreparedStatement) con.prepareStatement(sql1)) {
ps.setString(1, "1");
ps.setString(2, "2");
ps.executeQuery();
}

try (SQLServerPreparedStatement ps = (SQLServerPreparedStatement) con.prepareStatement(sql2)) {
ps.setString(1, "1");
ps.setString(2, "2");
ps.setString(3, "3");
ps.setString(4, "4");
ps.setString(5, "5");
ps.setString(6, "6");
ps.setString(7, "7");
ps.setString(8, "8");
ps.setString(9, "9");
ps.setString(10, "0");
ps.setString(11, "1");
ps.setString(12, "2");
ps.executeQuery();
}

try (SQLServerPreparedStatement ps = (SQLServerPreparedStatement) con.prepareStatement(sql3.toString())) {
ps.setString(1, "1");
for (int i = 2; i <= paramNameCount; i++) {
ps.setString(i, Integer.toString(i % 10));
}

ps.executeQuery();
}
}
}

@Test
public void testPreparedStatementPoolEvictionWithSpPrepare() throws SQLException {
int cacheSize = 3;
Expand Down Expand Up @@ -787,6 +876,7 @@ private static void dropTables() throws Exception {
TestUtils.dropTableIfExists(AbstractSQLGenerator.escapeIdentifier(tableName), stmt);
TestUtils.dropTableIfExists(AbstractSQLGenerator.escapeIdentifier(tableName2), stmt);
TestUtils.dropTableIfExists(AbstractSQLGenerator.escapeIdentifier(tableName3), stmt);
TestUtils.dropTableIfExists(AbstractSQLGenerator.escapeIdentifier(tableName4), stmt);
}
}

Expand Down