microsoft / sqltoolsservice

SQL Tools API service that provides SQL Server data management capabilities.
Other
439 stars 146 forks source link

Plans to open source Microsoft.SqlServer.Management.SqlParser.dll #623

Open clement911 opened 6 years ago

clement911 commented 6 years ago

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?

clement911 commented 6 years ago

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. If it was properly open sourced, it would give us confidence that this library is not going to be removed under us!

kburtram commented 6 years ago

@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.

clement911 commented 6 years ago

I see. Well I hope that the relevant team can get the ball rolling in this direction. I think it would open up a number of very interesting scenario that would empower the sql server community. For us, we want to pre-process our scripts with macros and also format consistently but I imagine there would be other cool scenarios such as linters and code analyzers.

clement911 commented 6 years ago

There should be a redist you can use Which redist do you mean? The dll in the sql server folder?

jzabroski commented 6 years ago

@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

clement911 commented 6 years ago

Great to hear. Here is my user voice entry if more people would like to vote: https://feedback.azure.com/forums/908035-sql-server/suggestions/34741105-open-source-microsoft-sqlserver-sqlparser-dll-re

clement911 commented 6 years ago

@jzabroski I found a few bugs in the parser from Microsoft.SqlServer.SqlParser. OVER clauses and NULLIF are not parsed correctly. Do you know where I can report bugs?

jzabroski commented 6 years ago

@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..

clement911 commented 6 years ago

Thanks @jzabroski , although I'm not really sure what you mean by 'reversing the string'. We're just trying to parse some sql code.

@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...

jzabroski commented 6 years ago

Have you looked at

http://www.sqlparser.com/tsql-sql-parser.php

On Tue, Aug 14, 2018, 7:34 PM Clement Gutel notifications@github.com 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 https://github.com/Microsoft/sqltoolsservice/issues/623#issuecomment-413048833, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbT_VlmBNsnj4DgxPVFkbkAhlg8fQjqks5uQ15qgaJpZM4UPgZd .

kburtram commented 6 years ago

@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).

clement911 commented 6 years ago

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.

clement911 commented 6 years ago

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.

nullif_bug

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.

over_bug

jzabroski commented 6 years ago

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 notifications@github.com 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 https://github.com/Microsoft/sqltoolsservice/issues/623#issuecomment-413077311, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbT_QMLZDlDUXgiVHnLyIbxaCj8YdR_ks5uQ4rIgaJpZM4UPgZd .

clement911 commented 6 years ago

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...

clement911 commented 6 years ago

I just came across another parser.

https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2010/dd194286(v=vs.100)

This just gives me hope. I will investigate further but for now I'm confused. What is the relationship between this project and Microsoft.SqlServer.SqlParser?

kburtram commented 6 years ago

@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?

clement911 commented 6 years ago

Good news! Microsoft.SqlServer.TransactSql.ScriptDom.TsqlParser is able to parse those test cases I posted above! This is looking very promising for us. The API is different but at least the AST covers the full syntax.

@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. https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/

FlorianGrimm commented 5 years ago

Is it possible to publish your ANTLR definition? I'm using currently ASP.NET Core with .NET Framework. And for ASP.Net Core 3.0 ....

clement911 commented 5 years ago

Hi @FlorianGrimm , Could you share your scenario? What are you trying to achieve?

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.

FlorianGrimm commented 5 years ago

I currently testing Microsoft.SqlServer.TransactSql.ScriptDom.TsqlParser, but it doesn't have a a .Net Core assembly. I saw the ANTLR classes in it. ANTLR.org has a grammer file, but I don't know how much close it is. I have to do some type analysis. Basically I want to know if i call a SQL Statement what will the result look like Name and Type - for more than one SELECT Statement. No GUI no external exe. It tried some libraries. Even for "simple" things I want behave like the server e.g. nested comments / \n--/ \n */. I guess I saw 3 different SQL Parser from Microsoft. TsqlParser works well, but it's only available for .Net 4.x. The ANTRL 4 runtime now has a return value for visitors which can be really useful - IMHO. That's why I asked for the grammer files

