diff --git a/CHANGELOG.md b/CHANGELOG.md index da8bb10e..fb2f99e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,70 @@ # Changelog +## 14.0.1 + +**Two defects a client found by being used, both in the planner, neither reachable from the SQL +the suite happens to write.** No file format change; no API change beyond one internal signature. + +### A join condition means the same thing whichever way round it is written + +`ON c.Id = o.CustomerId` returned rows. `ON o.CustomerId = c.Id` - the same condition, the same +two tables - failed at execution with `Column 'CustomerId' not found`. It affected qualified +names with or without aliases, and the shape most likely to be met by accident is a chain, where +"the left input" is everything joined so far: + +```sql +SELECT c.Country FROM Customers c JOIN Orders o ON c.Id = o.CustomerId + JOIN Items i ON i.OrderId = o.Id -- refused before 14.0.1 +``` + +**The cause was that the equi-join key pair was built from the order the condition was WRITTEN +in**, and never from where the columns come from: `LeftKey = binary.Left, RightKey = binary.Right`. +The extractor checked that the two column references named different tables and stopped there, so +the hash join looked for the right table’s column in rows of the left one. It now reads the +left input’s schema - which is a required parameter of `OptimizerJoinCondition.Analyze`, so the +question is asked at every call site - and orients each pair by which side its columns are on. + +Two conditions that used to become hash keys no longer do, and both are corrections: a pair whose +columns are BOTH from the left input is a filter on that input rather than a join key, and a pair +with an unqualified column cannot be attributed at all. Both now go to the residual condition, +which is evaluated over the joined row and cannot be wrong. + +**Where the engine does not know which side a column is on, the written order still decides.** +An `INFORMATION_SCHEMA` source reports no table name, so `tc.X = kcu.X` over two of them cannot be +attributed to either input; those keep the old behaviour deliberately. Reading it as «neither is +from the left» and sending it to the residual turned Studio's primary-key query into a cross +product - measured, five of its cases went red - because a qualified name over such a join +resolves by column name, and the same name appears once per side. + +**Why the suite said nothing:** every join case in it writes the equality left-hand-side first. +The new fixture asserts the two orders agree ROW FOR ROW across inner, left, chained, aliased, +unaliased, composite-key and residual-beside-key shapes - and carries a control that the hash join +is the path being taken, because a nested loop has never cared. + +### `EXPLAIN` names the line each line is really under + +The lines of each child’s subtree were re-based by a constant rather than by where that subtree +starts, so everything below the SECOND child of any node was attributed into the first child’s +subtree. A join has two children, and so does every set operation: + +``` +id parent detail +2 1 HASH INNER JOIN +3 2 ALIAS c +4 3 SCAN TABLE Customers +5 2 ALIAS o +6 3 SCAN TABLE Orders <- said 3, which is ALIAS c +``` + +Anything that draws the plan as a tree drew the wrong tree faithfully. The rows themselves were +all present and every parent was a real earlier line, which is why a structural check passes on +both versions; the new fixture asserts instead that each `ALIAS` stands over the scan of the table +it aliases, and that the two arms of a `UNION` keep their own children. + +### Known issues + +Issues 25 and 26 of [Docs/KnownIssues.md](Docs/KnownIssues.md) are these two, and are now marked +fixed there. ## 14.0.0 **An encrypted database written before 13.1.0 is no longer opened without being asked.** That is the diff --git a/Docs/KnownIssues.md b/Docs/KnownIssues.md index 34c10c2a..e9eb6d57 100644 --- a/Docs/KnownIssues.md +++ b/Docs/KnownIssues.md @@ -1530,7 +1530,19 @@ because a person meeting it will look for it here. ## 25. A join condition written the other way round is refused -> Measured 2026-08-19 on engine 14.0.0. **Root cause identified**, fix not written. +> **FIXED in 14.0.1.** The key pair is now oriented by the left input’s schema, which is a +> required parameter of `OptimizerJoinCondition.Analyze` so that the question is asked at every +> call site. `JoinKeysBelongToTheirOwnSideTests` asserts the two orders agree ROW FOR ROW over +> inner, left, chained, aliased, unaliased, composite-key and residual-beside-key shapes, and +> carries a control that the hash join is the path being taken. Seven of its ten cases fail if the +> orientation is removed. +> +> **One limit, deliberate:** where a source reports no table name - `INFORMATION_SCHEMA` does - +> neither column can be attributed and the written order still decides. Such a join resolves a +> qualified name by column name anyway, so nothing is gained by refusing it, and treating it as +> «neither from the left» turned Studio's primary-key query into a cross product. +> +> Measured 2026-08-19 on engine 14.0.0. The account below is what was found. In `A JOIN B ON x = y`, the column of the LEFT input has to be written FIRST. Written the other way, the query fails at execution with a `KeyNotFoundException`: @@ -1580,7 +1592,12 @@ order. ## 26. `EXPLAIN` gives the right input’s child the wrong parent -> Measured 2026-08-19 on engine 14.0.0. +> **FIXED in 14.0.1.** Each child’s subtree is re-based by where it actually starts rather than +> by a constant. `APlanNodeNamesItsRealParentTests` asserts that each `ALIAS` stands over the scan +> of the table it aliases and that the two arms of a `UNION` keep their own children - a purely +> structural check passes on both versions, which is why that is not the assertion. +> +> Measured 2026-08-19 on engine 14.0.0. The account below is what was found. For `SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId LIMIT 3`, `EXPLAIN` answers: diff --git a/Sources/Core/OutWit.Database.Core.BouncyCastle/OutWit.Database.Core.BouncyCastle.csproj b/Sources/Core/OutWit.Database.Core.BouncyCastle/OutWit.Database.Core.BouncyCastle.csproj index 680eadea..f9153b4a 100644 --- a/Sources/Core/OutWit.Database.Core.BouncyCastle/OutWit.Database.Core.BouncyCastle.csproj +++ b/Sources/Core/OutWit.Database.Core.BouncyCastle/OutWit.Database.Core.BouncyCastle.csproj @@ -5,7 +5,7 @@ enable enable - 14.0.0 + 14.0.1 ChaCha20-Poly1305 encryption provider for WitDatabase using BouncyCastle. Ideal for Blazor WebAssembly where hardware AES acceleration is unavailable. OutWit;database;encryption;chacha20;poly1305;bouncycastle;blazor;wasm;security diff --git a/Sources/Core/OutWit.Database.Core.BouncyCastle/README.md b/Sources/Core/OutWit.Database.Core.BouncyCastle/README.md index 4c312a1b..20811a0d 100644 --- a/Sources/Core/OutWit.Database.Core.BouncyCastle/README.md +++ b/Sources/Core/OutWit.Database.Core.BouncyCastle/README.md @@ -9,7 +9,7 @@ This package provides an alternative encryption algorithm when AES-NI hardware a ## Installation ```xml - + ``` --- diff --git a/Sources/Core/OutWit.Database.Core.IndexedDb/OutWit.Database.Core.IndexedDb.csproj b/Sources/Core/OutWit.Database.Core.IndexedDb/OutWit.Database.Core.IndexedDb.csproj index 80ed260c..d3319bd8 100644 --- a/Sources/Core/OutWit.Database.Core.IndexedDb/OutWit.Database.Core.IndexedDb.csproj +++ b/Sources/Core/OutWit.Database.Core.IndexedDb/OutWit.Database.Core.IndexedDb.csproj @@ -5,7 +5,7 @@ enable enable - 14.0.0 + 14.0.1 IndexedDB storage provider for WitDatabase. Enables running WitDatabase entirely in the browser with Blazor WebAssembly applications. OutWit;database;indexeddb;blazor;wasm;webassembly;storage;browser;client-side diff --git a/Sources/Core/OutWit.Database.Core.IndexedDb/README.md b/Sources/Core/OutWit.Database.Core.IndexedDb/README.md index 6d15cdf8..d829aa04 100644 --- a/Sources/Core/OutWit.Database.Core.IndexedDb/README.md +++ b/Sources/Core/OutWit.Database.Core.IndexedDb/README.md @@ -15,7 +15,7 @@ This package allows WitDatabase to run entirely in the browser with data persist ## Installation ```xml - + ``` Add the JavaScript files to your `index.html`: diff --git a/Sources/Core/OutWit.Database.Core/OutWit.Database.Core.csproj b/Sources/Core/OutWit.Database.Core/OutWit.Database.Core.csproj index 4dcf464f..ab6ae353 100644 --- a/Sources/Core/OutWit.Database.Core/OutWit.Database.Core.csproj +++ b/Sources/Core/OutWit.Database.Core/OutWit.Database.Core.csproj @@ -5,7 +5,7 @@ enable enable - 14.0.0 + 14.0.1 High-performance embedded key-value database engine for .NET. Features B+Tree and LSM-Tree storage engines, MVCC transactions, AES-256-GCM encryption, and full ACID compliance. OutWit;database;embedded;key-value;btree;lsm-tree;mvcc;transactions;encryption;acid;storage diff --git a/Sources/Core/OutWit.Database.Core/README.md b/Sources/Core/OutWit.Database.Core/README.md index 9e6abded..61f24f73 100644 --- a/Sources/Core/OutWit.Database.Core/README.md +++ b/Sources/Core/OutWit.Database.Core/README.md @@ -36,17 +36,17 @@ OutWit.Database.Core is a production-ready embedded database engine designed for ## Installation ```xml - + ``` For ChaCha20-Poly1305 encryption: ```xml - + ``` For Blazor WebAssembly (IndexedDB storage): ```xml - + ``` --- @@ -498,7 +498,7 @@ WitDatabase can run entirely in the browser using IndexedDB as the storage backe ### Installation ```xml - + ``` Add JavaScript files to `index.html`: diff --git a/Sources/Engine/OutWit.Database.Parser/OutWit.Database.Parser.csproj b/Sources/Engine/OutWit.Database.Parser/OutWit.Database.Parser.csproj index 9915090c..e8dc411a 100644 --- a/Sources/Engine/OutWit.Database.Parser/OutWit.Database.Parser.csproj +++ b/Sources/Engine/OutWit.Database.Parser/OutWit.Database.Parser.csproj @@ -7,7 +7,7 @@ $(MSBuildProjectDirectory)\MakeInternal.ps1 <_AntlrVendoredJar>$([System.IO.Path]::GetFullPath('$(MSBuildProjectDirectory)/../../../build/antlr/antlr4-4.13.1-complete.jar')) - 14.0.0 + 14.0.1 SQL parser for WitDatabase. ANTLR4-based parser for WitSQL dialect with full SQL-92 compatibility and .NET type extensions. OutWit;database;sql;parser;antlr;witsql;query;syntax diff --git a/Sources/Engine/OutWit.Database.Parser/README.md b/Sources/Engine/OutWit.Database.Parser/README.md index 3841d2e0..b396604e 100644 --- a/Sources/Engine/OutWit.Database.Parser/README.md +++ b/Sources/Engine/OutWit.Database.Parser/README.md @@ -26,7 +26,7 @@ OutWit.Database.Parser is a high-performance SQL parser built on [ANTLR4](https: ## Installation ```xml - + ``` --- diff --git a/Sources/Engine/OutWit.Database.Tests/Engine/APlanNodeNamesItsRealParentTests.cs b/Sources/Engine/OutWit.Database.Tests/Engine/APlanNodeNamesItsRealParentTests.cs new file mode 100644 index 00000000..3404eba7 --- /dev/null +++ b/Sources/Engine/OutWit.Database.Tests/Engine/APlanNodeNamesItsRealParentTests.cs @@ -0,0 +1,218 @@ +using OutWit.Database.Core.Builder; +using OutWit.Database.Engine; + +namespace OutWit.Database.Tests; + +/// +/// Every line of an EXPLAIN names the line it is really under. +/// +/// +/// +/// Found by looking at Studio's plan panel on 2026-08-19. The panel draws the plan as a tree +/// from the id and parent columns, and it drew SCAN TABLE Orders under +/// ALIAS c - the alias of the OTHER table. The panel was faithful; the plan was wrong. +/// +/// +/// The cause: the lines of each child's subtree were re-based by a constant instead of by where +/// that subtree actually starts, so everything below the SECOND child of any node was attributed to +/// the first child's subtree. A join has two children, and so does every set operation. +/// +/// +/// This fixture asserts the SHAPE rather than one row: one root, every parent a real earlier line, +/// and - the part that catches an off-by-anything - each ALIAS standing directly over the +/// scan of the table it aliases. +/// +/// +[TestFixture] +public class APlanNodeNamesItsRealParentTests +{ + #region Fields + + private WitSqlEngine m_engine = null!; + + #endregion + + #region Setup + + [SetUp] + public void SetUp() + { + var database = new WitDatabaseBuilder() + .WithMemoryStorage() + .WithBTree() + .WithTransactions() + .Build(); + + m_engine = new WitSqlEngine(database, ownsStore: true); + + m_engine.Execute("CREATE TABLE Customers (Id INT PRIMARY KEY, Country VARCHAR(60) NOT NULL)"); + m_engine.Execute("CREATE TABLE Orders (Id INT PRIMARY KEY, CustomerId INT NOT NULL, Total INT NOT NULL)"); + m_engine.Execute("CREATE TABLE Items (Id INT PRIMARY KEY, OrderId INT NOT NULL)"); + + for (var i = 1; i <= 60; i++) + m_engine.Execute($"INSERT INTO Customers (Id, Country) VALUES ({i}, 'C{i % 7}')"); + + for (var i = 1; i <= 200; i++) + m_engine.Execute($"INSERT INTO Orders (Id, CustomerId, Total) VALUES ({i}, {(i % 60) + 1}, {i})"); + + for (var i = 1; i <= 200; i++) + m_engine.Execute($"INSERT INTO Items (Id, OrderId) VALUES ({i}, {(i % 200) + 1})"); + } + + [TearDown] + public void TearDown() + { + m_engine?.Dispose(); + } + + #endregion + + #region The rule + + [Test] + public void EachAliasStandsOverTheTableItAliasesTest() + { + var plan = Plan("EXPLAIN SELECT c.Country, o.Total FROM Customers c JOIN Orders o " + + "ON c.Id = o.CustomerId LIMIT 3"); + + Assert.Multiple(() => + { + Assert.That(ChildrenOf(plan, "ALIAS c"), Is.EqualTo(new[] { "SCAN TABLE Customers" })); + Assert.That(ChildrenOf(plan, "ALIAS o"), Is.EqualTo(new[] { "SCAN TABLE Orders" })); + + // CONTROL: a plan that was not read at all has no aliases in it either. + Assert.That(plan.Select(line => line.Detail), Has.Some.Contains("ALIAS c"), + "CONTROL: the plan was not read - " + Written(plan)); + }); + } + + /// + /// Three tables, which is two joins - so the second join's right input sits under a node that + /// already has a subtree of its own. + /// + [Test] + public void AChainOfJoinsNamesEveryParentTest() + { + var plan = Plan("EXPLAIN SELECT c.Country FROM Customers c JOIN Orders o ON c.Id = o.CustomerId " + + "JOIN Items i ON o.Id = i.OrderId"); + + Assert.Multiple(() => + { + Assert.That(ChildrenOf(plan, "ALIAS c"), Is.EqualTo(new[] { "SCAN TABLE Customers" })); + Assert.That(ChildrenOf(plan, "ALIAS o"), Is.EqualTo(new[] { "SCAN TABLE Orders" })); + Assert.That(ChildrenOf(plan, "ALIAS i"), Is.EqualTo(new[] { "SCAN TABLE Items" })); + }); + } + + /// + /// The other two-child shape in the engine, and it was wrong for the same reason. + /// + [Test] + public void BothArmsOfAUnionKeepTheirOwnChildrenTest() + { + var plan = Plan("EXPLAIN SELECT Id FROM Customers UNION SELECT Id FROM Orders"); + + var scans = plan.Where(line => line.Detail.Contains("SCAN TABLE")).ToList(); + + Assert.Multiple(() => + { + Assert.That(scans, Has.Count.EqualTo(2), "CONTROL: two arms, two scans - " + Written(plan)); + + foreach (var scan in scans) + { + Assert.That(ParentOf(plan, scan), Is.Not.Null, + $"«{scan.Detail}» is under a line that is not in the plan"); + } + + Assert.That(scans.Select(scan => ParentOf(plan, scan)!.Id).Distinct().Count(), Is.EqualTo(2), + "the two arms are two branches, not one - " + Written(plan)); + }); + } + + [Test] + public void EveryLineIsUnderALineThatComesBeforeItTest() + { + var offenders = new List(); + var examined = 0; + + foreach (var sql in new[] + { + "EXPLAIN SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId LIMIT 3", + "EXPLAIN SELECT c.Country FROM Customers c JOIN Orders o ON c.Id = o.CustomerId " + + "JOIN Items i ON o.Id = i.OrderId", + "EXPLAIN SELECT Id FROM Customers UNION SELECT Id FROM Orders", + "EXPLAIN SELECT Country, COUNT(*) FROM Customers GROUP BY Country ORDER BY Country" + }) + { + var plan = Plan(sql); + + examined += plan.Count; + + var roots = plan.Count(line => line.Parent < 0); + + if (roots != 1) + offenders.Add($"{roots} roots in: {Written(plan)}"); + + foreach (var line in plan.Where(line => line.Parent >= 0)) + { + if (line.Parent >= line.Id) + offenders.Add($"line {line.Id} is under {line.Parent}, which comes after it: {Written(plan)}"); + + else if (plan.All(other => other.Id != line.Parent)) + offenders.Add($"line {line.Id} is under {line.Parent}, which is not in the plan: {Written(plan)}"); + } + } + + Assert.Multiple(() => + { + // CONTROL: four plans that were never read would report nothing wrong. + Assert.That(examined, Is.GreaterThan(12), "CONTROL: too few plan lines were read"); + + Assert.That(offenders, Is.Empty, string.Join(Environment.NewLine, offenders)); + }); + } + + #endregion + + #region Tools + + private sealed record PlanLine(int Id, int Parent, string Detail); + + private List Plan(string sql) + { + using var result = m_engine.Execute(sql); + + return result.ReadAll() + .Select(row => new PlanLine( + (int)row["id"].AsInt64(), + (int)row["parent"].AsInt64(), + row["detail"].AsString().Trim())) + .ToList(); + } + + private static IReadOnlyList ChildrenOf(List plan, string detail) + { + var parent = plan.FirstOrDefault(line => line.Detail.StartsWith(detail, StringComparison.Ordinal)); + + Assert.That(parent, Is.Not.Null, $"«{detail}» is not in the plan - {Written(plan)}"); + + // The head of the line only: a plain EXPLAIN also writes the schema after an arrow, and + // this fixture is about who is under whom. + return plan.Where(line => line.Parent == parent!.Id) + .Select(line => line.Detail.Split(" -> ")[0].Trim()) + .ToList(); + } + + private static PlanLine? ParentOf(List plan, PlanLine line) + { + return plan.FirstOrDefault(candidate => candidate.Id == line.Parent); + } + + private static string Written(List plan) + { + return Environment.NewLine + + string.Join(Environment.NewLine, plan.Select(line => $"{line.Id} <- {line.Parent} | {line.Detail}")); + } + + #endregion +} diff --git a/Sources/Engine/OutWit.Database.Tests/Engine/JoinKeysBelongToTheirOwnSideTests.cs b/Sources/Engine/OutWit.Database.Tests/Engine/JoinKeysBelongToTheirOwnSideTests.cs new file mode 100644 index 00000000..3611002c --- /dev/null +++ b/Sources/Engine/OutWit.Database.Tests/Engine/JoinKeysBelongToTheirOwnSideTests.cs @@ -0,0 +1,256 @@ +using OutWit.Database.Core.Builder; +using OutWit.Database.Engine; + +namespace OutWit.Database.Tests; + +/// +/// A join condition means the same thing whichever way round it is written. +/// +/// +/// +/// Found by using Studio on 2026-08-19, and it had shipped in 14.0.0. +/// ON c.Id = o.CustomerId answered three rows; ON o.CustomerId = c.Id - the same +/// condition, the same two tables - failed with Column 'CustomerId' not found. In a chain of +/// joins the trap is easier to fall into, because "the left input" is everything joined so far, so +/// JOIN Items i ON i.OrderId = o.Id is the WRONG way round however natural it reads. +/// +/// +/// The cause was in the planner rather than in the evaluator: the equi-join key pair was built as +/// LeftKey = binary.Left, RightKey = binary.Right, taking the written order of the equality +/// for the order of the join's inputs. It checked that the two column references named DIFFERENT +/// tables and never asked WHICH input each belonged to, so the hash join looked for the right +/// table's column in rows of the left one. +/// +/// +/// Why 106 join cases said nothing: every one of them writes the equality left-hand-side +/// first. That is what this fixture exists to stop - it asserts the two orders agree ROW FOR ROW, +/// over every shape the engine offers, rather than asserting that one case no longer throws. +/// +/// +[TestFixture] +public class JoinKeysBelongToTheirOwnSideTests +{ + #region Fields + + private WitSqlEngine m_engine = null!; + + #endregion + + #region Setup + + /// + /// Enough rows that the planner reaches for a hash join - which is the path that was broken. + /// is the control on that. + /// + [SetUp] + public void SetUp() + { + var database = new WitDatabaseBuilder() + .WithMemoryStorage() + .WithBTree() + .WithTransactions() + .Build(); + + m_engine = new WitSqlEngine(database, ownsStore: true); + + m_engine.Execute("CREATE TABLE Customers (Id INT PRIMARY KEY, Country VARCHAR(60) NOT NULL)"); + m_engine.Execute("CREATE TABLE Orders (Id INT PRIMARY KEY, CustomerId INT NOT NULL, Total INT NOT NULL)"); + m_engine.Execute("CREATE TABLE Items (Id INT PRIMARY KEY, OrderId INT NOT NULL, Quantity INT NOT NULL)"); + + // A composite key needs a pair of columns on each side. + m_engine.Execute("CREATE TABLE Legs (Region INT NOT NULL, Slot INT NOT NULL, Label VARCHAR(20) NOT NULL)"); + m_engine.Execute("CREATE TABLE Cargo (LegRegion INT NOT NULL, LegSlot INT NOT NULL, Weight INT NOT NULL)"); + + for (var i = 1; i <= 60; i++) + m_engine.Execute($"INSERT INTO Customers (Id, Country) VALUES ({i}, 'C{i % 7}')"); + + for (var i = 1; i <= 200; i++) + m_engine.Execute($"INSERT INTO Orders (Id, CustomerId, Total) VALUES ({i}, {(i % 60) + 1}, {i * 3})"); + + for (var i = 1; i <= 200; i++) + m_engine.Execute($"INSERT INTO Items (Id, OrderId, Quantity) VALUES ({i}, {(i % 200) + 1}, {i % 9})"); + + // Big enough for a hash join here too - at forty rows each the planner chose a nested + // loop, and the two composite cases passed without touching the path they are about. + for (var i = 1; i <= 120; i++) + m_engine.Execute($"INSERT INTO Legs (Region, Slot, Label) VALUES ({i % 5}, {i % 8}, 'L{i}')"); + + for (var i = 1; i <= 200; i++) + m_engine.Execute($"INSERT INTO Cargo (LegRegion, LegSlot, Weight) VALUES ({i % 5}, {i % 8}, {i})"); + } + + [TearDown] + public void TearDown() + { + m_engine?.Dispose(); + } + + #endregion + + #region The rule, over every shape + + [Test] + public void AnInnerJoinReadsTheSameBothWaysTest() + { + SameRows( + "SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId ORDER BY o.Total", + "SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON o.CustomerId = c.Id ORDER BY o.Total"); + } + + [Test] + public void ALeftJoinReadsTheSameBothWaysTest() + { + SameRows( + "SELECT c.Country, o.Total FROM Customers c LEFT JOIN Orders o ON c.Id = o.CustomerId ORDER BY c.Id, o.Total", + "SELECT c.Country, o.Total FROM Customers c LEFT JOIN Orders o ON o.CustomerId = c.Id ORDER BY c.Id, o.Total"); + } + + /// + /// The shape that is hardest to write correctly by accident: the left input of the second join + /// is not a table at all, it is everything joined before it. + /// + [Test] + public void AChainOfJoinsReadsTheSameBothWaysTest() + { + SameRows( + "SELECT c.Country, i.Quantity FROM Customers c JOIN Orders o ON c.Id = o.CustomerId " + + "JOIN Items i ON o.Id = i.OrderId ORDER BY i.Id", + "SELECT c.Country, i.Quantity FROM Customers c JOIN Orders o ON o.CustomerId = c.Id " + + "JOIN Items i ON i.OrderId = o.Id ORDER BY i.Id"); + } + + [Test] + public void AJoinWithoutAliasesReadsTheSameBothWaysTest() + { + SameRows( + "SELECT Customers.Country, Orders.Total FROM Customers JOIN Orders " + + "ON Customers.Id = Orders.CustomerId ORDER BY Orders.Total", + "SELECT Customers.Country, Orders.Total FROM Customers JOIN Orders " + + "ON Orders.CustomerId = Customers.Id ORDER BY Orders.Total"); + } + + /// + /// Two keys, and the second one written the other way round - so a fix that swaps the whole + /// condition rather than each pair is caught here. + /// + [Test] + public void ACompositeKeyReadsTheSameWithOnePartTurnedRoundTest() + { + SameRows( + "SELECT l.Label, g.Weight FROM Legs l JOIN Cargo g ON l.Region = g.LegRegion AND l.Slot = g.LegSlot " + + "ORDER BY l.Label, g.Weight", + "SELECT l.Label, g.Weight FROM Legs l JOIN Cargo g ON l.Region = g.LegRegion AND g.LegSlot = l.Slot " + + "ORDER BY l.Label, g.Weight"); + } + + [Test] + public void ACompositeKeyReadsTheSameWithBothPartsTurnedRoundTest() + { + SameRows( + "SELECT l.Label, g.Weight FROM Legs l JOIN Cargo g ON l.Region = g.LegRegion AND l.Slot = g.LegSlot " + + "ORDER BY l.Label, g.Weight", + "SELECT l.Label, g.Weight FROM Legs l JOIN Cargo g ON g.LegRegion = l.Region AND g.LegSlot = l.Slot " + + "ORDER BY l.Label, g.Weight"); + } + + /// + /// A residual condition beside the key, because the two are separated by the same walk. + /// + [Test] + public void AKeyBesideAResidualConditionReadsTheSameBothWaysTest() + { + SameRows( + "SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId AND o.Total > 100 " + + "ORDER BY o.Total", + "SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON o.CustomerId = c.Id AND o.Total > 100 " + + "ORDER BY o.Total"); + } + + /// + /// The same condition in WHERE, which never went through the key extraction and was the + /// workaround while this was open. + /// + [Test] + public void TheConditionInWhereReadsTheSameAsTheJoinTest() + { + SameRows( + "SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId ORDER BY o.Total", + "SELECT c.Country, o.Total FROM Customers c, Orders o WHERE o.CustomerId = c.Id ORDER BY o.Total"); + } + + #endregion + + #region The controls + + /// + /// CONTROL: the path this fixture is about is the one being taken. A nested loop join evaluates + /// the ON condition whole and has never cared which way round it is written - so if the planner + /// stopped choosing a hash join here, every case above would pass for a reason that has nothing + /// to do with the defect. + /// + [Test] + public void TheHashJoinIsTheOneBeingExercisedTest() + { + var plan = Details("EXPLAIN SELECT c.Country, o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId"); + + var composite = Details("EXPLAIN SELECT l.Label FROM Legs l JOIN Cargo g " + + "ON l.Region = g.LegRegion AND l.Slot = g.LegSlot"); + + Assert.Multiple(() => + { + Assert.That(plan.Any(line => line.Contains("HASH", StringComparison.OrdinalIgnoreCase)), Is.True, + "CONTROL: these tables are meant to be big enough for a hash join - " + string.Join(" | ", plan)); + + Assert.That(composite.Any(line => line.Contains("HASH", StringComparison.OrdinalIgnoreCase)), Is.True, + "CONTROL: the composite key must reach the hash join too - " + string.Join(" | ", composite)); + }); + } + + /// + /// CONTROL: the queries return something. Two empty results agree with each other. + /// + [Test] + public void TheJoinsUsedHereActuallyMatchRowsTest() + { + Assert.Multiple(() => + { + Assert.That(Rows("SELECT o.Total FROM Customers c JOIN Orders o ON c.Id = o.CustomerId"), + Has.Count.EqualTo(200), "every order has a customer"); + + Assert.That(Rows("SELECT l.Label FROM Legs l JOIN Cargo g ON l.Region = g.LegRegion AND l.Slot = g.LegSlot"), + Is.Not.Empty, "the composite key matches something"); + }); + } + + #endregion + + #region Tools + + private void SameRows(string oneWay, string theOther) + { + var expected = Rows(oneWay); + var actual = Rows(theOther); + + Assert.That(actual, Is.EqualTo(expected), + "the same condition, written the other way round:" + Environment.NewLine + + oneWay + Environment.NewLine + theOther); + } + + private List Rows(string sql) + { + using var result = m_engine.Execute(sql); + + return result.ReadAll() + .Select(row => string.Join("|", row.Values.Select(value => value.ToString()))) + .ToList(); + } + + private List Details(string sql) + { + using var result = m_engine.Execute(sql); + + return result.ReadAll().Select(row => row["detail"].AsString()).ToList(); + } + + #endregion +} diff --git a/Sources/Engine/OutWit.Database/Optimizers/OptimizerJoinCondition.cs b/Sources/Engine/OutWit.Database/Optimizers/OptimizerJoinCondition.cs index cdcdd687..517e3544 100644 --- a/Sources/Engine/OutWit.Database/Optimizers/OptimizerJoinCondition.cs +++ b/Sources/Engine/OutWit.Database/Optimizers/OptimizerJoinCondition.cs @@ -1,6 +1,7 @@ using OutWit.Database.Iterators; using OutWit.Database.Parser.Expressions; using OutWit.Database.Parser.Schema.Types; +using OutWit.Database.Sql; namespace OutWit.Database.Optimizers; @@ -16,8 +17,28 @@ public static class OptimizerJoinCondition /// Analyzes a join ON condition and extracts equi-join keys. /// /// The ON condition expression. + /// + /// The schema of the join's LEFT input, which is what says where each column of a key comes + /// from. + /// /// Result containing equi-join keys and any residual conditions. - public static JoinConditionAnalysis Analyze(WitSqlExpression? onCondition) + /// + /// + /// is required, and that is the whole of a defect shipped in + /// 14.0.0. A key pair used to be built as LeftKey = binary.Left, RightKey = binary.Right + /// - taking the ORDER THE CONDITION WAS WRITTEN IN for the order of the join's inputs. So + /// ON c.Id = o.CustomerId worked and ON o.CustomerId = c.Id, the same condition, + /// failed at execution with Column 'CustomerId' not found: the hash join looked for the + /// right table's column in rows of the left one. + /// + /// + /// A schema is the only thing that can answer which side a column is on, so it is a parameter + /// rather than an option - the compiler then asks the question at every call site. + /// + /// + public static JoinConditionAnalysis Analyze( + WitSqlExpression? onCondition, + IReadOnlyList leftSchema) { if (onCondition == null) { @@ -31,7 +52,15 @@ public static JoinConditionAnalysis Analyze(WitSqlExpression? onCondition) var equiKeys = new List(); var residualParts = new List(); - AnalyzeRecursive(onCondition, equiKeys, residualParts); + var leftTables = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var column in leftSchema) + { + if (!string.IsNullOrEmpty(column.TableName)) + leftTables.Add(column.TableName); + } + + AnalyzeRecursive(onCondition, leftTables, equiKeys, residualParts); return new JoinConditionAnalysis { @@ -60,9 +89,9 @@ public static bool ShouldUseHashJoin(long leftRowCount, long rightRowCount, Join var smallerTable = Math.Min(leftRowCount, rightRowCount); var largerTable = Math.Max(leftRowCount, rightRowCount); - // Nested loop cost: O(N × M) + // Nested loop cost: O(N � M) // Hash join cost: O(N + M) + hash overhead - // Use hash join when N × M > threshold × (N + M) + // Use hash join when N � M > threshold � (N + M) var nestedLoopCost = leftRowCount * rightRowCount; var hashJoinCost = (leftRowCount + rightRowCount) * HASH_JOIN_THRESHOLD; @@ -89,6 +118,7 @@ public static bool ShouldBuildLeft(long leftRowCount, long rightRowCount) private static void AnalyzeRecursive( WitSqlExpression expression, + HashSet leftTables, List equiKeys, List residualParts) { @@ -98,13 +128,13 @@ private static void AnalyzeRecursive( if (binary.Operator == BinaryOperatorType.And) { // Recursively process AND conditions - AnalyzeRecursive(binary.Left, equiKeys, residualParts); - AnalyzeRecursive(binary.Right, equiKeys, residualParts); + AnalyzeRecursive(binary.Left, leftTables, equiKeys, residualParts); + AnalyzeRecursive(binary.Right, leftTables, equiKeys, residualParts); } else if (binary.Operator == BinaryOperatorType.Equal) { // Check if this is an equi-join condition (column = column) - if (TryExtractEquiJoinKey(binary, out var keyPair)) + if (TryExtractEquiJoinKey(binary, leftTables, out var keyPair)) { equiKeys.Add(keyPair!); } @@ -130,6 +160,7 @@ private static void AnalyzeRecursive( private static bool TryExtractEquiJoinKey( WitSqlExpressionBinary binary, + HashSet leftTables, out IteratorHashJoin.JoinKeyPair? keyPair) { keyPair = null; @@ -145,19 +176,40 @@ private static bool TryExtractEquiJoinKey( // Must have table qualifiers to distinguish join sides // If no table qualifier, we can't determine which side the column belongs to // In that case, treat as residual and let IteratorJoin handle it - if (leftCol.TableName == null && rightCol.TableName == null) + if (leftCol.TableName == null || rightCol.TableName == null) return false; // If both have same table name, it's not a join condition - if (leftCol.TableName != null && rightCol.TableName != null && - leftCol.TableName.Equals(rightCol.TableName, StringComparison.OrdinalIgnoreCase)) + if (leftCol.TableName.Equals(rightCol.TableName, StringComparison.OrdinalIgnoreCase)) + return false; + + // WHICH SIDE each column is on, which is not the same question as which side of the + // EQUALS SIGN it was written on. A condition means the same thing either way round, and + // until 14.0.1 this pair was built from the written order alone - so half of every join + // anyone wrote naturally failed with "Column not found" (KnownIssues 25). + var leftIsFromTheLeft = leftTables.Contains(leftCol.TableName); + var rightIsFromTheLeft = leftTables.Contains(rightCol.TableName); + + // Both from the left input is not a join key at all - it is a filter on that input, and + // hashing on it would look for the second column in the rows of the other side. The whole + // condition goes to the residual, where it is evaluated over the joined row instead. + if (leftIsFromTheLeft && rightIsFromTheLeft) return false; - keyPair = new IteratorHashJoin.JoinKeyPair + // NEITHER attributable is a different case, and it must keep the old behaviour rather than + // join the one above. A source can report no table name at all - INFORMATION_SCHEMA does, + // and its columns then resolve by name over the joined row, where the same name appears + // once per side. Sending those to the residual turned Studio's primary-key query into a + // cross product: measured, five of its cases went red before this branch was written. + if (!leftIsFromTheLeft && !rightIsFromTheLeft) { - LeftKey = binary.Left, - RightKey = binary.Right - }; + keyPair = new IteratorHashJoin.JoinKeyPair { LeftKey = binary.Left, RightKey = binary.Right }; + return true; + } + + keyPair = leftIsFromTheLeft + ? new IteratorHashJoin.JoinKeyPair { LeftKey = binary.Left, RightKey = binary.Right } + : new IteratorHashJoin.JoinKeyPair { LeftKey = binary.Right, RightKey = binary.Left }; return true; } diff --git a/Sources/Engine/OutWit.Database/OutWit.Database.csproj b/Sources/Engine/OutWit.Database/OutWit.Database.csproj index 7b797ad0..b863cb3e 100644 --- a/Sources/Engine/OutWit.Database/OutWit.Database.csproj +++ b/Sources/Engine/OutWit.Database/OutWit.Database.csproj @@ -5,7 +5,7 @@ enable enable - 14.0.0 + 14.0.1 SQL execution engine for WitDatabase. Full SQL support including JOINs, subqueries, CTEs, window functions, triggers, and 60+ built-in functions. OutWit;database;sql;engine;query;execution;cte;window-functions;triggers diff --git a/Sources/Engine/OutWit.Database/Query/QueryPlanner.Sources.cs b/Sources/Engine/OutWit.Database/Query/QueryPlanner.Sources.cs index 9e316209..51578554 100644 --- a/Sources/Engine/OutWit.Database/Query/QueryPlanner.Sources.cs +++ b/Sources/Engine/OutWit.Database/Query/QueryPlanner.Sources.cs @@ -247,7 +247,9 @@ private IResultIterator CreateJoinIterator(TableSourceJoin join) var right = CreateTableSourceIterator(join.Right); // Analyze join condition for hash join eligibility - var analysis = OptimizerJoinCondition.Analyze(join.OnCondition); + // The left input's schema, because it is what says which side of the join a column of the + // ON condition belongs to - see OptimizerJoinCondition.Analyze. + var analysis = OptimizerJoinCondition.Analyze(join.OnCondition, left.Schema); // Try hash join for INNER and LEFT joins with equi-join conditions if (analysis.HasEquiJoinKeys && diff --git a/Sources/Engine/OutWit.Database/README.md b/Sources/Engine/OutWit.Database/README.md index a607c71f..d490ad11 100644 --- a/Sources/Engine/OutWit.Database/README.md +++ b/Sources/Engine/OutWit.Database/README.md @@ -28,7 +28,7 @@ OutWit.Database is the SQL execution engine built on top of OutWit.Database.Core ## Installation ```xml - + ``` --- diff --git a/Sources/Engine/OutWit.Database/Statements/StatementExecutor.Explain.cs b/Sources/Engine/OutWit.Database/Statements/StatementExecutor.Explain.cs index 6c9de3f6..30db191e 100644 --- a/Sources/Engine/OutWit.Database/Statements/StatementExecutor.Explain.cs +++ b/Sources/Engine/OutWit.Database/Statements/StatementExecutor.Explain.cs @@ -59,28 +59,31 @@ private List BuildPlanDescription(IResultIterator iterator, int depth, ? GetQueryPlanDescription(iterator) : GetDetailedDescription(iterator); - var currentId = lines.Count; - lines.Add(new PlanLine(depth > 0 ? currentId - 1 : -1, $"{indent}{detail}")); - + // This node is line 0 of its own subtree, and its parent is filled in by whoever appends + // that subtree - the top-level call is the one that keeps -1. + lines.Add(new PlanLine(-1, $"{indent}{detail}")); + // Add child iterators if they exist var children = GetChildIterators(iterator); foreach (var child in children) { - var childLines = BuildPlanDescription(child, depth + 1, queryPlan); - // Update parent references for child lines - foreach (var line in childLines) + // WHERE THIS CHILD'S SUBTREE STARTS, which is the whole of a defect shipped in 14.0.0: + // the lines used to be re-based by a constant (`currentId + 1`), which is right for the + // FIRST child and wrong for every one after it by the size of everything already + // appended. A join has two children, and so does every set operation - so EXPLAIN put + // the right input's scan under the LEFT input's alias, and anything drawing the plan as + // a tree drew the wrong tree faithfully (KnownIssues 26). + var offset = lines.Count; + + foreach (var line in BuildPlanDescription(child, depth + 1, queryPlan)) { - if (line.Parent == -1 && depth >= 0) - { - lines.Add(new PlanLine(currentId, line.Detail)); - } - else - { - lines.Add(new PlanLine(line.Parent + currentId + 1, line.Detail)); - } + // A subtree's own root is the child itself: it goes under THIS node, which is line + // 0 of this list. Everything else keeps the parent it had, moved to where the + // subtree has landed. + lines.Add(new PlanLine(line.Parent == -1 ? 0 : line.Parent + offset, line.Detail)); } } - + return lines; } diff --git a/Sources/Providers/OutWit.Database.AdoNet/OutWit.Database.AdoNet.csproj b/Sources/Providers/OutWit.Database.AdoNet/OutWit.Database.AdoNet.csproj index 9d4cff58..8c7f8f45 100644 --- a/Sources/Providers/OutWit.Database.AdoNet/OutWit.Database.AdoNet.csproj +++ b/Sources/Providers/OutWit.Database.AdoNet/OutWit.Database.AdoNet.csproj @@ -5,7 +5,7 @@ enable enable - 14.0.0 + 14.0.1 ADO.NET data provider for WitDatabase. Full System.Data compatibility with connection pooling, transactions, and parameterized queries. OutWit;database;ado-net;data-provider;sql;connection;transactions;dotnet diff --git a/Sources/Providers/OutWit.Database.EntityFramework/OutWit.Database.EntityFramework.csproj b/Sources/Providers/OutWit.Database.EntityFramework/OutWit.Database.EntityFramework.csproj index f74256d4..d8afcb65 100644 --- a/Sources/Providers/OutWit.Database.EntityFramework/OutWit.Database.EntityFramework.csproj +++ b/Sources/Providers/OutWit.Database.EntityFramework/OutWit.Database.EntityFramework.csproj @@ -5,7 +5,7 @@ enable enable - 14.0.0 + 14.0.1 Entity Framework Core provider for WitDatabase. Full EF Core support including migrations, scaffolding, LINQ translation, and design-time services. OutWit;database;entity-framework;ef-core;orm;linq;migrations;scaffolding