diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs index d9ac6b3f..a6fd198e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs @@ -9,7 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// /// Oracle Database dialect over the fully managed ODP.NET provider /// (Oracle.ManagedDataAccess.Core) — no Instant Client, no native library, no bitness constraint. -/// Double-quote identifier quoting, : parameter markers with bare bind names, the +/// Double-quote identifier quoting, : parameter markers with colon-less bind names, the /// ALL_* catalog views in place of the INFORMATION_SCHEMA Oracle does not have, and /// FETCH FIRST row limiting. /// Supported floor: Oracle 12.2. Two things pin it — FETCH FIRST n ROWS ONLY arrived @@ -34,21 +34,44 @@ public sealed class OracleDialect : ISqlDialect public DbProviderFactory Factory => OracleClientFactory.Instance; /// - /// Oracle markers are :-prefixed — ParameterMarker("k0") is ":k0". + /// Prepended to every Oracle bind name. It is what keeps a bind name from colliding with an + /// Oracle reserved word, which is a hard error rather than a style problem — see + /// . /// - /// The bare base name. - /// The :-prefixed marker. - public string ParameterMarker(string baseName) => ":" + baseName; + private const string BindPrefix = "p_"; /// - /// The bare name, with no : — ODP.NET's documented convention for + /// Oracle markers are :-prefixed and namespaced — ParameterMarker("k0") is + /// ":p_k0". + /// + /// + /// The p_ is load-bearing, and was found live. Oracle rejects a bind variable + /// whose name is a reserved word with ORA-01745: invalid host/bind variable name — and the + /// catalog readers' own base names include table (SqlCatalogLoader / + /// SqlBrowseSession bind schema + table). So an un-prefixed + /// 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 OracleReadTests against a real database; invisible to any offline + /// test, because the SQL is only ever parsed by a server. + /// Prefixing everything rather than maintaining a reserved-word list is deliberate: + /// the list is long, version-dependent, and a future base name (date, user, + /// comment, level…) 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. + /// + /// The bare base name. + /// The :-prefixed, namespaced marker. + public string ParameterMarker(string baseName) => ":" + BindPrefix + baseName; + + /// + /// The same namespaced name without the : — ODP.NET's documented convention for /// , and deliberately not /// . This is the divergence that makes the two members exist at all /// (see ). /// /// The bare base name. - /// The prefix-less bind name. - public string ParameterBindName(string baseName) => baseName; + /// The colon-less bind name. + public string ParameterBindName(string baseName) => BindPrefix + baseName; /// /// Sets OracleCommand.BindByName = true. This is the sharpest correctness edge in the @@ -56,10 +79,10 @@ public sealed class OracleDialect : ISqlDialect /// ODP.NET binds by position by default: parameter 0 fills the first marker in the /// statement, parameter 1 the second, and so on, regardless of the names. Every statement this /// driver builds with more than one marker would then be a coin flip — the planner's - /// IN (:k0, :k1, …) key list, and 's schema + table + /// IN (:p_k0, :p_k1, …) key list, and 's schema + table /// pair — and the failure mode is not an exception but a successful query over the wrong rows, /// publishing one tag's value onto another tag's node. Worse still, - /// names :schema twice (once per branch of its + /// names :p_schema twice (once per branch of its /// UNION ALL) and is bound once: under positional binding that statement cannot even execute, /// because the second occurrence has no parameter to take. /// Must be called while the parameter collection is still empty — which is exactly what every @@ -127,13 +150,13 @@ public sealed class OracleDialect : ISqlDialect /// /// UNION ALL rather than UNION: 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. - /// Note this statement names :schema twice and is bound once — see + /// Note this statement names :p_schema twice and is bound once — see /// . /// 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"; /// @@ -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"; /// /// Double-quotes one identifier part, doubling any embedded " — the Oracle escape rule, and the diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs index fd3f3f30..e8b0c375 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs @@ -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"); + } + + /// + /// No bind name may be spelled as its bare base name — the p_ namespace is what stops one + /// colliding with an Oracle reserved word. + /// + /// + /// This is a regression test for a live-found, provider-fatal defect. The catalog readers bind + /// a parameter whose base name is table, and Oracle rejects a bind variable named after a + /// reserved word outright: ORA-01745: invalid host/bind variable name. With the un-prefixed + /// names this dialect first shipped with, ListColumnsSql 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 OracleReadTests against a real database. + /// table is asserted explicitly because it is the one base name in use today that is + /// actually reserved; the others guard the general rule. + /// + [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 ----