Use Microsoft-Produced SQL Parser for SQL Server #455
Conversation
The "Microsoft.SqlServer.DACFx" package doesn't support netstandard1.3 or net35 so we will upgrade.
Use the Microsoft "TSql150Parser" class to parse and split the script content. This commit also duplicates the splitter tests. The tests are duplicated so they can be modified because some of the existing SqlCommandSplitterTests content isn't valid SQL so isn't supported by the parser.
The API approval test was giving me different results than the approved build so extra ordering was added to improve the situation around generating a good file for comparison.
| <PropertyGroup Condition=" '$(TargetFramework)' == 'net35' "> | ||
| <PropertyGroup Condition=" '$(TargetFramework)' == 'net46' "> | ||
| <DefineConstants>$(DefineConstants);SUPPORTS_SQL_CONTEXT</DefineConstants> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
I'm not sure if net46 is actually valid for SUPPORTS_SQL_CONTEXT to be set, but I was just changing the net35 straight to net46.
|
|
||
| public TSqlCommandSplitter() | ||
| { | ||
| parser = new TSql150Parser(true, SqlEngineType.All); |
There was a problem hiding this comment.
Maybe move these options to WithNewSqlParser?
There was a problem hiding this comment.
Great suggestion. Will definitely expose this somewhere.
|
|
||
| if (errors.Count > 0) | ||
| { | ||
| throw new TSqlParseException($"Failure parsing SQL script \"{errors[0].Message}\" at [{errors[0].Line}, {errors[0].Column}]"); |
There was a problem hiding this comment.
I was lazy & wanted to get the first iteration done. I definitely plan to go back and build out that exception to properly handle a list of errors.
| @@ -125,7 +125,7 @@ void Append(object argumentValue, string argumentName = null) | |||
|
|
|||
| void AppendTypes(StringBuilder sb, int indent, IEnumerable<Type> types) | |||
| { | |||
| foreach (var type in types) | |||
| foreach (var type in types.OrderBy(t => t.Name)) | |||
There was a problem hiding this comment.
It’s worth adding ordinal sorting inside OrderBy so that the results do not differ on different OS.
There was a problem hiding this comment.
Absolutely. Although in the harsh light of the morning I'll probably rebase these changes out and consider submitting them separately. They have nothing to do with parsing SQL.
|
@AdrianJSClark Please rebase onto |
sungam3r
added
dbup-sqlserver
(TO DO - write this summary properly)
Microsoft.SqlServer.DacFxpackage to parse script contents to split to commandsDacFxpackage)SqlCommandSplittertests aren't valid SQLResolves DbUp/dbup-sqlserver#4