From 1ca1334991095b9f3c13dee866e1554adaa10b3e Mon Sep 17 00:00:00 2001 From: Leila Lali Date: Mon, 20 Jul 2026 13:37:21 -0700 Subject: [PATCH] Make MODEL_TYPE fragment classification non-breaking Commit 20f9bf1 (PR #210, fixing #176) reclassified MODEL_TYPE as a TSqlFragment but broke the public API: it repurposed the enum ExternalModelTypeOption into a fragment class and changed the type of ExternalModelStatement.ModelType from ExternalModelTypeOption? to a fragment reference. This change delivers the same fix additively, without breaking the API: - Keep enum ExternalModelTypeOption { EMBEDDINGS } and the ExternalModelStatement.ModelType (ExternalModelTypeOption?) member unchanged for backward compatibility. - Add a visitable fragment ExternalModelTypeSpecification whose OptionKind reuses the existing ExternalModelTypeOption enum, plus a new ModelTypeSpecification member on ExternalModelStatement. - The parser populates both representations; the script generator prefers the fragment and falls back to the legacy enum. All 624 unit tests pass on net8.0 and net472. --- SqlScriptDom/Parser/TSql/Ast.xml | 7 +++--- .../Parser/TSql/ExternalModelTypeOption.cs | 9 ++++---- SqlScriptDom/Parser/TSql/TSql170.g | 15 ++++++------ SqlScriptDom/Parser/TSql/TSql180.g | 15 ++++++------ ...ptGenerator.AlterExternalModelStatement.cs | 15 ++++++++++-- ...torVisitor.CreateExternalModelStatement.cs | 23 ++++++++++++++----- Test/SqlDom/Only170SyntaxTests.cs | 16 +++++++++---- 7 files changed, 65 insertions(+), 35 deletions(-) diff --git a/SqlScriptDom/Parser/TSql/Ast.xml b/SqlScriptDom/Parser/TSql/Ast.xml index d90c664a..ebf274a3 100644 --- a/SqlScriptDom/Parser/TSql/Ast.xml +++ b/SqlScriptDom/Parser/TSql/Ast.xml @@ -1388,7 +1388,8 @@ - + + @@ -1404,8 +1405,8 @@ - - + + diff --git a/SqlScriptDom/Parser/TSql/ExternalModelTypeOption.cs b/SqlScriptDom/Parser/TSql/ExternalModelTypeOption.cs index 6f16dc2a..0a7bfdac 100644 --- a/SqlScriptDom/Parser/TSql/ExternalModelTypeOption.cs +++ b/SqlScriptDom/Parser/TSql/ExternalModelTypeOption.cs @@ -10,16 +10,15 @@ namespace Microsoft.SqlServer.TransactSql.ScriptDom #pragma warning disable 1591 /// - /// The enumeration specifies the external model type - /// Currently, we support Embeddings only. + /// The enumeration specifies the external model type. + /// Currently, we support EMBEDDINGS only. /// - public enum ExternalModelTypeOptionKind + public enum ExternalModelTypeOption { /// /// MODEL_TYPE = EMBEDDINGS /// - Embeddings = 0, - + EMBEDDINGS = 0, } #pragma warning restore 1591 diff --git a/SqlScriptDom/Parser/TSql/TSql170.g b/SqlScriptDom/Parser/TSql/TSql170.g index 0babede1..ceb78600 100644 --- a/SqlScriptDom/Parser/TSql/TSql170.g +++ b/SqlScriptDom/Parser/TSql/TSql170.g @@ -24171,14 +24171,14 @@ StringLiteral vApiFormat; externalModelModelType[ExternalModelStatement vParent] { - ExternalModelTypeOption vModelTypeOption = null; + ExternalModelTypeSpecification vModelTypeSpec = null; } : tModelType:Identifier { Match(tModelType, CodeGenerationSupporter.ModelType); - vModelTypeOption = this.FragmentFactory.CreateFragment(); - UpdateTokenInfo(vModelTypeOption, tModelType); + vModelTypeSpec = this.FragmentFactory.CreateFragment(); + UpdateTokenInfo(vModelTypeSpec, tModelType); } EqualsSign ( @@ -24186,10 +24186,11 @@ externalModelModelType[ExternalModelStatement vParent] { if (TryMatch(tEmbeddings, CodeGenerationSupporter.Embeddings)) { - vModelTypeOption.OptionKind = ExternalModelTypeOptionKind.Embeddings; - UpdateTokenInfo(vModelTypeOption, tEmbeddings); - vParent.ModelType = vModelTypeOption; - vParent.UpdateTokenInfo(vModelTypeOption); + vModelTypeSpec.OptionKind = ExternalModelTypeOption.EMBEDDINGS; + UpdateTokenInfo(vModelTypeSpec, tEmbeddings); + vParent.ModelTypeSpecification = vModelTypeSpec; + vParent.ModelType = ExternalModelTypeOption.EMBEDDINGS; + vParent.UpdateTokenInfo(vModelTypeSpec); } else { diff --git a/SqlScriptDom/Parser/TSql/TSql180.g b/SqlScriptDom/Parser/TSql/TSql180.g index ae426b05..934911db 100644 --- a/SqlScriptDom/Parser/TSql/TSql180.g +++ b/SqlScriptDom/Parser/TSql/TSql180.g @@ -24400,14 +24400,14 @@ StringLiteral vApiFormat; externalModelModelType[ExternalModelStatement vParent] { - ExternalModelTypeOption vModelTypeOption = null; + ExternalModelTypeSpecification vModelTypeSpec = null; } : tModelType:Identifier { Match(tModelType, CodeGenerationSupporter.ModelType); - vModelTypeOption = this.FragmentFactory.CreateFragment(); - UpdateTokenInfo(vModelTypeOption, tModelType); + vModelTypeSpec = this.FragmentFactory.CreateFragment(); + UpdateTokenInfo(vModelTypeSpec, tModelType); } EqualsSign ( @@ -24415,10 +24415,11 @@ externalModelModelType[ExternalModelStatement vParent] { if (TryMatch(tEmbeddings, CodeGenerationSupporter.Embeddings)) { - vModelTypeOption.OptionKind = ExternalModelTypeOptionKind.Embeddings; - UpdateTokenInfo(vModelTypeOption, tEmbeddings); - vParent.ModelType = vModelTypeOption; - vParent.UpdateTokenInfo(vModelTypeOption); + vModelTypeSpec.OptionKind = ExternalModelTypeOption.EMBEDDINGS; + UpdateTokenInfo(vModelTypeSpec, tEmbeddings); + vParent.ModelTypeSpecification = vModelTypeSpec; + vParent.ModelType = ExternalModelTypeOption.EMBEDDINGS; + vParent.UpdateTokenInfo(vModelTypeSpec); } else { diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGenerator.AlterExternalModelStatement.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGenerator.AlterExternalModelStatement.cs index 10e8a45f..1d8da7ad 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGenerator.AlterExternalModelStatement.cs +++ b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGenerator.AlterExternalModelStatement.cs @@ -53,7 +53,7 @@ protected void GenerateAlterExternalModelStatementBody(AlterExternalModelStateme } // external model Model Type options - if (node.ModelType != null) + if (node.ModelTypeSpecification != null) { if (!ifFirst) { @@ -61,7 +61,18 @@ protected void GenerateAlterExternalModelStatementBody(AlterExternalModelStateme } ifFirst = false; NewLine(); - GenerateFragmentIfNotNull(node.ModelType); + GenerateFragmentIfNotNull(node.ModelTypeSpecification); + } + else if (node.ModelType != null) + { + if (!ifFirst) + { + GenerateSymbol(TSqlTokenType.Comma); + } + ifFirst = false; + NewLine(); + string externalModelTypeOption = GetValueForEnumKey(_externalModelTypeOption, node.ModelType.Value); + GenerateNameEqualsValue(CodeGenerationSupporter.ModelType, externalModelTypeOption); } // external model name options diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CreateExternalModelStatement.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CreateExternalModelStatement.cs index 8d6b3174..543f15a8 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CreateExternalModelStatement.cs +++ b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CreateExternalModelStatement.cs @@ -17,9 +17,9 @@ public override void ExplicitVisit(CreateExternalModelStatement node) GenerateSpaceAndIdentifier(CodeGenerationSupporter.Model); GenerateCreateExternalModelStatementBody(node); } - protected static Dictionary _externalModelTypeOptionKind = new Dictionary() + protected static Dictionary _externalModelTypeOption = new Dictionary() { - {ExternalModelTypeOptionKind.Embeddings, CodeGenerationSupporter.Embeddings} + {ExternalModelTypeOption.EMBEDDINGS, CodeGenerationSupporter.Embeddings} }; protected void GenerateCreateExternalModelStatementBody(CreateExternalModelStatement node) @@ -54,7 +54,7 @@ protected void GenerateCreateExternalModelStatementBody(CreateExternalModelState } // external model Model Type options - if (node.ModelType != null) + if (node.ModelTypeSpecification != null) { if (!ifFirst) { @@ -62,7 +62,18 @@ protected void GenerateCreateExternalModelStatementBody(CreateExternalModelState } ifFirst = false; NewLine(); - GenerateFragmentIfNotNull(node.ModelType); + GenerateFragmentIfNotNull(node.ModelTypeSpecification); + } + else if (node.ModelType != null) + { + if (!ifFirst) + { + GenerateSymbol(TSqlTokenType.Comma); + } + ifFirst = false; + NewLine(); + string externalModelTypeOption = GetValueForEnumKey(_externalModelTypeOption, node.ModelType.Value); + GenerateNameEqualsValue(CodeGenerationSupporter.ModelType, externalModelTypeOption); } // external model name options @@ -117,9 +128,9 @@ protected void GenerateCreateExternalModelStatementBody(CreateExternalModelState GenerateKeyword(TSqlTokenType.RightParenthesis); } - public override void ExplicitVisit(ExternalModelTypeOption node) + public override void ExplicitVisit(ExternalModelTypeSpecification node) { - string optionKindString = GetValueForEnumKey(_externalModelTypeOptionKind, node.OptionKind); + string optionKindString = GetValueForEnumKey(_externalModelTypeOption, node.OptionKind); GenerateNameEqualsValue(CodeGenerationSupporter.ModelType, optionKindString); } } diff --git a/Test/SqlDom/Only170SyntaxTests.cs b/Test/SqlDom/Only170SyntaxTests.cs index 51c95f38..322f993e 100644 --- a/Test/SqlDom/Only170SyntaxTests.cs +++ b/Test/SqlDom/Only170SyntaxTests.cs @@ -407,18 +407,24 @@ public void TestExternalModelModelTypeVisitor() Assert.IsNotNull(fragment); // Collect all visited fragments using a custom visitor - var visitor = new ExternalModelTypeOptionVisitor(); + var visitor = new ExternalModelTypeSpecificationVisitor(); fragment.Accept(visitor); Assert.AreEqual(1, visitor.VisitedOptions.Count); - Assert.AreEqual(ExternalModelTypeOptionKind.Embeddings, visitor.VisitedOptions[0].OptionKind); + Assert.AreEqual(ExternalModelTypeOption.EMBEDDINGS, visitor.VisitedOptions[0].OptionKind); + + // Legacy enum member must still be populated for backward compatibility + var createStatement = ((TSqlScript)fragment).Batches[0].Statements[0] as CreateExternalModelStatement; + Assert.IsNotNull(createStatement); + Assert.AreEqual(ExternalModelTypeOption.EMBEDDINGS, createStatement.ModelType); + Assert.IsNotNull(createStatement.ModelTypeSpecification); } - private class ExternalModelTypeOptionVisitor : TSqlFragmentVisitor + private class ExternalModelTypeSpecificationVisitor : TSqlFragmentVisitor { - public List VisitedOptions { get; } = new List(); + public List VisitedOptions { get; } = new List(); - public override void Visit(ExternalModelTypeOption node) + public override void Visit(ExternalModelTypeSpecification node) { VisitedOptions.Add(node); base.Visit(node);