Skip to content

Commit ff21795

Browse files
bgraingerbgrainger
committed
Only insert a semicolon when concatenating SQL. Fixes #1133
This avoids modifying the user's query if they're executing a single statement (not using MySqlBatch). This undoes statement modification that was introduced in 378c4b8.
1 parent c2697b1 commit ff21795

9 files changed

+44
-32
lines changed

‎src/MySqlConnector/Core/BatchedCommandPayloadCreator.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ internal sealed class BatchedCommandPayloadCreator : ICommandPayloadCreator
88
{
99
public static ICommandPayloadCreator Instance { get; } = new BatchedCommandPayloadCreator();
1010

11-
public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer)
11+
public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer, bool appendSemicolon)
1212
{
1313
writer.Write((byte) CommandKind.Multi);
1414
bool? firstResult = default;
@@ -20,7 +20,7 @@ public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDict
2020
var position = writer.Position;
2121
writer.Write(padding);
2222

23-
wroteCommand = SingleCommandPayloadCreator.Instance.WriteQueryCommand(ref commandListPosition, cachedProcedures, writer);
23+
wroteCommand = SingleCommandPayloadCreator.Instance.WriteQueryCommand(ref commandListPosition, cachedProcedures, writer, appendSemicolon);
2424
firstResult ??= wroteCommand;
2525

2626
// write command length

‎src/MySqlConnector/Core/CommandExecutor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public static async Task<MySqlDataReader> ExecuteReaderAsync(IReadOnlyList<IMySq
4242

4343
var writer = new ByteBufferWriter();
4444
//// cachedProcedures will be non-null if there is a stored procedure, which is also the only time it will be read
45-
if (!payloadCreator.WriteQueryCommand(ref commandListPosition, cachedProcedures!, writer))
45+
if (!payloadCreator.WriteQueryCommand(ref commandListPosition, cachedProcedures!, writer, false))
4646
throw new InvalidOperationException("ICommandPayloadCreator failed to write query payload");
4747

4848
cancellationToken.ThrowIfCancellationRequested();

‎src/MySqlConnector/Core/ConcatenatedCommandPayloadCreator.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ internal sealed class ConcatenatedCommandPayloadCreator : ICommandPayloadCreator
88
{
99
public static ICommandPayloadCreator Instance { get; } = new ConcatenatedCommandPayloadCreator();
1010

11-
public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer)
11+
public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer, bool appendSemicolon)
1212
{
1313
if (commandListPosition.CommandIndex == commandListPosition.Commands.Count)
1414
return false;
@@ -33,7 +33,7 @@ public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDict
3333
if (Log.IsTraceEnabled())
3434
Log.Trace("Session{0} Preparing command payload; CommandText: {1}", command.Connection!.Session.Id, command.CommandText);
3535

36-
isComplete = SingleCommandPayloadCreator.WriteQueryPayload(command, cachedProcedures, writer);
36+
isComplete = SingleCommandPayloadCreator.WriteQueryPayload(command, cachedProcedures, writer, commandListPosition.CommandIndex < commandListPosition.Commands.Count - 1 || appendSemicolon);
3737
commandListPosition.CommandIndex++;
3838
}
3939
while (commandListPosition.CommandIndex < commandListPosition.Commands.Count && isComplete);

‎src/MySqlConnector/Core/ICommandPayloadCreator.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ internal interface ICommandPayloadCreator
1313
/// <param name="commandListPosition">The command list and its current position. This will be updated to the position of the next command to write (or past the end if there are no more commands).</param>
1414
/// <param name="cachedProcedures">A <see cref="CachedProcedure"/> for all the stored procedures in the command list, if any.</param>
1515
/// <param name="writer">The <see cref="ByteBufferWriter"/> to write the payload to.</param>
16+
/// <param name="appendSemicolon">Whether a statement-separating semicolon should be appended if it's missing.</param>
1617
/// <returns><c>true</c> if a command was written; otherwise, <c>false</c> (if there were no more commands in the list).</returns>
17-
bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer);
18+
bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer, bool appendSemicolon);
1819
}

