diff --git a/Docs/KnownIssues.md b/Docs/KnownIssues.md index e9eb6d5..5080fb8 100644 --- a/Docs/KnownIssues.md +++ b/Docs/KnownIssues.md @@ -1620,6 +1620,12 @@ Studio’s Plan panel does - draws the wrong tree, faithfully. **The renderer is ## 27. Studio: a function or a procedure can only be refreshed +> **FIXED in Studio 3.1.2.** A routine is offered *View definition* and *Drop*, the definition is +> the whole `CREATE` assembled from `ROUTINES` and `PARAMETERS`, and - found beside it - +> **`Dump Database…` had been writing no routines at all**, so a dumped database restored without +> its functions and said nothing. `ARoutineIsAnObjectLikeAnyOtherTests` drops each routine and runs +> the definition Studio wrote back into the database. +> > Studio 3.1.1. A gap rather than a wrong answer. The tree’s context menu offers a routine exactly one item, `Refresh`. The engine has @@ -1628,7 +1634,7 @@ inspector on the right shows it - so both *View definition* and *Drop* are possi offered. Every other kind of object in the tree was given its own menu in 3.1.0; routines were not included. -Until they are: drop a routine by running the statement in a query tab. +Until 3.1.2 the way to drop a routine was to run the statement in a query tab. --- ## Verifying a fix diff --git a/Tools/OutWit.Database.Studio.Tests/ViewModels/ARoutineIsAnObjectLikeAnyOtherTests.cs b/Tools/OutWit.Database.Studio.Tests/ViewModels/ARoutineIsAnObjectLikeAnyOtherTests.cs new file mode 100644 index 0000000..89e61f6 --- /dev/null +++ b/Tools/OutWit.Database.Studio.Tests/ViewModels/ARoutineIsAnObjectLikeAnyOtherTests.cs @@ -0,0 +1,299 @@ +using OutWit.Database.Studio.Models; +using OutWit.Database.Studio.Tests.Helpers; + +namespace OutWit.Database.Studio.Tests.ViewModels; + +/// +/// A function and a procedure are objects in the tree, not labels in it. +/// +/// +/// +/// Known issue 27, found by clicking through 3.1.1. The sixth folder arrived with WS-21 and +/// nothing else did: a routine's context menu offered one item, Refresh, while the engine has +/// had DROP FUNCTION and DROP PROCEDURE since phase 9d and the catalogue already +/// carried the body - the inspector on the right was showing it. +/// +/// +/// And the dump had the same hole, which is worse than a missing menu item: it wrote views, +/// indexes and triggers, so a database with routines dumped to a script that restored without them +/// and said nothing about it. +/// +/// +/// The assertion that matters here is not "a definition is produced" but that the definition RUNS +/// BACK: the routine is dropped and the text Studio wrote is executed, and the routine is there +/// again and still answers. +/// +/// +[TestFixture] +public class ARoutineIsAnObjectLikeAnyOtherTests +{ + #region Constants + + private const string FUNCTION = "CREATE FUNCTION DiscountedTotal(Amount DECIMAL(18,2), Percent INT) " + + "RETURNS DECIMAL(18,2) AS BEGIN RETURN (Amount - ((Amount * Percent) / 100)); END"; + + private const string PROCEDURE = "CREATE PROCEDURE ArchiveOld() AS BEGIN " + + "UPDATE Orders SET Status = 'archived' WHERE Status = 'new'; END"; + + #endregion + + #region Fields + + private StudioFixture m_studio = null!; + + #endregion + + #region Setup + + [SetUp] + public async Task SetUp() + { + m_studio = await StudioFixture.CreateAsync(); + + await m_studio.Database.ExecuteNonQueryAsync(FUNCTION); + await m_studio.Database.ExecuteNonQueryAsync(PROCEDURE); + + await m_studio.Explorer.RefreshAsync(); + } + + [TearDown] + public async Task TearDown() + { + await m_studio.DisposeAsync(); + } + + #endregion + + #region What the tree offers + + [Test] + public void ARoutineIsOfferedItsDefinitionAndItsRemovalTest() + { + Select("DiscountedTotal"); + + var explorer = m_studio.Explorer; + + Assert.Multiple(() => + { + Assert.That(explorer.ShowsViewDefinition, Is.True, "the catalogue holds its body"); + Assert.That(explorer.ShowsDrop, Is.True, "and the engine has DROP FUNCTION"); + + Assert.That(explorer.CanViewDefinition, Is.True); + Assert.That(explorer.CanDropObject, Is.True); + + // What a routine still does NOT have, so that this is not a licence to offer everything. + Assert.That(explorer.ShowsRename, Is.False, "there is no ALTER FUNCTION in this language"); + Assert.That(explorer.ShowsEditData, Is.False); + Assert.That(explorer.ShowsBrowseData, Is.False); + }); + } + + [Test] + public void TheTreeKnowsWhichOfTheTwoEachOneIsTest() + { + Assert.Multiple(() => + { + Assert.That(Node("DiscountedTotal").IsFunction, Is.True); + Assert.That(Node("ArchiveOld").IsFunction, Is.False); + + // A fact rather than a rendering: the label is what the fact is drawn as, not the other + // way round. + Assert.That(Node("DiscountedTotal").Detail, Does.StartWith("function")); + Assert.That(Node("ArchiveOld").Detail, Is.EqualTo("procedure")); + }); + } + + #endregion + + #region The definition runs back + + [Test] + public async Task TheDefinitionOfAFunctionRunsBackIntoTheDatabaseTest() + { + await ItRunsBackAsync("DiscountedTotal", isFunction: true); + } + + [Test] + public async Task TheDefinitionOfAProcedureRunsBackIntoTheDatabaseTest() + { + await ItRunsBackAsync("ArchiveOld", isFunction: false); + } + + /// + /// A function's parameters and return type are part of it. A definition without them parses and + /// creates a DIFFERENT routine, which is the failure this case exists to catch. + /// + [Test] + public async Task TheDefinitionCarriesTheParametersAndTheReturnTypeTest() + { + var definition = await m_studio.Database.GetRoutineDefinitionAsync("DiscountedTotal"); + + Assert.That(definition, Is.Not.Null); + + // The SIGNATURE, not the whole text: the parameter names also appear in the body, so + // «the definition contains Amount» passes on a definition with no parameters at all - + // measured, by removing them. + var signature = definition![..definition.IndexOf("RETURNS", StringComparison.Ordinal)]; + + Assert.Multiple(() => + { + Assert.That(signature, Does.Contain("Amount")); + Assert.That(signature, Does.Contain("Percent")); + Assert.That(signature, Does.Contain("DECIMAL(18,2)"), + "a parameter carries its type WITH its precision - DECIMAL alone restores a " + + "routine that rounds differently from the one in the database"); + + Assert.That(definition, Does.Contain("RETURNS DECIMAL"), "and the function its return type"); + }); + } + + #endregion + + #region Dropping one + + [Test] + public async Task DroppingAFunctionTakesItOutOfTheDatabaseAndTheTreeTest() + { + Select("DiscountedTotal"); + + m_studio.Confirmations.AllowDestructive = true; + + await StudioFixture.PressAsync(m_studio.Explorer.DropObjectCommand); + + Assert.Multiple(() => + { + Assert.That(Routines(), Does.Not.Contain("DiscountedTotal")); + Assert.That(Walk().Any(node => node.Name == "DiscountedTotal"), Is.False, + "and the tree was refreshed"); + }); + } + + [Test] + public async Task DroppingAProcedureTakesItOutTooTest() + { + Select("ArchiveOld"); + + m_studio.Confirmations.AllowDestructive = true; + + await StudioFixture.PressAsync(m_studio.Explorer.DropObjectCommand); + + Assert.That(Routines(), Does.Not.Contain("ArchiveOld")); + } + + /// + /// The other direction: a refused question leaves the routine alone. DROP PROCEDURE and DROP + /// FUNCTION are different statements, and a menu that asks and drops anyway would be worse than + /// one that never offered. + /// + [Test] + public async Task ARefusedQuestionLeavesTheRoutineAloneTest() + { + Select("DiscountedTotal"); + + m_studio.Confirmations.AllowDestructive = false; + + await StudioFixture.PressAsync(m_studio.Explorer.DropObjectCommand); + + Assert.That(Routines(), Does.Contain("DiscountedTotal")); + } + + /// + /// The line under the tree counts what the tree draws. It named five folders while the tree + /// drew six, so a database whose only objects are routines was summarised as empty. + /// + [Test] + public async Task TheSummaryCountsTheSixthFolderTest() + { + await m_studio.Explorer.RefreshAsync(); + + Assert.Multiple(() => + { + Assert.That(m_studio.MainWindow.StatusText, Does.Contain("2 routines")); + + // CONTROL: the same line, still naming what it named before. + Assert.That(m_studio.MainWindow.StatusText, Does.Contain("1 trigger")); + }); + } + + #endregion + + #region The dump + + [Test] + public async Task ADumpCarriesTheRoutinesTest() + { + var script = await Studio.Services.DatabaseDump.WriteAsync( + m_studio.Database, new Studio.Services.DumpOptions()); + + Assert.Multiple(() => + { + Assert.That(script, Does.Contain("CREATE FUNCTION"), "a dump without them restores without them"); + Assert.That(script, Does.Contain("DiscountedTotal")); + + Assert.That(script, Does.Contain("CREATE PROCEDURE")); + Assert.That(script, Does.Contain("ArchiveOld")); + + // CONTROL: the dump is the real one, with the objects that were already written. + Assert.That(script, Does.Contain("CREATE TABLE")); + Assert.That(script, Does.Contain("CREATE TRIGGER")); + }); + } + + #endregion + + #region Tools + + private async Task ItRunsBackAsync(string name, bool isFunction) + { + var definition = await m_studio.Database.GetRoutineDefinitionAsync(name); + + Assert.That(definition, Is.Not.Null, $"{name} has a definition"); + + await m_studio.Database.ExecuteNonQueryAsync( + $"DROP {(isFunction ? "FUNCTION" : "PROCEDURE")} {name}"); + + Assume.That(Routines(), Does.Not.Contain(name), "it is gone before the definition is run"); + + await m_studio.Database.ExecuteNonQueryAsync(definition!); + + Assert.That(Routines(), Does.Contain(name), + "the definition Studio wrote does not restore the routine:" + Environment.NewLine + definition); + } + + private IReadOnlyList Routines() + { + return m_studio.Database.GetRoutinesAsync().GetAwaiter().GetResult() + .Select(routine => routine.Name) + .ToList(); + } + + private DatabaseNode Node(string name) + { + var node = Walk().FirstOrDefault(candidate => + candidate.NodeType == DatabaseNodeType.Routine && candidate.Name == name); + + Assert.That(node, Is.Not.Null, $"the tree has a routine called {name}"); + + return node!; + } + + private void Select(string name) + { + m_studio.Explorer.SelectedNode = Node(name); + } + + private IEnumerable Walk() + { + return m_studio.Explorer.Nodes.SelectMany(Flatten); + } + + private static IEnumerable Flatten(DatabaseNode node) + { + yield return node; + + foreach (var child in node.Children.SelectMany(Flatten)) + yield return child; + } + + #endregion +} diff --git a/Tools/OutWit.Database.Studio/CHANGELOG.md b/Tools/OutWit.Database.Studio/CHANGELOG.md index d3f0c6d..2aefeb0 100644 --- a/Tools/OutWit.Database.Studio/CHANGELOG.md +++ b/Tools/OutWit.Database.Studio/CHANGELOG.md @@ -3,6 +3,44 @@ Studio is versioned separately from the WitDatabase engine and released under its own `studio-v*` tag. The engine's changelog is `/CHANGELOG.md`. +## 3.1.2 + +**Engine: 14.0.1**, and that is the first reason for this release. Two planner defects were found +by using Studio and fixed in the engine the same day: a join condition written `ON right.x = +left.y` was refused outright, and `EXPLAIN` gave the right input’s child the wrong parent - so the +Plan panel drew the wrong tree faithfully. Studio needed no change for either; it needed the +engine. + +### A function and a procedure are objects, not labels + +The sixth folder arrived with WS-21 and nothing else did: a routine’s context menu offered one +item, *Refresh*. It now offers **View definition** and **Drop**, like every other kind of object +in the tree - the engine has had `DROP FUNCTION` and `DROP PROCEDURE` since phase 9d, and the +catalogue already carried the body the inspector was showing. + +The definition is the whole `CREATE`, assembled from `INFORMATION_SCHEMA.ROUTINES` and +`PARAMETERS`: a body without its parameters and return type parses and creates a DIFFERENT +routine. A routine the catalogue cannot render is NAMED rather than half-written, which is the +rule a view and a trigger already followed. + +### And the dump was losing them + +Worse than the missing menu item, and found beside it: **`Dump Database…` wrote views, indexes and +triggers, and no routines at all.** A database with functions dumped to a script that restored +without them and said nothing. The dump now carries them, and names one it cannot render. + +The test that matters is not that a definition appears but that it **runs back**: each routine is +dropped and the text Studio wrote is executed, and the routine is there again. + +### And the line under the tree counts six folders + +It named five - tables, views, indexes, triggers, sequences - while the tree has drawn six since +WS-21, so a database whose only objects are routines was summarised as having nothing in it. + +### Known issues + +Issue 27 of [Docs/KnownIssues.md](../../Docs/KnownIssues.md) is this, and is marked fixed. Issues +25 and 26 went with engine 14.0.1. ## 3.1.1 **Four corrections, three of them to things 3.1.0 itself broke.** They were found the day 3.1.0 diff --git a/Tools/OutWit.Database.Studio/Models/DatabaseNode.cs b/Tools/OutWit.Database.Studio/Models/DatabaseNode.cs index ab953b8..41ec7cc 100644 --- a/Tools/OutWit.Database.Studio/Models/DatabaseNode.cs +++ b/Tools/OutWit.Database.Studio/Models/DatabaseNode.cs @@ -67,6 +67,19 @@ public override DatabaseNode Clone() [Notify] public bool IsExpanded { get; set; } + /// + /// For a : whether it is a function rather than a + /// procedure. Null for every other kind of node. + /// + /// + /// One node type covers both because the tree draws them in one folder, and two DDL keywords sit + /// behind it - DROP FUNCTION and DROP PROCEDURE are different statements, and + /// "function deleted" and "procedure deleted" are different sentences in a language with cases. + /// The alternative, reading it back out of the label, is a rendering answering a question about + /// the schema. + /// + public bool? IsFunction { get; set; } + /// /// A stand-in child, there so that the node draws an expander. /// diff --git a/Tools/OutWit.Database.Studio/Models/IndexDraft.cs b/Tools/OutWit.Database.Studio/Models/IndexDraft.cs index 5098c6f..934d93b 100644 --- a/Tools/OutWit.Database.Studio/Models/IndexDraft.cs +++ b/Tools/OutWit.Database.Studio/Models/IndexDraft.cs @@ -106,3 +106,39 @@ public sealed class TriggerDraft /// public string Body { get; set; } = string.Empty; } + +/// +/// A function or a procedure, as much of one as the catalogue can answer for. +/// +/// +/// The two kinds are one draft because they differ in three places - the keyword, the RETURNS clause +/// and what stands between BEGIN and END - and are otherwise the same object. The engine has had both +/// since phase 9d; nothing outside the tree has ever written one. +/// +public sealed class RoutineDraft +{ + public string Name { get; set; } = string.Empty; + + /// A function returns a value and a procedure does not, which is the whole difference. + public bool IsFunction { get; set; } + + /// What a function returns. Null for a procedure. + public string? ReturnType { get; set; } + + /// The parameters in ordinal order, as name and type. + public IReadOnlyList Parameters { get; set; } = []; + + /// + /// The expression a function returns, or the statements a procedure runs - as the catalogue + /// renders them, without BEGIN and END. + /// + public string Body { get; set; } = string.Empty; +} + +/// One parameter of a routine. +public sealed class RoutineParameterDraft +{ + public string Name { get; set; } = string.Empty; + + public string Type { get; set; } = string.Empty; +} diff --git a/Tools/OutWit.Database.Studio/OutWit.Database.Studio.csproj b/Tools/OutWit.Database.Studio/OutWit.Database.Studio.csproj index 03b2450..7b99040 100644 --- a/Tools/OutWit.Database.Studio/OutWit.Database.Studio.csproj +++ b/Tools/OutWit.Database.Studio/OutWit.Database.Studio.csproj @@ -18,7 +18,7 @@ - 3.1.1 + 3.1.2 WitDatabase Studio OutWit diff --git a/Tools/OutWit.Database.Studio/Resources/Strings.en.json b/Tools/OutWit.Database.Studio/Resources/Strings.en.json index 83498e4..e3b1d44 100644 --- a/Tools/OutWit.Database.Studio/Resources/Strings.en.json +++ b/Tools/OutWit.Database.Studio/Resources/Strings.en.json @@ -784,7 +784,7 @@ "Explorer.Folder.Triggers": "Triggers", "Explorer.Folder.Sequences": "Sequences", "Explorer.Folder.Routines": "Routines", - "Explorer.Summary": "{0}: {1}, {2}, {3}, {4}, {5}", + "Explorer.Summary": "{0}: {1}, {2}, {3}, {4}, {5}, {6}", "Query.Autocommit": "Autocommit", "Query.TransactionOpen": "Transaction open · {0}", "Settings.Theme.System": "System", @@ -889,6 +889,10 @@ "one": "{0} sequence", "other": "{0} sequences" }, + "Count.Routines": { + "one": "{0} routine", + "other": "{0} routines" + }, "Count.Tabs": { "one": "{0} tab", "other": "{0} tabs" @@ -1129,6 +1133,12 @@ "Confirm.Drop.Headline.Sequence": "Delete sequence \"{0}\"? This cannot be undone.", "Explorer.Dropped.Sequence": "Sequence deleted: {0}", "Explorer.DropFailed.Sequence": "Could not delete the sequence: {0}", + "Confirm.Drop.Headline.Function": "Delete function \"{0}\"? This cannot be undone.", + "Explorer.Dropped.Function": "Function deleted: {0}", + "Explorer.DropFailed.Function": "Could not delete the function: {0}", + "Confirm.Drop.Headline.Procedure": "Delete procedure \"{0}\"? This cannot be undone.", + "Explorer.Dropped.Procedure": "Procedure deleted: {0}", + "Explorer.DropFailed.Procedure": "Could not delete the procedure: {0}", "Query.ReadOnly.Banner": "This connection is open read-only - a write will be refused by the engine.", "Query.ReadOnly.Short": "Read-only" } diff --git a/Tools/OutWit.Database.Studio/Resources/Strings.ru.json b/Tools/OutWit.Database.Studio/Resources/Strings.ru.json index 1ad341f..9d8e117 100644 --- a/Tools/OutWit.Database.Studio/Resources/Strings.ru.json +++ b/Tools/OutWit.Database.Studio/Resources/Strings.ru.json @@ -797,7 +797,7 @@ "Explorer.Folder.Triggers": "Триггеры", "Explorer.Folder.Sequences": "Последовательности", "Explorer.Folder.Routines": "Процедуры и функции", - "Explorer.Summary": "{0}: {1}, {2}, {3}, {4}, {5}", + "Explorer.Summary": "{0}: {1}, {2}, {3}, {4}, {5}, {6}", "Query.Autocommit": "Автофиксация", "Query.TransactionOpen": "Транзакция открыта · {0}", "Settings.Theme.System": "Как в системе", @@ -909,6 +909,11 @@ "few": "{0} последовательности", "many": "{0} последовательностей" }, + "Count.Routines": { + "one": "{0} подпрограмма", + "few": "{0} подпрограммы", + "many": "{0} подпрограмм" + }, "Count.Tabs": { "one": "{0} вкладка", "few": "{0} вкладки", @@ -1153,6 +1158,12 @@ "Confirm.Drop.Headline.Sequence": "Удалить последовательность «{0}»? Отменить будет нельзя.", "Explorer.Dropped.Sequence": "Последовательность удалена: {0}", "Explorer.DropFailed.Sequence": "Не удалось удалить последовательность: {0}", + "Confirm.Drop.Headline.Function": "Удалить функцию «{0}»? Отменить это будет нельзя.", + "Explorer.Dropped.Function": "Функция удалена: {0}", + "Explorer.DropFailed.Function": "Не удалось удалить функцию: {0}", + "Confirm.Drop.Headline.Procedure": "Удалить процедуру «{0}»? Отменить это будет нельзя.", + "Explorer.Dropped.Procedure": "Процедура удалена: {0}", + "Explorer.DropFailed.Procedure": "Не удалось удалить процедуру: {0}", "Query.ReadOnly.Banner": "Подключение открыто только для чтения — запись будет отклонена движком.", "Query.ReadOnly.Short": "Только чтение" } diff --git a/Tools/OutWit.Database.Studio/Services/DatabaseDump.cs b/Tools/OutWit.Database.Studio/Services/DatabaseDump.cs index 8d30b55..2a48a29 100644 --- a/Tools/OutWit.Database.Studio/Services/DatabaseDump.cs +++ b/Tools/OutWit.Database.Studio/Services/DatabaseDump.cs @@ -215,6 +215,18 @@ private static async Task WriteDefinitionsAsync(IDatabaseSession session, String ? $"-- SKIPPED: the catalogue cannot render trigger {trigger}." : definition.TrimEnd().TrimEnd(';') + ";"); } + + // Functions and procedures, since 3.1.2. They were absent, so a database with routines dumped + // to a script that restored WITHOUT THEM and said nothing - the exact failure the rule above + // exists to prevent, in the one object kind the rule had never been applied to. + foreach (var routine in await session.GetRoutinesAsync(ct)) + { + var definition = await session.GetRoutineDefinitionAsync(routine.Name, ct); + + script.AppendLine(string.IsNullOrEmpty(definition) + ? $"-- SKIPPED: the catalogue cannot render {(routine.IsFunction ? "function" : "procedure")} {routine.Name}." + : definition.TrimEnd().TrimEnd(';') + ";"); + } } #endregion diff --git a/Tools/OutWit.Database.Studio/Services/DatabaseSession.Ddl.cs b/Tools/OutWit.Database.Studio/Services/DatabaseSession.Ddl.cs index 1bdb50a..d6a655d 100644 --- a/Tools/OutWit.Database.Studio/Services/DatabaseSession.Ddl.cs +++ b/Tools/OutWit.Database.Studio/Services/DatabaseSession.Ddl.cs @@ -146,6 +146,121 @@ FROM INFORMATION_SCHEMA.TRIGGERS } } + /// + /// + /// + /// A routine whose body the catalogue cannot render is NAMED by the caller, not written half + /// way - the phase-8 rule, the same one a view and a trigger already follow here. + /// + /// + /// The parameters come from INFORMATION_SCHEMA.PARAMETERS in ordinal order, and the row + /// that describes what a function RETURNS is excluded by IS_RESULT: it is a parameter in + /// the standard’s model and not one in this grammar, where the return type is written after + /// RETURNS. + /// + /// + public async Task GetRoutineDefinitionAsync(string routineName, CancellationToken ct = default) + { + EnsureConnected(); + + try + { + const string sql = @" + SELECT ROUTINE_TYPE, DATA_TYPE, ROUTINE_DEFINITION + FROM INFORMATION_SCHEMA.ROUTINES + WHERE ROUTINE_NAME = @routineName"; + + using var command = m_connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("@routineName", routineName); + + string type, body; + string? dataType; + + using (var reader = await command.ExecuteReaderAsync(ct)) + { + if (!await reader.ReadAsync(ct)) + return null; + + type = reader.IsDBNull(0) ? "FUNCTION" : reader.GetString(0); + dataType = reader.IsDBNull(1) ? null : reader.GetString(1); + + if (reader.IsDBNull(2) || string.IsNullOrWhiteSpace(reader.GetString(2))) + return null; + + body = reader.GetString(2); + } + + var isFunction = type.Equals("FUNCTION", StringComparison.OrdinalIgnoreCase); + + // A function with no return type cannot be written: RETURNS is not optional in this + // grammar, and guessing one would produce a routine that is not the one in the database. + if (isFunction && string.IsNullOrWhiteSpace(dataType)) + return null; + + return DdlWriter.CreateRoutine(new RoutineDraft + { + Name = routineName, + IsFunction = isFunction, + ReturnType = dataType, + Parameters = await ReadRoutineParametersAsync(routineName, ct), + Body = body + }); + } + catch (Exception ex) + { + m_logger.LogDebug(ex, "Failed to get routine definition for {RoutineName}", routineName); + return null; + } + } + + /// + /// One routine's parameters, in the order they are declared. + /// + /// + /// Deliberately NOT wrapped in a catch of its own, for the reason + /// gives: a routine written without the parameters it has is + /// a different routine, and the caller's catch reports it as one that cannot be written - which + /// is the honest answer. + /// + internal async Task> ReadRoutineParametersAsync( + string routineName, CancellationToken ct = default) + { + // The length and the precision are read with the type, and composed with it. Without them a + // DECIMAL(18,2) parameter comes back as DECIMAL and a VARCHAR(60) as VARCHAR - a definition + // that parses, restores, and is not the routine that was there. + const string sql = @" + SELECT PARAMETER_NAME, DATA_TYPE, + CHARACTER_MAXIMUM_LENGTH, NUMERIC_PRECISION, NUMERIC_SCALE + FROM INFORMATION_SCHEMA.PARAMETERS + WHERE SPECIFIC_NAME = @routineName + AND IS_RESULT = 'NO' + ORDER BY ORDINAL_POSITION"; + + using var command = m_connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("@routineName", routineName); + + using var reader = await command.ExecuteReaderAsync(ct); + + var parameters = new List(); + + while (await reader.ReadAsync(ct)) + { + parameters.Add(new RoutineParameterDraft + { + Name = reader.GetString(0), + Type = ColumnDraft.FormatType( + reader.IsDBNull(1) ? "TEXT" : reader.GetString(1), + reader.IsDBNull(2) ? null : reader.GetInt32(2), + reader.IsDBNull(3) ? null : reader.GetInt32(3), + reader.IsDBNull(4) ? null : reader.GetInt32(4)) + }); + } + + return parameters; + } + /// /// The columns one trigger's UPDATE OF names, as the standard publishes them: one row per /// column, and none at all when the trigger watches every column. diff --git a/Tools/OutWit.Database.Studio/Services/DdlWriter.cs b/Tools/OutWit.Database.Studio/Services/DdlWriter.cs index 7a19f4a..152949b 100644 --- a/Tools/OutWit.Database.Studio/Services/DdlWriter.cs +++ b/Tools/OutWit.Database.Studio/Services/DdlWriter.cs @@ -272,6 +272,50 @@ public static string CreateView(string name, string body) => #endregion + #region Routines + + /// + /// A whole CREATE FUNCTION or CREATE PROCEDURE, from what the catalogue answers. + /// + /// + /// + /// The brackets after the name are written even when there are no parameters: this grammar has + /// LPAREN routineParameters? RPAREN, so they are not optional and a function written + /// without them does not parse. + /// + /// + /// A function’s body is one expression and is written after RETURN; a procedure’s is a + /// statement list and stands on its own, exactly as a trigger’s does. + /// + /// + public static string CreateRoutine(RoutineDraft routine) + { + var keyword = routine.IsFunction ? "FUNCTION" : "PROCEDURE"; + + var parameters = string.Join(", ", routine.Parameters + .Select(parameter => $"{Identifier(parameter.Name)} {parameter.Type}")); + + var sb = new StringBuilder($"CREATE {keyword} {Identifier(routine.Name)}({parameters})"); + + if (routine.IsFunction) + sb.Append($" RETURNS {routine.ReturnType}"); + + sb.Append("\nAS BEGIN\n"); + + var body = routine.Body.Trim().TrimEnd(';'); + + sb.Append(Indent(routine.IsFunction ? $"RETURN {body};" : body + ";")); + + sb.Append("\nEND;"); + + return sb.ToString(); + } + + public static string DropRoutine(string name, bool isFunction) => + $"DROP {(isFunction ? "FUNCTION" : "PROCEDURE")} {Identifier(name)};"; + + #endregion + #region Tools private static string Columns(IEnumerable columns) => diff --git a/Tools/OutWit.Database.Studio/Services/IDatabaseSession.cs b/Tools/OutWit.Database.Studio/Services/IDatabaseSession.cs index aa3a5be..3ef15e7 100644 --- a/Tools/OutWit.Database.Studio/Services/IDatabaseSession.cs +++ b/Tools/OutWit.Database.Studio/Services/IDatabaseSession.cs @@ -182,6 +182,17 @@ public interface IDatabaseSession /// Task GetIndexDefinitionAsync(string indexName, CancellationToken ct = default); + /// + /// Gets the definition (DDL) for a function or a procedure - the whole CREATE, assembled + /// from INFORMATION_SCHEMA.ROUTINES and INFORMATION_SCHEMA.PARAMETERS. + /// + /// + /// The catalogue’s ROUTINE_DEFINITION is the body alone, and a body without its + /// parameters and return type is not something that can be run back into a database - the same + /// reason a trigger is assembled here rather than reported as its ACTION_STATEMENT. + /// + Task GetRoutineDefinitionAsync(string routineName, CancellationToken ct = default); + /// /// Gets the definition (DDL) for a table (CREATE TABLE statement). /// diff --git a/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs b/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs index 1594d04..4b84e1f 100644 --- a/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs +++ b/Tools/OutWit.Database.Studio/ViewModels/DatabaseExplorerViewModel.cs @@ -344,6 +344,7 @@ private async Task ViewDefinitionAsync() DatabaseNodeType.Table => await session.GetTableDefinitionAsync(SelectedNode.Name), DatabaseNodeType.View => await session.GetViewDefinitionAsync(SelectedNode.Name), DatabaseNodeType.Trigger => await session.GetTriggerDefinitionAsync(SelectedNode.Name), + DatabaseNodeType.Routine => await session.GetRoutineDefinitionAsync(SelectedNode.Name), DatabaseNodeType.Index => await session.GetIndexDefinitionAsync(SelectedNode.Name), _ => null }; @@ -381,6 +382,11 @@ private async Task DropObjectAsync() DatabaseNodeType.Index => "INDEX", DatabaseNodeType.Trigger => "TRIGGER", DatabaseNodeType.Sequence => "SEQUENCE", + + // Two keywords behind one node type: what the tree calls a routine is a FUNCTION or a + // PROCEDURE, and the node carries which. + DatabaseNodeType.Routine => SelectedNode.IsFunction == true ? "FUNCTION" : "PROCEDURE", + _ => null }; @@ -398,6 +404,10 @@ private async Task DropObjectAsync() // time. Found by running the application, which is the only place it was ever visible. var nodeType = SelectedNode.NodeType; + // Captured with it, and for the same reason: a routine is a function or a procedure, and + // they are two nouns. + var isFunction = SelectedNode.IsFunction == true; + // WS-20. Until 2026-08-10 this method went straight to ExecuteNonQueryAsync: one click in the // tree and the table was gone, while the settings page showed a ticked "ask before dropping an // object". The question is asked through the confirmation service so that the SETTING is @@ -408,7 +418,7 @@ private async Task DropObjectAsync() var proceed = await ApplicationVm.Confirmations.AskAboutDestructiveActionAsync( new DestructiveAction( ConfirmationKind.DroppingObject, - Localization.Format(SentenceKey("Confirm.Drop.Headline", nodeType), objectName), + Localization.Format(SentenceKey("Confirm.Drop.Headline", nodeType, isFunction), objectName), sql, consequences)); @@ -428,13 +438,13 @@ private async Task DropObjectAsync() await RefreshAsync(session); - ApplicationVm.MainWindowVm.StatusText = Localization.Format(SentenceKey("Explorer.Dropped", nodeType), objectName); + ApplicationVm.MainWindowVm.StatusText = Localization.Format(SentenceKey("Explorer.Dropped", nodeType, isFunction), objectName); Logger.LogInformation("Dropped {ObjectType}: {ObjectName} in {Connection}", objectType, objectName, session.DisplayName); } catch (Exception ex) { - ErrorMessage = Localization.Format(SentenceKey("Explorer.DropFailed", nodeType), ex.Message); + ErrorMessage = Localization.Format(SentenceKey("Explorer.DropFailed", nodeType, isFunction), ex.Message); Logger.LogError(ex, "Failed to drop {ObjectType}: {ObjectName}", objectType, objectName); } } @@ -462,28 +472,49 @@ private async Task DropObjectAsync() /// gets one of them wrong. /// /// - private static string SentenceKey(string family, DatabaseNodeType nodeType) => (family, nodeType) switch + private static string SentenceKey(string family, DatabaseNodeType nodeType, bool isFunction = false) { - ("Confirm.Drop.Headline", DatabaseNodeType.Table) => "Confirm.Drop.Headline.Table", - ("Confirm.Drop.Headline", DatabaseNodeType.View) => "Confirm.Drop.Headline.View", - ("Confirm.Drop.Headline", DatabaseNodeType.Index) => "Confirm.Drop.Headline.Index", - ("Confirm.Drop.Headline", DatabaseNodeType.Trigger) => "Confirm.Drop.Headline.Trigger", - ("Confirm.Drop.Headline", DatabaseNodeType.Sequence) => "Confirm.Drop.Headline.Sequence", - - ("Explorer.Dropped", DatabaseNodeType.Table) => "Explorer.Dropped.Table", - ("Explorer.Dropped", DatabaseNodeType.View) => "Explorer.Dropped.View", - ("Explorer.Dropped", DatabaseNodeType.Index) => "Explorer.Dropped.Index", - ("Explorer.Dropped", DatabaseNodeType.Trigger) => "Explorer.Dropped.Trigger", - ("Explorer.Dropped", DatabaseNodeType.Sequence) => "Explorer.Dropped.Sequence", - - ("Explorer.DropFailed", DatabaseNodeType.Table) => "Explorer.DropFailed.Table", - ("Explorer.DropFailed", DatabaseNodeType.View) => "Explorer.DropFailed.View", - ("Explorer.DropFailed", DatabaseNodeType.Index) => "Explorer.DropFailed.Index", - ("Explorer.DropFailed", DatabaseNodeType.Trigger) => "Explorer.DropFailed.Trigger", - ("Explorer.DropFailed", DatabaseNodeType.Sequence) => "Explorer.DropFailed.Sequence", - - _ => family + ".Table" - }; + // A ROUTINE is two nouns behind one node type, so the kind is resolved first and the + // keys stay whole below. + var kind = nodeType switch + { + DatabaseNodeType.View => "View", + DatabaseNodeType.Index => "Index", + DatabaseNodeType.Trigger => "Trigger", + DatabaseNodeType.Sequence => "Sequence", + DatabaseNodeType.Routine => isFunction ? "Function" : "Procedure", + _ => "Table" + }; + + return (family, kind) switch + { + ("Confirm.Drop.Headline", "Table") => "Confirm.Drop.Headline.Table", + ("Confirm.Drop.Headline", "View") => "Confirm.Drop.Headline.View", + ("Confirm.Drop.Headline", "Index") => "Confirm.Drop.Headline.Index", + ("Confirm.Drop.Headline", "Trigger") => "Confirm.Drop.Headline.Trigger", + ("Confirm.Drop.Headline", "Sequence") => "Confirm.Drop.Headline.Sequence", + ("Confirm.Drop.Headline", "Function") => "Confirm.Drop.Headline.Function", + ("Confirm.Drop.Headline", "Procedure") => "Confirm.Drop.Headline.Procedure", + + ("Explorer.Dropped", "Table") => "Explorer.Dropped.Table", + ("Explorer.Dropped", "View") => "Explorer.Dropped.View", + ("Explorer.Dropped", "Index") => "Explorer.Dropped.Index", + ("Explorer.Dropped", "Trigger") => "Explorer.Dropped.Trigger", + ("Explorer.Dropped", "Sequence") => "Explorer.Dropped.Sequence", + ("Explorer.Dropped", "Function") => "Explorer.Dropped.Function", + ("Explorer.Dropped", "Procedure") => "Explorer.Dropped.Procedure", + + ("Explorer.DropFailed", "Table") => "Explorer.DropFailed.Table", + ("Explorer.DropFailed", "View") => "Explorer.DropFailed.View", + ("Explorer.DropFailed", "Index") => "Explorer.DropFailed.Index", + ("Explorer.DropFailed", "Trigger") => "Explorer.DropFailed.Trigger", + ("Explorer.DropFailed", "Sequence") => "Explorer.DropFailed.Sequence", + ("Explorer.DropFailed", "Function") => "Explorer.DropFailed.Function", + ("Explorer.DropFailed", "Procedure") => "Explorer.DropFailed.Procedure", + + _ => family + ".Table" + }; + } /// /// What breaks if this object goes, in the user's language (WS-20). @@ -819,6 +850,9 @@ public async Task RefreshAsync(IDatabaseSession session) node.Detail = routine.IsFunction ? $"function -> {routine.DataType}" : "procedure"; + + // Which of the two it is, kept as a fact rather than read back out of the label. + node.IsFunction = routine.IsFunction; node.ChildrenLoaded = true; } @@ -854,11 +888,17 @@ public async Task RefreshAsync(IDatabaseSession session) Localization.Plural("Count.Views", views.Count), Localization.Plural("Count.Indexes", indexes.Count), Localization.Plural("Count.Triggers", triggers.Count), - Localization.Plural("Count.Sequences", sequences.Count)); + Localization.Plural("Count.Sequences", sequences.Count), + + // The sixth folder counts here too. It did not until 3.1.2: the tree drew six + // folders and this line named five, so a database whose only objects were + // routines was summarised as having nothing in it. + Localization.Plural("Count.Routines", routines.Count)); Logger.LogInformation( - "Explorer refreshed {Connection}: {Tables} tables, {Views} views, {Indexes} indexes, {Triggers} triggers, {Sequences} sequences", - session.DisplayName, tables.Count, views.Count, indexes.Count, triggers.Count, sequences.Count); + "Explorer refreshed {Connection}: {Tables} tables, {Views} views, {Indexes} indexes, {Triggers} triggers, {Sequences} sequences, {Routines} routines", + session.DisplayName, tables.Count, views.Count, indexes.Count, triggers.Count, sequences.Count, + routines.Count); } catch (Exception ex) { @@ -1249,16 +1289,22 @@ or DatabaseNodeType.SequencesFolder or DatabaseNodeType.View or DatabaseNodeType.Index; + // A ROUTINE is here since 3.1.2, and so is it in ShowsDrop below. The tree gained the + // sixth folder in WS-21 and nothing else did: a function offered one item, Refresh, + // while the engine has DROP FUNCTION and the catalogue already holds the body the + // inspector shows (KnownIssues 27). ShowsViewDefinition = nodeType is DatabaseNodeType.Table or DatabaseNodeType.View or DatabaseNodeType.Trigger - or DatabaseNodeType.Index; + or DatabaseNodeType.Index + or DatabaseNodeType.Routine; ShowsDrop = nodeType is DatabaseNodeType.Table or DatabaseNodeType.View or DatabaseNodeType.Index or DatabaseNodeType.Trigger - or DatabaseNodeType.Sequence; + or DatabaseNodeType.Sequence + or DatabaseNodeType.Routine; // A separator is a rule BETWEEN two groups, so it is drawn only when both sides of it have // something. The items above each learned to hide themselves and the five separators did @@ -1289,12 +1335,14 @@ or DatabaseNodeType.View CanViewDefinition = connected && nodeType is DatabaseNodeType.Table or DatabaseNodeType.View or DatabaseNodeType.Trigger - or DatabaseNodeType.Index; + or DatabaseNodeType.Index + or DatabaseNodeType.Routine; CanDropObject = connected && nodeType is DatabaseNodeType.Table or DatabaseNodeType.View or DatabaseNodeType.Index or DatabaseNodeType.Trigger - or DatabaseNodeType.Sequence; + or DatabaseNodeType.Sequence + or DatabaseNodeType.Routine; // Only a table. ALTER VIEW, ALTER INDEX and ALTER TRIGGER do not exist in this language, so // F2 on any of those would be a button that cannot work.