clement911 commented 5 years ago

Got it. I see your points. It would be great if the team could update Microsoft.SqlServer.TransactSql.ScriptDom.TsqlParser to be available on .net core. Asp.net core 3.0 won't be available on .net so we will definitely have to upgrade to .net core and we do use TsqlParser... It's frustrating, there seems to be no way to reach the relevant teams...

With respect to your scenario have you considered calling this stored proc: https://docs.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-describe-first-result-set-transact-sql?view=sql-server-2017

MarcusKohnert commented 5 years ago

@FlorianGrimm This pre-release package is working well for our scenarios on .NET Core.

clement911 commented 5 years ago

@MarcusKohnert , is there a difference between this package and https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ ?

MarcusKohnert commented 5 years ago

@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. ;)

srutzky commented 5 years ago

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

dmarkle commented 5 years ago

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?

clement911 commented 5 years ago

Have you considered using https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/?

shueybubbles commented 5 years ago

the sqlparser is part of Microsoft.SqlServer.SqlManagementObjects

Get Outlook for Androidhttps://aka.ms/ghei36


From: Clement Gutel notifications@github.com Sent: Thursday, May 23, 2019 7:51:00 AM To: microsoft/sqltoolsservice Cc: David Shiflet; Mention Subject: Re: [microsoft/sqltoolsservice] Plans to open source Microsoft.SqlServer.Management.SqlParser.dll (#623)

Have you considered using https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nuget.org%2Fpackages%2FMicrosoft.SqlServer.TransactSql.ScriptDom%2F&data=02%7C01%7C%7C1c9a5d1af464488836bb08d6df74ed33%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636942090611834021&sdata=BuRCbFP%2FxyyLbEGKOJ9UEt9gDttYOTrNAOv9mKYf1a4%3D&reserved=0?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fsqltoolsservice%2Fissues%2F623%3Femail_source%3Dnotifications%26email_token%3DAAQ7GCVQBUQ7E3KJC6BZAKTPW2AKJA5CNFSM4FB6AZO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWB653I%23issuecomment-495185645&data=02%7C01%7C%7C1c9a5d1af464488836bb08d6df74ed33%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636942090611844029&sdata=ku%2BXd2RgmnIXhooIrM6tlf4SaWI81XmZkZEngXy5o8M%3D&reserved=0, or mute the threadhttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAQ7GCW3XDRFZCGQGBPOZH3PW2AKJANCNFSM4FB6AZOQ&data=02%7C01%7C%7C1c9a5d1af464488836bb08d6df74ed33%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636942090611844029&sdata=oWMWYDc478awx7RKt2JwLK6vxVTdilrYTWH1TBMK%2FS8%3D&reserved=0.

dmarkle commented 5 years ago

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)

clement911 commented 5 years ago

Are there plans to port https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ to .net core?

ststeiger commented 4 years ago

Anybody that needs an ANTLR4 version for .NET Core:

https://github.com/ststeiger/SqlParser

clement911 commented 4 years ago

Checking back again to see if Microsoft.SqlServer.TransactSql.ScriptDom might be ported to .Net core? We want to migrate to .net core but can't because of this dependency.

ststeiger commented 4 years ago

Both ScriptDom and SqlParser are ported to netstandard 2.0, which you can run on .NET Core.

Just download SQL-Ops Studio (latest-version) there you find it in subdirectory resources\app\extensions\mssql\sqltoolsservice\Windows\<VERSION>

But it uses ANTLR behind the scenes, so you might as well use ANTLR yourselfs. It would be great if they published the full T-SQL antlr grammar file for each version, though.

If you need to make any fixes, you can download ILSpy v5, decompile the netstandard dll, and with 5 small fixes it compiles again. Then you can fix the bugs yourselfs.

But again, instead of that, just download the ANTLR 4 T-SQL grammar, and create your own ANTLR parser. Fix the grammar file, if you need better T-SQL support.

clement911 commented 4 years ago

As far as I can see the latest ScriptDom only target .net framework. See https://www.fuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ Am I missing something?

