Skip to content

Commit c70ab8b

Browse files
rojiroji
authored
Implement AppContext-switch mode to disable SQL parsing/rewriting (#3921)
Also switches type-loading to use the new batching API. Closes #3920
1 parent 0d8dd34 commit c70ab8b

7 files changed

Lines changed: 125 additions & 32 deletions

File tree

‎src/Npgsql/NpgsqlBatch.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Data.Common;
44
using System.Threading;
55
using System.Threading.Tasks;
6+
using Npgsql.Internal;
67

78
namespace Npgsql
89
{
@@ -52,6 +53,17 @@ protected override DbTransaction? DbTransaction
5253
set => Transaction = (NpgsqlTransaction?)value;
5354
}
5455

56+
/// <summary>
57+
/// Marks all of the batch's result columns as either known or unknown.
58+
/// Unknown results column are requested them from PostgreSQL in text format, and Npgsql makes no
59+
/// attempt to parse them. They will be accessible as strings only.
60+
/// </summary>
61+
public bool AllResultTypesAreUnknown
62+
{
63+
get => _command.AllResultTypesAreUnknown;
64+
set => _command.AllResultTypesAreUnknown = value;
65+
}
66+
5567
/// <summary>
5668
/// Initializes a new <see cref="NpgsqlBatch"/>.
5769
/// </summary>
@@ -67,6 +79,13 @@ public NpgsqlBatch(NpgsqlConnection? connection = null, NpgsqlTransaction? trans
6779
Transaction = transaction;
6880
}
6981

82+
internal NpgsqlBatch(NpgsqlConnector connector)
83+
{
84+
var batchCommands = new List<NpgsqlBatchCommand>(5);
85+
_command = new(connector, batchCommands);
86+
BatchCommands = new NpgsqlBatchCommandCollection(batchCommands);
87+
}
88+
7089
/// <inheritdoc />
7190
protected override DbBatchCommand CreateDbBatchCommand()
7291
=> new NpgsqlBatchCommand();

‎src/Npgsql/NpgsqlCommand.cs‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ public sealed class NpgsqlCommand : DbCommand, ICloneable, IComponent
7070
/// </summary>
7171
bool _isCached;
7272

73+
#if DEBUG
74+
internal static bool EnableSqlRewriting;
75+
#else
76+
static readonly bool EnableSqlRewriting;
77+
#endif
78+
7379
static readonly List<NpgsqlParameter> EmptyParameters = new();
7480

7581
static readonly SingleThreadSynchronizationContext SingleThreadSynchronizationContext = new("NpgsqlRemainingAsyncSendWorker");
@@ -86,6 +92,9 @@ public sealed class NpgsqlCommand : DbCommand, ICloneable, IComponent
8692

8793
#region Constructors
8894

95+
static NpgsqlCommand()
96+
=> EnableSqlRewriting = !AppContext.TryGetSwitch("Npgsql.EnableSqlRewriting", out var enabled) || enabled;
97+
8998
/// <summary>
9099
/// Initializes a new instance of the <see cref="NpgsqlCommand"/> class.
91100
/// </summary>
@@ -140,7 +149,15 @@ internal NpgsqlCommand(List<NpgsqlBatchCommand> batchCommands)
140149
_parameters = null!;
141150
}
142151

143-
internal NpgsqlCommand(string? cmdText, NpgsqlConnector connector) : this(cmdText) => _connector = connector;
152+
internal NpgsqlCommand(string? cmdText, NpgsqlConnector connector) : this(cmdText)
153+
=> _connector = connector;
154+
155+
/// <summary>
156+
/// Used when this <see cref="NpgsqlCommand"/> instance is wrapped inside an <see cref="NpgsqlBatch"/>.
157+
/// </summary>
158+
internal NpgsqlCommand(NpgsqlConnector connector, List<NpgsqlBatchCommand> batchCommands)
159+
: this(batchCommands)
160+
=> _connector = connector;
144161

145162
internal static NpgsqlCommand CreateCachedCommand(NpgsqlConnection connection)
146163
=> new(null, connection) { _isCached = true };
@@ -818,9 +835,17 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
818835
break;
819836

