fix(sql): Oracle bind names must not be reserved words (ORA-01745)
Found by the new OracleReadTests against a real Oracle 23 Free: EVERY Oracle
driver instance failed its Initialize-time catalog load, and every AdminUI browse
of a table's columns threw, with
ORA-01745: invalid host/bind variable name
Oracle rejects a bind variable named after a reserved word, and the catalog
readers bind a parameter whose base name is `table` (SqlCatalogLoader /
SqlBrowseSession bind `schema` + `table`), so `ListColumnsSql` could not execute
at all. The provider was structurally non-functional.
OracleDialect now namespaces every marker/bind name through one `p_` constant
(`:p_k0` / `p_k0`), which a reserved word can never collide with, and its catalog
SQL is respelled to match. Prefixing everything rather than carrying a
reserved-word list is deliberate: the list is long and version-dependent, and a
future base name (`date`, `user`, `comment`, `level`…) would silently re-open the
same hole. Marker and bind name are derived from the same constant, so they
cannot drift apart.
No offline test could have caught this — only a server parses the SQL — so the
regression test asserts the invariant that made it possible (a bind name is never
the bare base name), with `table` called out explicitly.
Live evidence: 6/6 OracleReadTests red with ORA-01745 before, 6/6 green after,
against gvenzl/oracle-free:23-slim.
This commit is contained in:
@@ -9,7 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
|
||||
/// <summary>
|
||||
/// Oracle Database dialect over the <b>fully managed</b> ODP.NET provider
|
||||
/// (<c>Oracle.ManagedDataAccess.Core</c>) — no Instant Client, no native library, no bitness constraint.
|
||||
/// Double-quote identifier quoting, <c>:</c> parameter markers with <b>bare</b> bind names, the
|
||||
/// Double-quote identifier quoting, <c>:</c> parameter markers with <b>colon-less</b> bind names, the
|
||||
/// <c>ALL_*</c> catalog views in place of the <c>INFORMATION_SCHEMA</c> Oracle does not have, and
|
||||
/// <c>FETCH FIRST</c> row limiting.
|
||||
/// <para><b>Supported floor: Oracle 12.2.</b> Two things pin it — <c>FETCH FIRST n ROWS ONLY</c> arrived
|
||||
@@ -34,21 +34,44 @@ public sealed class OracleDialect : ISqlDialect
|
||||
public DbProviderFactory Factory => OracleClientFactory.Instance;
|
||||
|
||||
/// <summary>
|
||||
/// Oracle markers are <c>:</c>-prefixed — <c>ParameterMarker("k0")</c> is <c>":k0"</c>.
|
||||
/// Prepended to every Oracle bind name. <b>It is what keeps a bind name from colliding with an
|
||||
/// Oracle reserved word</b>, which is a hard error rather than a style problem — see
|
||||
/// <see cref="ParameterMarker"/>.
|
||||
/// </summary>
|
||||
/// <param name="baseName">The bare base name.</param>
|
||||
/// <returns>The <c>:</c>-prefixed marker.</returns>
|
||||
public string ParameterMarker(string baseName) => ":" + baseName;
|
||||
private const string BindPrefix = "p_";
|
||||
|
||||
/// <summary>
|
||||
/// The <b>bare</b> name, with no <c>:</c> — ODP.NET's documented convention for
|
||||
/// Oracle markers are <c>:</c>-prefixed <b>and namespaced</b> — <c>ParameterMarker("k0")</c> is
|
||||
/// <c>":p_k0"</c>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para><b>The <c>p_</c> is load-bearing, and was found live.</b> Oracle rejects a bind variable
|
||||
/// whose name is a reserved word with <c>ORA-01745: invalid host/bind variable name</c> — and the
|
||||
/// catalog readers' own base names include <c>table</c> (<c>SqlCatalogLoader</c> /
|
||||
/// <c>SqlBrowseSession</c> bind <c>schema</c> + <c>table</c>). So an un-prefixed
|
||||
/// <see cref="ListColumnsSql"/> could not execute at all: every Oracle driver instance failed its
|
||||
/// Initialize-time catalog load and rejected every authored tag, and every AdminUI browse of a table's
|
||||
/// columns threw. Caught by <c>OracleReadTests</c> against a real database; invisible to any offline
|
||||
/// test, because the SQL is only ever parsed by a server.</para>
|
||||
/// <para>Prefixing <em>everything</em> rather than maintaining a reserved-word list is deliberate:
|
||||
/// the list is long, version-dependent, and a future base name (<c>date</c>, <c>user</c>,
|
||||
/// <c>comment</c>, <c>level</c>…) would re-open the same hole silently. A prefixed name can never be
|
||||
/// a reserved word, and the marker and the bind name are derived from the same constant, so they
|
||||
/// cannot drift apart.</para>
|
||||
/// </remarks>
|
||||
/// <param name="baseName">The bare base name.</param>
|
||||
/// <returns>The <c>:</c>-prefixed, namespaced marker.</returns>
|
||||
public string ParameterMarker(string baseName) => ":" + BindPrefix + baseName;
|
||||
|
||||
/// <summary>
|
||||
/// The same namespaced name <b>without the <c>:</c></b> — ODP.NET's documented convention for
|
||||
/// <see cref="DbParameter.ParameterName"/>, and deliberately <em>not</em>
|
||||
/// <see cref="ParameterMarker"/>. This is the divergence that makes the two members exist at all
|
||||
/// (see <see cref="ISqlDialect.ParameterBindName"/>).
|
||||
/// </summary>
|
||||
/// <param name="baseName">The bare base name.</param>
|
||||
/// <returns>The prefix-less bind name.</returns>
|
||||
public string ParameterBindName(string baseName) => baseName;
|
||||
/// <returns>The colon-less bind name.</returns>
|
||||
public string ParameterBindName(string baseName) => BindPrefix + baseName;
|
||||
|
||||
/// <summary>
|
||||
/// Sets <c>OracleCommand.BindByName = true</c>. <b>This is the sharpest correctness edge in the
|
||||
@@ -56,10 +79,10 @@ public sealed class OracleDialect : ISqlDialect
|
||||
/// <para>ODP.NET binds <em>by position</em> by default: parameter 0 fills the first marker in the
|
||||
/// statement, parameter 1 the second, and so on, <em>regardless of the names</em>. Every statement this
|
||||
/// driver builds with more than one marker would then be a coin flip — the planner's
|
||||
/// <c>IN (:k0, :k1, …)</c> key list, and <see cref="ListColumnsSql"/>'s <c>schema</c> + <c>table</c>
|
||||
/// <c>IN (:p_k0, :p_k1, …)</c> key list, and <see cref="ListColumnsSql"/>'s <c>schema</c> + <c>table</c>
|
||||
/// pair — and the failure mode is not an exception but a <em>successful query over the wrong rows</em>,
|
||||
/// publishing one tag's value onto another tag's node. Worse still,
|
||||
/// <see cref="ListTablesSql"/> names <c>:schema</c> <b>twice</b> (once per branch of its
|
||||
/// <see cref="ListTablesSql"/> names <c>:p_schema</c> <b>twice</b> (once per branch of its
|
||||
/// <c>UNION ALL</c>) and is bound once: under positional binding that statement cannot even execute,
|
||||
/// because the second occurrence has no parameter to take.</para>
|
||||
/// <para>Must be called while the parameter collection is still empty — which is exactly what every
|
||||
@@ -127,13 +150,13 @@ public sealed class OracleDialect : ISqlDialect
|
||||
/// <remarks>
|
||||
/// <c>UNION ALL</c> rather than <c>UNION</c>: the two sources are disjoint by construction (a name
|
||||
/// cannot be both a table and a view in one schema), so de-duplicating would only buy a sort.
|
||||
/// <para>Note this statement names <c>:schema</c> twice and is bound <b>once</b> — see
|
||||
/// <para>Note this statement names <c>:p_schema</c> twice and is bound <b>once</b> — see
|
||||
/// <see cref="ConfigureCommand"/>.</para>
|
||||
/// </remarks>
|
||||
public string ListTablesSql =>
|
||||
"SELECT table_name AS TABLE_NAME, 'BASE TABLE' AS TABLE_TYPE FROM ALL_TABLES WHERE owner = :schema " +
|
||||
"SELECT table_name AS TABLE_NAME, 'BASE TABLE' AS TABLE_TYPE FROM ALL_TABLES WHERE owner = :p_schema " +
|
||||
"UNION ALL " +
|
||||
"SELECT view_name AS TABLE_NAME, 'VIEW' AS TABLE_TYPE FROM ALL_VIEWS WHERE owner = :schema " +
|
||||
"SELECT view_name AS TABLE_NAME, 'VIEW' AS TABLE_TYPE FROM ALL_VIEWS WHERE owner = :p_schema " +
|
||||
"ORDER BY TABLE_NAME";
|
||||
|
||||
/// <summary>
|
||||
@@ -150,7 +173,7 @@ public sealed class OracleDialect : ISqlDialect
|
||||
public string ListColumnsSql =>
|
||||
"SELECT column_name AS COLUMN_NAME, data_type AS DATA_TYPE, " +
|
||||
"CASE nullable WHEN 'Y' THEN 'YES' ELSE 'NO' END AS IS_NULLABLE " +
|
||||
"FROM ALL_TAB_COLUMNS WHERE owner = :schema AND table_name = :table ORDER BY column_id";
|
||||
"FROM ALL_TAB_COLUMNS WHERE owner = :p_schema AND table_name = :p_table ORDER BY column_id";
|
||||
|
||||
/// <summary>
|
||||
/// Double-quotes one identifier part, doubling any embedded <c>"</c> — the Oracle escape rule, and the
|
||||
|
||||
@@ -57,15 +57,45 @@ public class OracleDialectTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ParameterMarker_isColonPrefixed_andTheBindNameIsBare()
|
||||
public void ParameterMarker_isColonPrefixed_andTheBindNameIsTheSameNameWithoutTheColon()
|
||||
{
|
||||
var d = new OracleDialect();
|
||||
|
||||
// The whole reason ISqlDialect models these as two members: on ODP.NET they are different strings.
|
||||
d.ParameterMarker("k0").ShouldBe(":k0");
|
||||
d.ParameterBindName("k0").ShouldBe("k0");
|
||||
d.ParameterMarker("schema").ShouldBe(":schema");
|
||||
d.ParameterBindName("schema").ShouldBe("schema");
|
||||
d.ParameterMarker("k0").ShouldBe(":p_k0");
|
||||
d.ParameterBindName("k0").ShouldBe("p_k0");
|
||||
d.ParameterMarker("schema").ShouldBe(":p_schema");
|
||||
d.ParameterBindName("schema").ShouldBe("p_schema");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// No bind name may be spelled as its bare base name — the <c>p_</c> namespace is what stops one
|
||||
/// colliding with an Oracle reserved word.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <b>This is a regression test for a live-found, provider-fatal defect.</b> The catalog readers bind
|
||||
/// a parameter whose base name is <c>table</c>, and Oracle rejects a bind variable named after a
|
||||
/// reserved word outright: <c>ORA-01745: invalid host/bind variable name</c>. With the un-prefixed
|
||||
/// names this dialect first shipped with, <c>ListColumnsSql</c> could not execute at all — so every
|
||||
/// Oracle driver instance failed its Initialize-time catalog load and rejected every authored tag, and
|
||||
/// every AdminUI browse of a table's columns threw. It was invisible to every offline test (only a
|
||||
/// server parses the SQL) and was caught by <c>OracleReadTests</c> against a real database.
|
||||
/// <para><c>table</c> is asserted explicitly because it is the one base name in use today that is
|
||||
/// actually reserved; the others guard the general rule.</para>
|
||||
/// </remarks>
|
||||
[Theory]
|
||||
[InlineData("table")]
|
||||
[InlineData("schema")]
|
||||
[InlineData("k0")]
|
||||
public void ParameterBindName_neverSurfacesTheBareBaseName_soItCannotBeAReservedWord(string baseName)
|
||||
{
|
||||
var d = new OracleDialect();
|
||||
|
||||
d.ParameterBindName(baseName).ShouldNotBe(baseName);
|
||||
d.ParameterMarker(baseName).ShouldNotBe(":" + baseName);
|
||||
// The two must stay derived from one another, or a statement's marker and its bind drift apart and
|
||||
// ODP.NET silently leaves the parameter unbound.
|
||||
d.ParameterMarker(baseName).ShouldBe(":" + d.ParameterBindName(baseName));
|
||||
}
|
||||
|
||||
// ---- identity ----
|
||||
|
||||
Reference in New Issue
Block a user