-
Notifications
You must be signed in to change notification settings - Fork 246
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
throw exception when the statement missing the semicolon as splitter #159
Comments
Sorry for the late reply. This is both expected and unexpected. It's expected because as you can see from Text View in UI (or toggle -v for verbose option in command line), this SQL is recognized as 2 statements SQL. The first one clearly is syntactically wrong. But because we're using sqlparse as parser, which according to its homepage, is a "non-validating SQL parser". In other words, we don't validate against the SQL syntax, we just try our best to parse it. This approach has its advantage and disadvantage. The disadvantage is obvious, we had this issue coming. On the other side, talking about the good part, personally I never used MSSQL before, so I'm amazed to see SQL like And it's unexpected because as a user, I'd like the library to throw me an error, or at least give me some warnings in case like this, as opposed to just showing this confusing graph, which leads me nowhere for debugging. Of course it's user's responsibility to get the syntax right, whilst it's library's responsibility to at least warn the user about this. Although I have no clue at this point how we're going to detect potential syntax error so as to warn user, let's leave this ticket open for future reference. |
After releasing v1.4.0 next month, we shall be able to use --dialect=tsql in command line, or in python LineageRunner(sql, dialect="tsql") to leverage our new parsing capability. sqllineage will throw a InvalidSyntaxException for most syntax error. However, this missing semicolon issue is not well handled yet, because it will be parsed as Batches of SQL Statements (https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/batches-of-sql-statements?view=sql-server-ver16) |
We will close this issue as we already support throwing exception generally when semicolon is missing. With tsql, I'm surprised to understand that semicolon is actually not a must-have. Currently after closing #422 , we trigger a SyntaxWarning, letting people know that SQLLineage is not guaranteed to output correct result with tsql when semicolon is missing. We'll try supporting that feature in #384 . |
I'm confused if the result is expected:
If I add a semicolon between the first two statement, the graph output is normal.
The text was updated successfully, but these errors were encountered: