Fixes attempt to scaffold triggers on synapse re: issue #30998 #31001
Conversation
|
@dotnet-policy-service agree |
|
I can't make a reasonable test case as testing this would require an accessible and queryable synapse instance. Afaik no such publicly accessible test instance exists. |
| @@ -1343,6 +1347,9 @@ private bool SupportsMemoryOptimizedTable() | |||
| private bool SupportsSequences() | |||
| => _compatibilityLevel >= 110 && _engineEdition != 6; | |||
|
|
|||
| private bool SupportsTriggers() | |||
There was a problem hiding this comment.
You should add 11 and 1000 as well.
There was a problem hiding this comment.
Looking at the other Supports methods, is it correct to say that Synapse can be detected by checking if _engineEdition is 6, 11 or 1000? If so, we should extract an IsSynapse private property that does that and reuse that everywhere...
There was a problem hiding this comment.
11 and 1000 are not Synapse Serverless Pool, on 6 is. 11 is Data Warehouse, and 1000 is Dynamics CRM TDS endpoint
There was a problem hiding this comment.
OK - I still think it would make the code more readable to have at least constants for this to make the code more readable.
There was a problem hiding this comment.
Agree that constants would improved readability/understanding
There was a problem hiding this comment.
There was a problem hiding this comment.
Great, we should probably just have an EngineEdition enum...
There was a problem hiding this comment.
Possible Enum names (from some Microsoft code base): https://github.com/microsoft/vscode-mssql/blob/59a921a03d1564ac8441f460e7374c15d3bbbf47/typings/vscode-mssql.d.ts#L221
There was a problem hiding this comment.
I see in the docs that 11 is synapse serverless pool, but what is 1000?
There was a problem hiding this comment.
1000 is the second link: Dynamics CRM TDS Endpoint
There was a problem hiding this comment.
@memory-thrasher can you please target the main branch? We'll discuss whether to backport the fix later.
| @@ -1343,6 +1347,9 @@ private bool SupportsMemoryOptimizedTable() | |||
| private bool SupportsSequences() | |||
| => _compatibilityLevel >= 110 && _engineEdition != 6; | |||
|
|
|||
| private bool SupportsTriggers() | |||
There was a problem hiding this comment.
Looking at the other Supports methods, is it correct to say that Synapse can be detected by checking if _engineEdition is 6, 11 or 1000? If so, we should extract an IsSynapse private property that does that and reuse that everywhere...
There was a problem hiding this comment.
@memory-thrasher can you please target the main branch? We'll discuss whether to backport the fix later.
|
I can't retarget this PR so I'll make a new one to target main once the test cases finish. |
No description provided.