With respect to the correct grammar, are you speaking of SqlParser specifically ? It is my understanding that Microsoft.SqlServer.TransactSql.ScriptDom is maintained by the sql server team and is already a full-fidelity parser.

clement911 commented 4 years ago

See below. Only one target framework moniker, for net40

image

ststeiger commented 4 years ago

NetStandard 2.0 Show NetStandard 2.0

Build output Show build output

.NET Core 2.1 Show .NET Core

And it's running: ScriptDom on NetCore

ststeiger commented 4 years ago

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 resources\app\extensions\mssql\sqltoolsservice\Windows\<VERSION>

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.

clement911 commented 4 years ago

Oh I see. Great. Are there any plans to publish this Net standard dll in Nuget, or update the existing nuget package to use this dll? It would mean we can easily update to the latest version moving forward, as opposed to referencing the dll manually.

shueybubbles commented 4 years ago

The Microsoft.SqlServer.DacFx nuget package has an appropriate license for ScriptDom, which it includes.

clement911 commented 4 years ago

@shueybubbles Microsoft.SqlServer.DacFx target Net framework though. I want to use .net core.

ststeiger commented 4 years ago

@clement911: I have no idea what license Microsoft.SqlServer.TransactSql.ScriptDom comes with, so I'd rather not publish it.

clement911 commented 4 years ago

Referencing the dll from Azure Data Studio works. Great! Hopefully it can be packaged in nuget.

clement911 commented 4 years ago

Me neither. I guess if the right people at Microsoft could get together maybe they could sort this out. At least there is a work around for now.

shueybubbles commented 4 years ago

The SqlToolsService used by ADS is itself open source. It gets all its dependencies from nuget. Checking its PackageReference (see https://github.com/microsoft/sqltoolsservice/blob/master/src/Microsoft.SqlTools.ServiceLayer/Microsoft.SqlTools.ServiceLayer.csproj) for DacFx, we find:

<PackageReference Include="Microsoft.SqlServer.DacFx" Version="150.4534.2-preview" />

Installing that package, we find the following netstandard2.0 DLLs:

 Directory of C:\Users\username\.nuget\packages\microsoft.sqlserver.dacfx\150.4534.2-preview\lib\netstandard2.0

09/11/2019  02:09 PM    <DIR>          .
09/11/2019  02:09 PM    <DIR>          ..
09/05/2019  09:05 PM        11,212,920 Microsoft.Data.Tools.Schema.Sql.dll
09/05/2019  09:04 PM           257,144 Microsoft.Data.Tools.Utilities.dll
09/05/2019  09:04 PM           329,848 Microsoft.SqlServer.Dac.dll
09/05/2019  09:04 PM           752,760 Microsoft.SqlServer.Dac.Extensions.dll
09/05/2019  09:06 PM         4,559,128 Microsoft.SqlServer.TransactSql.ScriptDom.dll
09/05/2019  09:07 PM            22,296 Microsoft.SqlServer.Types.dll
jzabroski commented 4 years ago

Thanks, David Shiflet! @shueybubbles

ststeiger commented 4 years ago

What do you use Dac for ?

Anybody knows what Microsoft.Data.Tools.Schema.Sql.dll and Microsoft.Data.Tools.Utilities.dll can do ?

clement911 commented 4 years ago

@shueybubbles oh I see, so it's a preview package. I was looking at Microsoft.SqlServer.DacFx.x64...

I don't want to be a pain here, but in our project, we only need Microsoft.SqlServer.TransactSql.ScriptDom.dll The package Microsoft.SqlServer.DacFx has a LOT of dependencies and we'd love to see small package for Microsoft.SqlServer.TransactSql.ScriptDom.dll

sherland commented 4 years ago

This is rather anoying ;-) https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ should be netstandard compatilbe, and Microsoft.SqlServer.DacFx should have a dependency on the nuget, instead of embedding its own copy. As an example: Which dll will be used in projects that reference both Microsoft.SqlServer.TransactSql.ScriptDom (could be indirecty) and Microsoft.SqlServer.DacFx?

kevcunnane commented 4 years ago

@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.