‎src/MySqlConnector/Core/SingleCommandPayloadCreator.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ internal sealed class SingleCommandPayloadCreator : ICommandPayloadCreator
1313
// with this as the first column name, the result set will be treated as 'out' parameters for the previous command.
1414
public static string OutParameterSentinelColumnName => "\uE001\b\x0B";
1515

16-
public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer)
16+
public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer, bool appendSemicolon)
1717
{
1818
if (commandListPosition.CommandIndex == commandListPosition.Commands.Count)
1919
return false;
@@ -44,7 +44,7 @@ public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDict
4444
Log.Warn("Session{0} has query attributes but server doesn't support them; CommandText: {1}", command.Connection!.Session.Id, command.CommandText);
4545
}
4646

47-
WriteQueryPayload(command, cachedProcedures, writer);
47+
WriteQueryPayload(command, cachedProcedures, writer, appendSemicolon);
4848

4949
commandListPosition.CommandIndex++;
5050
}
@@ -69,9 +69,10 @@ public bool WriteQueryCommand(ref CommandListPosition commandListPosition, IDict
6969
/// <param name="command">The command.</param>
7070
/// <param name="cachedProcedures">The cached procedures.</param>
7171
/// <param name="writer">The output writer.</param>
72+
/// <param name="appendSemicolon">Whether a statement-separating semicolon should be appended if it's missing.</param>
7273
/// <returns><c>true</c> if a complete command was written; otherwise, <c>false</c>.</returns>
73-
public static bool WriteQueryPayload(IMySqlCommand command, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer) =>
74-
(command.CommandType == CommandType.StoredProcedure) ? WriteStoredProcedure(command, cachedProcedures, writer) : WriteCommand(command, writer);
74+
public static bool WriteQueryPayload(IMySqlCommand command, IDictionary<string, CachedProcedure?> cachedProcedures, ByteBufferWriter writer, bool appendSemicolon) =>
75+
(command.CommandType == CommandType.StoredProcedure) ? WriteStoredProcedure(command, cachedProcedures, writer) : WriteCommand(command, writer, appendSemicolon);
7576

7677
private static void WritePreparedStatement(IMySqlCommand command, PreparedStatement preparedStatement, ByteBufferWriter writer)
7778
{
@@ -242,7 +243,7 @@ private static bool WriteStoredProcedure(IMySqlCommand command, IDictionary<stri
242243
return preparer.ParseAndBindParameters(writer);
243244
}
244245

245-
private static bool WriteCommand(IMySqlCommand command, ByteBufferWriter writer)
246+
private static bool WriteCommand(IMySqlCommand command, ByteBufferWriter writer, bool appendSemicolon)
246247
{
247248
var isSchemaOnly = (command.CommandBehavior & CommandBehavior.SchemaOnly) != 0;
248249
var isSingleRow = (command.CommandBehavior & CommandBehavior.SingleRow) != 0;
@@ -256,7 +257,7 @@ private static bool WriteCommand(IMySqlCommand command, ByteBufferWriter writer)
256257
ReadOnlySpan<byte> setSqlSelectLimit1 = new byte[] { 83, 69, 84, 32, 115, 113, 108, 95, 115, 101, 108, 101, 99, 116, 95, 108, 105, 109, 105, 116, 61, 49, 59, 10 }; // SET sql_select_limit=1;\n
257258
writer.Write(setSqlSelectLimit1);
258259
}
259-
var preparer = new StatementPreparer(command.CommandText!, command.RawParameters, command.CreateStatementPreparerOptions());
260+
var preparer = new StatementPreparer(command.CommandText!, command.RawParameters, command.CreateStatementPreparerOptions() | ((appendSemicolon || isSchemaOnly || isSingleRow) ? StatementPreparerOptions.AppendSemicolon : StatementPreparerOptions.None));
260261
var isComplete = preparer.ParseAndBindParameters(writer);
261262
if (isComplete && (isSchemaOnly || isSingleRow))
262263
{

‎src/MySqlConnector/Core/StatementPreparer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ protected override void OnParsed(FinalParseStates states)
9292
m_writer.Write(Preparer.m_commandText, m_lastIndex, Preparer.m_commandText.Length - m_lastIndex);
9393
if ((states & FinalParseStates.NeedsNewline) == FinalParseStates.NeedsNewline)
9494
m_writer.Write((byte) '\n');
95-
if ((states & FinalParseStates.NeedsSemicolon) == FinalParseStates.NeedsSemicolon)
95+
if ((states & FinalParseStates.NeedsSemicolon) == FinalParseStates.NeedsSemicolon && (Preparer.Options & StatementPreparerOptions.AppendSemicolon) == StatementPreparerOptions.AppendSemicolon)
9696
m_writer.Write((byte) ';');
9797
IsComplete = (states & FinalParseStates.Complete) == FinalParseStates.Complete;
9898
}

‎src/MySqlConnector/Core/StatementPreparerOptions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,5 @@ internal enum StatementPreparerOptions
1515
GuidFormatLittleEndianBinary16 = 0xA0,
1616
GuidFormatMask = 0xE0,
1717
NoBackslashEscapes = 0x100,
18+
AppendSemicolon = 0x200,
1819
}

‎src/MySqlConnector/MySqlDataReader.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ internal async Task<bool> NextResultAsync(IOBehavior ioBehavior, CancellationTok
7373
using (Command.CancellableCommand.RegisterCancel(cancellationToken))
7474
{
7575
var writer = new ByteBufferWriter();
76-
if (!Command.Connection!.Session.IsCancelingQuery && m_payloadCreator.WriteQueryCommand(ref m_commandListPosition, m_cachedProcedures!, writer))
76+
if (!Command.Connection!.Session.IsCancelingQuery && m_payloadCreator.WriteQueryCommand(ref m_commandListPosition, m_cachedProcedures!, writer, false))
7777
{
7878
using var payload = writer.ToPayloadData();
7979
await Command.Connection.Session.SendAsync(payload, ioBehavior, cancellationToken).ConfigureAwait(false);

‎tests/MySqlConnector.Tests/StatementPreparerTests.cs

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public void Bug429(string sql)
2323
var parameters = new MySqlParameterCollection();
2424
parameters.AddWithValue("@param", 123);
2525
var parsedSql = GetParsedSql(sql, parameters);
26-
Assert.Equal(sql.Replace("@param", "123") + ";", parsedSql);
26+
Assert.Equal(sql.Replace("@param", "123"), parsedSql);
2727
}
2828

2929
[Theory]
@@ -131,7 +131,7 @@ public void Bug589(string sql)
131131
var parameters = new MySqlParameterCollection();
132132
parameters.AddWithValue("@foo", 22);
133133
var parsedSql = GetParsedSql(sql, parameters, StatementPreparerOptions.AllowUserVariables);
134-
Assert.Equal(sql.Replace("@foo", "22") + ";", parsedSql);
134+
Assert.Equal(sql.Replace("@foo", "22"), parsedSql);
135135
}
136136

137137
[Theory]
@@ -200,24 +200,33 @@ public void GuidFormat(object options, string replacedValue)
200200
}
201201

202202
[Theory]
203-
[InlineData("SELECT 1;", "SELECT 1;", true)]
204-
[InlineData("SELECT 1", "SELECT 1;", true)]
205-
[InlineData("SELECT 1 -- comment", "SELECT 1 -- comment\n;", true)]
206-
[InlineData("SELECT 1 # comment", "SELECT 1 # comment\n;", true)]
207-
[InlineData("SELECT '1", "SELECT '1", false)]
208-
[InlineData("SELECT '1' /* test", "SELECT '1' /* test", false)]
209-
[InlineData("SELECT '1';", "SELECT '1';", true)]
210-
[InlineData("SELECT '1'", "SELECT '1';", true)]
211-
[InlineData("SELECT \"1\";", "SELECT \"1\";", true)]
212-
[InlineData("SELECT \"1\"", "SELECT \"1\";", true)]
213-
[InlineData("SELECT * FROM `SELECT`;", "SELECT * FROM `SELECT`;", true)]
214-
[InlineData("SELECT * FROM `SELECT`", "SELECT * FROM `SELECT`;", true)]
215-
[InlineData("SELECT * FROM test WHERE id = ?;", "SELECT * FROM test WHERE id = 0;", true)]
216-
[InlineData("SELECT * FROM test WHERE id = ?", "SELECT * FROM test WHERE id = 0;", true)]
217-
public void CompleteStatements(string sql, string expectedSql, bool expectedComplete)
203+
[InlineData("SELECT 1;", "SELECT 1;", false, true)]
204+
[InlineData("SELECT 1;", "SELECT 1;", true, true)]
205+
[InlineData("SELECT 1", "SELECT 1", false, true)]
206+
[InlineData("SELECT 1", "SELECT 1;", true, true)]
207+
[InlineData("SELECT 1 -- comment", "SELECT 1 -- comment\n", false, true)]
208+
[InlineData("SELECT 1 -- comment", "SELECT 1 -- comment\n;", true, true)]
209+
[InlineData("SELECT 1 # comment", "SELECT 1 # comment\n", false, true)]
210+
[InlineData("SELECT 1 # comment", "SELECT 1 # comment\n;", true, true)]
211+
[InlineData("SELECT '1", "SELECT '1", false, false)]
212+
[InlineData("SELECT '1", "SELECT '1", true, false)]
213+
[InlineData("SELECT '1' /* test", "SELECT '1' /* test", false, false)]
214+
[InlineData("SELECT '1';", "SELECT '1';", false, true)]
215+
[InlineData("SELECT '1';", "SELECT '1';", true, true)]
216+
[InlineData("SELECT '1'", "SELECT '1'", false, true)]
217+
[InlineData("SELECT '1'", "SELECT '1';", true, true)]
218+
[InlineData("SELECT \"1\";", "SELECT \"1\";", false, true)]
219+
[InlineData("SELECT \"1\";", "SELECT \"1\";", true, true)]
220+
[InlineData("SELECT \"1\"", "SELECT \"1\"", false, true)]
221+
[InlineData("SELECT \"1\"", "SELECT \"1\";", true, true)]
222+
[InlineData("SELECT * FROM `SELECT`;", "SELECT * FROM `SELECT`;", false, true)]
223+
[InlineData("SELECT * FROM `SELECT`", "SELECT * FROM `SELECT`", false, true)]
224+
[InlineData("SELECT * FROM test WHERE id = ?;", "SELECT * FROM test WHERE id = 0;", false, true)]
225+
[InlineData("SELECT * FROM test WHERE id = ?", "SELECT * FROM test WHERE id = 0", false, true)]
226+
public void CompleteStatements(string sql, string expectedSql, bool appendSemicolon, bool expectedComplete)
218227
{
219228
var parameters = new MySqlParameterCollection { new() { Value = 0 } };
220-
var preparer = new StatementPreparer(sql, parameters, new StatementPreparerOptions());
229+
var preparer = new StatementPreparer(sql, parameters, appendSemicolon ? StatementPreparerOptions.AppendSemicolon : StatementPreparerOptions.None);
221230
var writer = new ByteBufferWriter();
222231
var isComplete = preparer.ParseAndBindParameters(writer);
223232
Assert.Equal(expectedComplete, isComplete);

0 commit comments

Comments
 (0)