820837
case PlaceholderType.NoParameters:
821-
// Note that queries with no parameters are parsed just like queries with named parameters, since they may contain a
822-
// semicolon (legacy batching).
838+
// Unless the EnableSqlRewriting AppContext switch is explicitly disabled, queries with no parameters are parsed just
839+
// like queries with named parameters, since they may contain a semicolon (legacy batching).
840+
if (EnableSqlRewriting)
841+
goto case PlaceholderType.Named;
842+
else
843+
goto case PlaceholderType.Positional;
844+
823845
case PlaceholderType.Named:
846+
if (!EnableSqlRewriting)
847+
throw new NotSupportedException($"Named parameters are not supported when Npgsql.{nameof(EnableSqlRewriting)} is disabled");
848+
824849
// The parser is cached on NpgsqlConnector - unless we're in multiplexing mode.
825850
parser ??= new SqlQueryParser();
826851

‎src/Npgsql/NpgsqlParameterCollection.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,8 @@ public sealed class NpgsqlParameterCollection : DbParameterCollection, IList<Npg
2626
#endif
2727

2828
static NpgsqlParameterCollection()
29-
{
30-
AppContext.TryGetSwitch("Npgsql.EnableLegacyCaseInsensitiveDbParameters", out var enabled);
31-
CaseInsensitiveCompatMode = enabled;
32-
}
29+
=> CaseInsensitiveCompatMode = AppContext.TryGetSwitch("Npgsql.EnableLegacyCaseInsensitiveDbParameters", out var enabled)
30+
&& enabled;
3331

3432
// Dictionary lookups for GetValue to improve performance
3533
Dictionary<string, int>? _lookup;

‎src/Npgsql/PostgresDatabaseInfo.cs‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,6 @@ internal async Task LoadPostgresInfo(NpgsqlConnector conn, NpgsqlTimeout timeout
103103
/// Generates a raw SQL query string to select type information.
104104
/// </summary>
105105
/// <param name="withRange">True to load range types.</param>
106-
/// <param name="withEnum">True to load enum types.</param>
107-
/// <param name="withEnumSortOrder"></param>
108106
/// <param name="loadTableComposites">True to load table composites.</param>
109107
/// <returns>
110108
/// A raw SQL query string that selects type information.
@@ -116,11 +114,8 @@ internal async Task LoadPostgresInfo(NpgsqlConnector conn, NpgsqlTimeout timeout
116114
/// For arrays and ranges, join in the element OID and type (to filter out arrays of unhandled
117115
/// types).
118116
/// </remarks>
119-
static string GenerateTypesQuery(bool withRange, bool withEnum, bool withEnumSortOrder,
120-
bool loadTableComposites)
117+
static string GenerateLoadTypesQuery(bool withRange, bool loadTableComposites)
121118
=> $@"
122-
SELECT version();
123-
124119
SELECT ns.nspname, typ_and_elem_type.*,
125120
CASE
126121
WHEN typtype IN ('b', 'e', 'p') THEN 0 -- First base types, enums, pseudo-types
@@ -165,8 +160,10 @@ elemtyptype IN ('b', 'r', 'e', 'd') OR -- Array of base, range, enum, domain
165160
(elemtyptype = 'p' AND elemtypname IN ('record', 'void')) OR -- Arrays of special supported pseudo-types
166161
(elemtyptype = 'c' AND {(loadTableComposites ? "ns.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')" : "elemrelkind='c'")}) -- Array of user-defined free-standing composites (not table composites) by default
167162
))
168-
ORDER BY ord;
163+
ORDER BY ord;";
169164

165+
static string GenerateLoadCompositeTypesQuery(bool loadTableComposites)
166+
=> $@"
170167
-- Load field definitions for (free-standing) composite types
171168
SELECT typ.oid, att.attname, att.atttypid
172169
FROM pg_type AS typ
@@ -177,15 +174,15 @@ JOIN pg_attribute AS att ON (att.attrelid = typ.typrelid)
177174
(typ.typtype = 'c' AND {(loadTableComposites ? "ns.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')" : "cls.relkind='c'")}) AND
178175
attnum > 0 AND -- Don't load system attributes
179176
NOT attisdropped
180-
ORDER BY typ.oid, att.attnum;
177+
ORDER BY typ.oid, att.attnum;";
181178

