-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Plans to open source Microsoft.SqlServer.Management.SqlParser.dll #623
Comments
Not going to happen? We use the parser in our app but it's always scary to grab a dll from the GAC and just copy it to our app. |
@clement911 SQLParser is the primary components used for SSMS, SQL Ops Studio and vscode-mssql language services. It is safe to assume the parser will continue to be maintained for the foreseeable future. There should be a redist you can use which is the recommend way to install this component. The SQL Parser component is currently build as part of the SSMS internal repository. SSMS (and therefore SQL Parser) is built using a custom build environment that we'd need to update to a more standard environment prior to open sourcing. Also, there are various internal procedures we need to figure out, such as how best to handle vNext changes prior to public disclosure, etc. We have backlog items for open-sourcing Parser, SMO, DacFx and other tools components, but there are quite a bit of details we need to work through prior to publishing these components that weren't originally developed as OSS. Unfortunately, I don't have an ETA for this work. |
I see. |
|
@clement911 Microsoft has started publishing the SqlParser as a nuget package, which is a step in the right direction: https://www.nuget.org/packages/Microsoft.SqlServer.SqlParser/140.17279.0-xplat |
Great to hear. |
@jzabroski I found a few bugs in the parser from Microsoft.SqlServer.SqlParser. |
@kburtram works for Microsoft and would be better to answer that. Unless it's open source I doubt these issues get resolved. I anecdotally believe a lot of bugs have crept into SqlParser over the last 3 releases. I have done very odd things where I reverse the string manually in the text, assign it to @sqlcommand, reverse it again in memory in TSQL, and execute it using EXEC (@sqlcommand). Then it executes fine. Most of the issues have to do with privileged commands like sp_dropuser. I posted about it on StackOverflow. Maybe try the same trick.. |
Thanks @jzabroski , although I'm not really sure what you mean by 'reversing the string'. @kburtram I'm calling on to you again then :) If this is a dead end, are there any other TSQL parser out there you know of? We need a parser capable of producing a full fidelity AST... |
Have you looked at
http://www.sqlparser.com/tsql-sql-parser.php
…On Tue, Aug 14, 2018, 7:34 PM Clement Gutel ***@***.***> wrote:
Thanks @jzabroski <https://github.com/jzabroski> , although I'm not
really sure what you mean by 'reversing the string'.
We're just trying to parse some sql code.
@kburtram <https://github.com/kburtram> I'm calling on to you again then
:)
We're taking a pretty big dependency on the sql parser nuget package.
We're really glad it is now on nuget!
We are relying on the parser to provide us with an AST that we can visit
to inspect various things about
our TSQL code and even change on the fly via pseudo macros.
It is very powerful and pretty cool but the problem is that the AST is
often not full fidelity to the code given. It seems that certain code paths
are not fully parsed, such as OVER bodies, NULLIF parameters, and more.
Is there anything we can do to facilitate the open sourcing a parser?
I'm sure it would open up some really cool possibilities such as linters,
analyzer, editors, macros, etc...
It's pretty frustrating because it looks like the parsing works 95% great,
but there are just a few issues.
If this is a dead end, are there any other TSQL parser out there you know
of? We need a parser capable of producing a full fidelity AST...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#623 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbT_VlmBNsnj4DgxPVFkbkAhlg8fQjqks5uQ15qgaJpZM4UPgZd>
.
|
@clement911 I don't think we have plans to open source the parser within the next few months. We'd probably want to do that as part of opening a larger body of code, such as SMO, which will take a while. @shueybubbles is the owner of the Parser Nuget package (and SMO) and may have additional context. If you provide example SQL that the parser isn't correctly handling I can open an internal bug against the SQL Parser component. In the near-term it's possible something like the parser that @jzabroski mentioned may work better for you. Though we certainly would want to fix any significant bugs in our language service SQL parser (and eventually open-up that codebase). |
I haven't looked at that other parser no. If we get desperate we may have to look at it... I will get some minimal bug repros and post them here shortly. |
So here are the 2 bugs I found so far. Bug 1) Query 1: SELECT NULLIF([col], 0) FROM [Table] The queries are identical except that one is using NULLIF and the other is using ISNULL. Bug 2) The OVER() clause seems to be swallowed up in the SUM aggregate function call and is not present at all in the AST. |
Anecdotally, it used to be that Microsoft's SQL Parser was very bad and the
one I linked to was better. I used it in a demo years ago, on SQL Server
2005 and 2008, because it did a MUCH better job finding missing rows in
sys.sql_dependencies and sys.expression_dependencies. At that same time,
ApexSQL published whitepapers explaining their parser bundled with ApexSQL
Complete was better at finding (broken) dependencies than Microsoft's
provided views. Most of the edge ApexSQL had is now gone, and I would
expect that GSP T-SQL Parser's edge to also go away.
There is one remaining issue I'm aware of: Tracking of temp tables and
common table expression identifiers is still super crappy.
…On Tue, Aug 14, 2018 at 10:43 PM Clement Gutel ***@***.***> wrote:
So here are the 2 bugs I found so far.
I parse and inspect the AST with Parser.Parse(sqlQuery).Script.Xml
Bug 1)
Compare the two ASTs for the two following queries.
Query 1: SELECT NULLIF([col], 0) FROM [Table]
Query 2: SELECT ISNULL([col], 0) FROM [Table]
The queries are identical except that one is using NULLIF and the other is
using ISNULL.
Yet, the AST of the query with ISNULL includes the parameters within the
function call whereas the AST of the query with NULLIF doesn't.
[image: nullif_bug]
<https://user-images.githubusercontent.com/3426504/44128702-b826be5a-a087-11e8-9376-d33038cb5a93.PNG>
Bug 2)
The OVER() clause seems to be swallowed up in the SUM aggregate function
call and is not present at all in the AST.
[image: over_bug]
<https://user-images.githubusercontent.com/3426504/44128713-c1e0dcf0-a087-11e8-87be-14fd1dec5509.PNG>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#623 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbT_QMLZDlDUXgiVHnLyIbxaCj8YdR_ks5uQ4rIgaJpZM4UPgZd>
.
|
Of course we know that there is at least one parser that is comprehensive, and that is the parser used by Sql Server itself. If only that was available... On the programming language side, the situation has become ideal, with C# and Typescript both coming with a language service as a first class citizen, which is powered by the same parser that the compilers use. The tooling has become much better thanks to that... |
I just came across another parser. https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ This just gives me hope. |
@clement911 we use Microsoft.SqlServer.Management.SqlParser for our language service, so it's optimized around those types of tasks (e.g., IntelliSense suggestions, QuickInfo tooltips, diagnostic errors). I think ScriptDom is more around working with the AST, which may work better for your scenario. You may also need to bring in DacFx to work with ScriptDom, and it's not available on .Net Core AFAIK. @kevcunnane do you have additional context on ScriptDom vs. SqlParser? |
Good news! @kburtram I see what you're saying, although it seems sharing a single parser would be worthwhile given the complexity of the task... FYI it is also available as a nuget package and the only dependency is .Net 4.0. |
Is it possible to publish your ANTLR definition? |
Hi @FlorianGrimm , I'm not sure if you checked out Microsoft.SqlServer.TransactSql.ScriptDom.TsqlParser but for us it worked much better. As I understand it, it is the same parser as the one used by Sql server itself, but exposed as a .net dll. |
I currently testing Microsoft.SqlServer.TransactSql.ScriptDom.TsqlParser, but it doesn't have a a .Net Core assembly. |
Got it. With respect to your scenario have you considered calling this stored proc: |
@FlorianGrimm This pre-release package is working well for our scenarios on .NET Core. |
@MarcusKohnert , is there a difference between this package and https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ ? |
@clement911 I don't know your referenced package so I don't know if there are any differences. Microsoft.SqlServer.SqlParser/150.18068.0-xplat is packaged by Microsoft at least which is a plus for me. ;) |
Just FYI: Another parsing bug that is still affecting SSMS (and likely also SQLCMD) is related to the odd (and probably not well known) ability of SQL Server to allow nested block comments. Splitting up batches based on the GO separator has issues with nested block comments, which I detailed in the following ticket: "GO" in 2nd half of nested block comments breaks batch parsing in SSMS and SQLCMD |
Ugh, the pre-release package has been unlisted from nuget: "The owner has unlisted this package. This could mean that the package is deprecated or shouldn't be used anymore." Does anyone see if there is an update, maybe if MS is shipping this somewhere else now? |
Have you considered using https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/? |
Oh. That's awesome. I've been waiting for that to go on Nuget forever. It's like a dream come true, and SqlParser from SMO works great (except SMO doesn't really like installing on my dev workstation with tons of other assemblies already installed). But I can get around that.... Next thing you know, they'll be porting SQL AMO to .Net Core (ha) |
What you're doing wrong is taking the DLL from nuget, instead of taking the one that AzureDataStudio uses. (AzureDataStudio runs on Linux) https://github.com/microsoft/azuredatastudio Download the zip archive, and get the NetStandard2.0 dlls from subfolder With respect to "correct grammar", you only need the grammar file if you build your own parser with ANTLR. If you use the SqlParser provided by MS, you don't need any grammar file at all - the corresponding code has already been created and is part of ScriptDom/SqlParser. |
Oh I see. Great. |
The Microsoft.SqlServer.DacFx nuget package has an appropriate license for ScriptDom, which it includes. |
@shueybubbles Microsoft.SqlServer.DacFx target Net framework though. I want to use .net core. |
@clement911: I have no idea what license Microsoft.SqlServer.TransactSql.ScriptDom comes with, so I'd rather not publish it. |
Referencing the dll from Azure Data Studio works. Great! |
Me neither. I guess if the right people at Microsoft could get together maybe they could sort this out. |
The SqlToolsService used by ADS is itself open source. It gets all its dependencies from nuget.
Installing that package, we find the following netstandard2.0 DLLs:
|
Thanks, David Shiflet! @shueybubbles |
What do you use Dac for ? Anybody knows what Microsoft.Data.Tools.Schema.Sql.dll and Microsoft.Data.Tools.Utilities.dll can do ? |
@shueybubbles oh I see, so it's a preview package. I don't want to be a pain here, but in our project, we only need Microsoft.SqlServer.TransactSql.ScriptDom.dll |
This is rather anoying ;-) |
@sherland that's not an official Microsoft package. The only official, NetStandard-compatible way to get ScriptDom is to reference the DacFx nuget package. I hear you on it being inconvenient to pull in all the DLLs if you just need ScriptDom - @shueybubbles or @pensivebrian can likely comment on priority of separating it into its own package. |
@duncansmart The "Microsoft." prefix is supposed to be reserved for Microsoft projects. On top of that, " |
@jzabroski I've been getting emails from people asking for a .NET Core version of that package and people here seem to think it's an official release. I added it years before the official DacFx packages were added. Naming it after the assembly it packaged seemed sensible in allowing people to be able to find it, indeed this was pretty common practice in the early days of NuGet when some parts of Microsoft were slow in releasing official NuGet packages. |
@duncansmart Either way, your naming of the NuGet package isn't desirable today given Nuget package prefix id's, specified here: https://github.com/NuGet/Home/wiki/NuGet-Package-Identity-Verification#what-does-a-reserved-package-id-prefix-do - This RFC went into effect in 2017. In general, since the 1990s and Java engineers using "reverse domain name" for namespaces, e.g. com.java.*, it's well-established not to use somebody else's company name as a prefix for a code artifact, whether it be an assembly name, package name or namespace. You're in a strange spot right now, because permanently deleting a package is not supported by Nuget, yet that would be the right thing to do. |
Any progress? |
Coming back to the original question. Still no progress on open sourcing the parser? |
Any progress on open sourcing the parser Microsoft.SqlServer.Management.SqlParser? |
Just a guess: $$$ :) |
This would probably be an investment and cost initially but it would probably pay off in the long term. |
The VS SQL Analyzer uses an modified / improved version. |
This thread seems to be the closest thing we have to a place to report bugs in I just ran across an annoying problem in the current NuGet build: calls to |
@masonwheeler Could you please open a new issue? This repo is good enough for now, but finding specific issues buried in unrelated threads is difficult 😄 |
@Charles-Gagnon Done |
Apologies if this is the wrong place to ask, but is it possible to also open source Microsoft.SqlServer.Management.SqlParser.dll ?
It would seem like the right repo for it?
The text was updated successfully, but these errors were encountered: