Skip to content

Commit 2594199

Browse files
rojiroji
authored
Implement raw SQL mode for prepared, non-batched commands (take 2) (#3852)
Part of #1042 Leads to #2317
1 parent f239fdd commit 2594199

File tree

9 files changed

+378
-118
lines changed

9 files changed

+378
-118
lines changed

‎src/Npgsql/Internal/NpgsqlConnector.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ public sealed partial class NpgsqlConnector : IDisposable
139139

140140
internal PreparedStatementManager PreparedStatementManager;
141141

142+
internal SqlQueryParser SqlQueryParser { get; } = new();
143+
142144
/// <summary>
143145
/// If the connector is currently in COPY mode, holds a reference to the importer/exporter object.
144146
/// Otherwise null.

‎src/Npgsql/MultiplexingConnectorPool.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ public bool IsBootstrapped
2828
readonly ChannelReader<NpgsqlCommand>? _multiplexCommandReader;
2929
internal ChannelWriter<NpgsqlCommand>? MultiplexCommandWriter { get; }
3030

31-
const int WriteCoalescingDelayAdaptivityUs = 10;
32-
3331
/// <summary>
3432
/// A pool-wide type mapper used when multiplexing. This is necessary because binding parameters
3533
/// to their type handlers happens *before* the command is enqueued for execution, so there's no
@@ -388,9 +386,9 @@ void Flush(NpgsqlConnector connector, ref MultiplexingStats stats)
388386

389387
void FailWrite(NpgsqlConnector connector, Exception exception)
390388
{
391-
// Note that all commands already passed validation before being enqueued. This means any error
392-
// here is either an unrecoverable network issue (in which case we're already broken), or some other
393-
// issue while writing (e.g. invalid UTF8 characters in the SQL query) - unrecoverable in any case.
389+
// Note that all commands already passed validation. This means any error here is either an unrecoverable network issue
390+
// (in which case we're already broken), or some other issue while writing (e.g. invalid UTF8 characters in the SQL query) -
391+
// unrecoverable in any case.
394392

395393
// All commands enqueued in CommandsInFlightWriter will be drained by the reader and failed.
396394
// Note that some of these commands where only written to the connector's buffer, but never

‎src/Npgsql/NpgsqlCommand.cs‎

Lines changed: 105 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,9 @@ internal void DeriveParameters()
386386
var conn = CheckAndGetConnection();
387387
Debug.Assert(conn is not null);
388388

389+
if (string.IsNullOrEmpty(CommandText))
390+
throw new InvalidOperationException("CommandText property has not been initialized");
391+
389392
using var _ = conn.StartTemporaryBindingScope(out var connector);
390393

391394
if (Statements.Any(s => s.PreparedStatement?.IsExplicit == true))
@@ -482,7 +485,8 @@ void DeriveParametersForQuery(NpgsqlConnector connector)
482485
using (connector.StartUserAction())
483486
{
484487
Log.Debug($"Deriving Parameters for query: {CommandText}", connector.Id);
485-
ProcessRawQuery(connector.UseConformingStrings, deriveParameters: true);
488+
489+
connector.SqlQueryParser.ParseRawQuery(CommandText, _parameters, _statements, connector.UseConformingStrings, deriveParameters: true);
486490

487491
var sendTask = SendDeriveParameters(connector, false);
488492
if (sendTask.IsFaulted)
@@ -578,11 +582,16 @@ Task Prepare(bool async, CancellationToken cancellationToken = default)
578582
throw new NotSupportedException("Explicit preparation not supported with multiplexing");
579583
var connector = connection.Connector!;
580584

581-
for (var i = 0; i < Parameters.Count; i++)
582-
Parameters[i].Bind(connector.TypeMapper);
585+
foreach (var p in Parameters.InternalList)
586+
{
587+
Parameters.CalculatePlaceholderType(p);
588+
p.Bind(connector.TypeMapper);
589+
}
590+
591+
ProcessRawQuery(connector.SqlQueryParser, connector.UseConformingStrings);
583592

584-
ProcessRawQuery(connector.UseConformingStrings, deriveParameters: false);
585-
Log.Debug($"Preparing: {CommandText}", connector.Id);
593+
if (Log.IsEnabled(NpgsqlLogLevel.Debug))
594+
Log.Debug($"Preparing: {CommandText}", connector.Id);
586595

587596
var needToPrepare = false;
588597
foreach (var statement in _statements)
@@ -740,31 +749,49 @@ async Task Unprepare(bool async, CancellationToken cancellationToken = default)
740749

741750
#region Query analysis
742751

743-
internal void ProcessRawQuery(bool standardConformingStrings, bool deriveParameters)
752+
internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStrings)
744753
{
745754
if (string.IsNullOrEmpty(CommandText))
746755
throw new InvalidOperationException("CommandText property has not been initialized");
747756

748757
NpgsqlStatement statement;
758+
749759
switch (CommandType) {
750760
case CommandType.Text:
751-
var parser = new SqlQueryParser();
752-
parser.ParseRawQuery(CommandText, _parameters, _statements, standardConformingStrings, deriveParameters);
761+
switch (Parameters.PlaceholderType)
762+
{
763+
case PlaceholderType.Positional:
764+
// In positional parameter mode, we don't need to parse/rewrite the CommandText or reorder the parameters - just use
765+
// them as is. If the SQL contains a semicolon (legacy batching) when positional parameters are in use, we just send
766+
// that and PostgreSQL will error (this behavior is by-design - use the new batching API).
767+
statement = TruncateStatementsToOne();
768+
statement.SQL = CommandText;
769+
statement.InputParameters = Parameters.InternalList;
770+
break;
771+
772+
case PlaceholderType.NoParameters:
773+
// Note that queries with no parameters are parsed just like queries with named parameters, since they may contain a
774+
// semicolon (legacy batching).
775+
case PlaceholderType.Named:
776+
parser ??= new SqlQueryParser();
777+
parser.ParseRawQuery(CommandText, _parameters, _statements, standardConformingStrings);
778+
779+
if (_statements.Count > 1 && _parameters.HasOutputParameters)
780+
throw new NotSupportedException("Commands with multiple queries cannot have out parameters");
781+
break;
782+
783+
case PlaceholderType.Mixed:
784+
throw new NotSupportedException("Mixing named and positional parameters isn't supported");
785+
786+
default:
787+
throw new ArgumentOutOfRangeException(
788+
nameof(PlaceholderType), $"Unknown {nameof(PlaceholderType)} value: {Parameters.PlaceholderType}");
789+
}
753790

754-
if (_statements.Count > 1 && _parameters.HasOutputParameters)
755-
throw new NotSupportedException("Commands with multiple queries cannot have out parameters");
756791
break;
757792

758793
case CommandType.TableDirect:
759-
if (_statements.Count == 0)
760-
statement = new NpgsqlStatement();
761-
else
762-
{
763-
statement = _statements[0];
764-
statement.Reset();
765-
_statements.Clear();
766-
}
767-
_statements.Add(statement);
794+
statement = TruncateStatementsToOne();
768795
statement.SQL = "SELECT * FROM " + CommandText;
769796
break;
770797

@@ -778,7 +805,7 @@ internal void ProcessRawQuery(bool standardConformingStrings, bool deriveParamet
778805
var hasWrittenFirst = false;
779806
for (var i = 1; i <= numInput; i++) {
780807
var param = inputList[i - 1];
781-
if (param.TrimmedName == "")
808+
if (param.IsPositional)
782809
{
783810
if (hasWrittenFirst)
784811
sb.Append(',');
@@ -790,7 +817,7 @@ internal void ProcessRawQuery(bool standardConformingStrings, bool deriveParamet
790817
for (var i = 1; i <= numInput; i++)
791818
{
792819
var param = inputList[i - 1];
793-
if (param.TrimmedName != "")
820+
if (!param.IsPositional)
794821
{
795822
if (hasWrittenFirst)
796823
sb.Append(',');
@@ -804,17 +831,9 @@ internal void ProcessRawQuery(bool standardConformingStrings, bool deriveParamet
804831
}
805832
sb.Append(')');
806833

807-
if (_statements.Count == 0)
808-
statement = new NpgsqlStatement();
809-
else
810-
{
811-
statement = _statements[0];
812-
statement.Reset();
813-
_statements.Clear();
814-
}
834+
statement = TruncateStatementsToOne();
815835
statement.SQL = sb.ToString();
816836
statement.InputParameters.AddRange(inputList);
817-
_statements.Add(statement);
818837
break;
819838
default:
820839
throw new InvalidOperationException($"Internal Npgsql bug: unexpected value {CommandType} of enum {nameof(CommandType)}. Please file a bug.");
@@ -831,15 +850,43 @@ internal void ProcessRawQuery(bool standardConformingStrings, bool deriveParamet
831850

832851
void ValidateParameters(ConnectorTypeMapper typeMapper)
833852
{
834-
for (var i = 0; i < Parameters.Count; i++)
853+
var hasOutputParameters = false;
854+
855+
foreach (var p in Parameters.InternalList)
835856
{
836-
var p = Parameters[i];
837-
if (!p.IsInputDirection)
857+
Parameters.CalculatePlaceholderType(p);
858+
859+
switch (p.Direction)
860+
{
861+
case ParameterDirection.Input:
862+
break;
863+
864+
case ParameterDirection.InputOutput:
865+
if (Parameters.PlaceholderType == PlaceholderType.Positional)
866+
throw new NotSupportedException("Output parameters are not supported in positional mode");
867+
hasOutputParameters = true;
868+
break;
869+
870+
case ParameterDirection.Output:
871+
if (Parameters.PlaceholderType == PlaceholderType.Positional)
872+
throw new NotSupportedException("Output parameters are not supported in positional mode");
873+
hasOutputParameters = true;
874+
continue;
875+
876+
case ParameterDirection.ReturnValue:
838877
continue;
878+
879+
default:
880+
throw new ArgumentOutOfRangeException(nameof(ParameterDirection),
881+
$"Unhandled {nameof(ParameterDirection)} value: {p.Direction}");
882+
}
883+
839884
p.Bind(typeMapper);
840885
p.LengthCache?.Clear();
841886
p.ValidateAndGetLength();
842887
}
888+
889+
Parameters.HasOutputParameters = hasOutputParameters;
843890
}
844891

845892
#endregion
@@ -1224,7 +1271,7 @@ internal async ValueTask<NpgsqlDataReader> ExecuteReader(CommandBehavior behavio
12241271
break;
12251272

12261273
case false:
1227-
ProcessRawQuery(connector.UseConformingStrings, deriveParameters: false);
1274+
ProcessRawQuery(connector.SqlQueryParser, connector.UseConformingStrings);
12281275

12291276
if (connector.Settings.MaxAutoPrepare > 0)
12301277
{
@@ -1317,7 +1364,7 @@ internal async ValueTask<NpgsqlDataReader> ExecuteReader(CommandBehavior behavio
13171364
}
13181365

13191366
ValidateParameters(pool.MultiplexingTypeMapper!);
1320-
ProcessRawQuery(standardConformingStrings: true, deriveParameters: false);
1367+
ProcessRawQuery(null, standardConformingStrings: true);
13211368

13221369
State = CommandState.InProgress;
13231370

@@ -1440,6 +1487,29 @@ protected override void Dispose(bool disposing)
14401487

14411488
#region Misc
14421489

1490+
NpgsqlStatement TruncateStatementsToOne()
1491+
{
1492+
switch (_statements.Count)
1493+
{
1494+
case 0:
1495+
var statement = new NpgsqlStatement();
1496+
_statements.Add(statement);
1497+
return statement;
1498+
1499+
case 1:
1500+
statement = _statements[0];
1501+
statement.Reset();
1502+
return statement;
1503+
1504+
default:
1505+
statement = _statements[0];
1506+
statement.Reset();
1507+
_statements.Clear();
1508+
_statements.Add(statement);
1509+
return statement;
1510+
}
1511+
}
1512+
14431513
/// <summary>
14441514
/// Fixes up the text/binary flag on result columns.
14451515
/// Since Prepare() describes a statement rather than a portal, the resulting RowDescription

0 commit comments

Comments
 (0)