182-
{(withEnum ? $@"
179+
static string GenerateLoadEnumFieldsQuery(bool withEnumSortOrder)
180+
=> $@"
183181
-- Load enum fields
184182
SELECT pg_type.oid, enumlabel
185183
FROM pg_enum
186184
JOIN pg_type ON pg_type.oid=enumtypid
187-
ORDER BY oid{(withEnumSortOrder ? ", enumsortorder" : "")};" : "")}
188-
";
185+
ORDER BY oid{(withEnumSortOrder ? ", enumsortorder" : "")};";
189186

190187
/// <summary>
191188
/// Loads type information from the backend specified by <paramref name="conn"/>.
@@ -204,13 +201,24 @@ internal async Task<List<PostgresType>> LoadBackendTypes(NpgsqlConnector conn, N
204201
if (timeout.IsSet)
205202
commandTimeout = (int)timeout.CheckAndGetTimeLeft().TotalSeconds;
206203

207-
var typeLoadingQuery = GenerateTypesQuery(SupportsRangeTypes, SupportsEnumTypes, HasEnumSortOrder, conn.Settings.LoadTableComposites);
208-
using var command = conn.CreateCommand(typeLoadingQuery);
209-
command.CommandTimeout = commandTimeout;
210-
command.AllResultTypesAreUnknown = true;
204+
using var batch = new NpgsqlBatch(conn)
205+
{
206+
BatchCommands =
207+
{
208+
new("SELECT version()"),
209+
new(GenerateLoadTypesQuery(SupportsRangeTypes, conn.Settings.LoadTableComposites)),
210+
new(GenerateLoadCompositeTypesQuery(conn.Settings.LoadTableComposites)),
211+
}
212+
};
213+
214+
if (SupportsEnumTypes)
215+
batch.BatchCommands.Add(new(GenerateLoadEnumFieldsQuery(HasEnumSortOrder)));
216+
217+
batch.Timeout = commandTimeout;
218+
batch.AllResultTypesAreUnknown = true;
211219

212220
timeout.CheckAndApply(conn);
213-
var reader = async ? await command.ExecuteReaderAsync() : command.ExecuteReader();
221+
var reader = async ? await batch.ExecuteReaderAsync() : batch.ExecuteReader();
214222
try
215223
{
216224
var byOID = new Dictionary<uint, PostgresType>();

‎src/Npgsql/PublicAPI.Unshipped.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ Npgsql.NpgsqlBatch.ExecuteReaderAsync(System.Threading.CancellationToken cancell
1111
Npgsql.NpgsqlBatch.NpgsqlBatch(Npgsql.NpgsqlConnection? connection = null, Npgsql.NpgsqlTransaction? transaction = null) -> void
1212
Npgsql.NpgsqlBatch.Transaction.get -> Npgsql.NpgsqlTransaction?
1313
Npgsql.NpgsqlBatch.Transaction.set -> void
14+
Npgsql.NpgsqlBatch.AllResultTypesAreUnknown.get -> bool
15+
Npgsql.NpgsqlBatch.AllResultTypesAreUnknown.set -> void
1416
Npgsql.NpgsqlBatchCommand
1517
Npgsql.NpgsqlBatchCommand.NpgsqlBatchCommand() -> void
1618
Npgsql.NpgsqlBatchCommand.NpgsqlBatchCommand(string! commandText) -> void

‎test/Npgsql.Tests/CommandTests.cs‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace Npgsql.Tests
1616
{
1717
public class CommandTests : MultiplexingTestBase
1818
{
19-
#region Batching
19+
#region Legacy batching
2020

2121
[Test]
2222
[TestCase(new[] { true }, TestName = "SingleQuery")]
@@ -119,6 +119,17 @@ public async Task MultipleStatementsLargeFirstCommand()
119119
Assert.That(reader.GetString(0), Is.EqualTo(expected2));
120120
}
121121

122+
[Test, NonParallelizable]
123+
public async Task Legacy_batching_is_not_supported_when_EnableSqlParsing_is_disabled()
124+
{
125+
using var _ = DisableSqlRewriting();
126+
127+
using var conn = await OpenConnectionAsync();
128+
using var cmd = new NpgsqlCommand("SELECT 1; SELECT 2", conn);
129+
Assert.That(async () => await cmd.ExecuteReaderAsync(), Throws.Exception.TypeOf<PostgresException>()
130+
.With.Property(nameof(PostgresException.SqlState)).EqualTo("42601"));
131+
}
132+
122133
#endregion
123134

124135
#region Timeout
@@ -549,7 +560,7 @@ public async Task Positional_parameter()
549560
}
550561

551562
[Test]
552-
public async Task Positional_parameters_arent_supported_with_legacy_batching()
563+
public async Task Positional_parameters_are_not_supported_with_legacy_batching()
553564
{
554565
await using var conn = await OpenConnectionAsync();
555566
await using var cmd = new NpgsqlCommand("SELECT $1; SELECT $1", conn);
@@ -558,6 +569,28 @@ public async Task Positional_parameters_arent_supported_with_legacy_batching()
558569
.With.Property(nameof(PostgresException.SqlState)).EqualTo(PostgresErrorCodes.SyntaxError));
559570
}
560571

572+
[Test, NonParallelizable]
573+
public async Task Positional_parameters_are_supported_when_EnableSqlParsing_is_disabled()
574+
{
575+
using var _ = DisableSqlRewriting();
576+
577+
using var conn = await OpenConnectionAsync();
578+
using var cmd = new NpgsqlCommand("SELECT $1", conn);
579+
cmd.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = 8 });
580+
Assert.That(await cmd.ExecuteScalarAsync(), Is.EqualTo(8));
581+
}
582+
583+
[Test, NonParallelizable]
584+
public async Task Named_parameters_are_not_supported_when_EnableSqlParsing_is_disabled()
585+
{
586+
using var _ = DisableSqlRewriting();
587+
588+
using var conn = await OpenConnectionAsync();
589+
using var cmd = new NpgsqlCommand("SELECT @p", conn);
590+
cmd.Parameters.Add(new NpgsqlParameter("p", 8));
591+
Assert.That(async () => await cmd.ExecuteScalarAsync(), Throws.Exception.TypeOf<NotSupportedException>());
592+
}
593+
561594
[Test, Description("Makes sure writing an unset parameter isn't allowed")]
562595
public async Task Parameter_without_Value()
563596
{

‎test/Npgsql.Tests/TestUtil.cs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Text;
66
using System.Threading;
77
using System.Threading.Tasks;
8-
using JetBrains.Annotations;
98
using NUnit.Framework;
109
using NUnit.Framework.Interfaces;
1110

@@ -324,8 +323,14 @@ internal static IDisposable SetEnvironmentVariable(string name, string? value)
324323
return resetter;
325324
}
326325

327-
internal static IDisposable SetCurrentCulture(CultureInfo culture) =>
328-
new CultureSetter(culture);
326+
internal static IDisposable SetCurrentCulture(CultureInfo culture)
327+
=> new CultureSetter(culture);
328+
329+
internal static IDisposable DisableSqlRewriting()
330+
{
331+
NpgsqlCommand.EnableSqlRewriting = false;
332+
return new SqlRewritingEnabler();
333+
}
329334

330335
class EnvironmentVariableResetter : IDisposable
331336
{
@@ -338,8 +343,7 @@ internal EnvironmentVariableResetter(string name, string? value)
338343
_value = value;
339344
}
340345

341-
public void Dispose() =>
342-
Environment.SetEnvironmentVariable(_name, _value);
346+
public void Dispose() => Environment.SetEnvironmentVariable(_name, _value);
343347
}
344348

345349
class CultureSetter : IDisposable
@@ -352,8 +356,12 @@ internal CultureSetter(CultureInfo newCulture)
352356
CultureInfo.CurrentCulture = newCulture;
353357
}
354358

355-
public void Dispose() =>
356-
CultureInfo.CurrentCulture = _oldCulture;
359+
public void Dispose() => CultureInfo.CurrentCulture = _oldCulture;
360+
}
361+
362+
class SqlRewritingEnabler : IDisposable
363+
{
364+
public void Dispose() => NpgsqlCommand.EnableSqlRewriting = true;
357365
}
358366
}
359367

0 commit comments

Comments
 (0)