Fix | Build ReservedWords table in code#4328
Conversation
| <ReservedWord>CURSOR</ReservedWord> | ||
| </ReservedWords> | ||
| <ReservedWords> | ||
| <ReservedWord>NATIONAL </ReservedWord> |
There was a problem hiding this comment.
This is the only reserved word I expect to be different between the old and the new structure. I've assumed that it was a typo.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
benrr101
left a comment
There was a problem hiding this comment.
I like getting rid of xml files - especially when the list seems completely arbitrary :D
| { | ||
| (DataTable syncReservedWordTable, DataTable asyncReservedWordsTable) = await VerifySchemaTable(DbMetaDataCollectionNames.ReservedWords, new string[] { "ReservedWord" }); | ||
|
|
||
| VerifyReservedWordsTable(syncReservedWordTable); |
There was a problem hiding this comment.
I'd prefer it if these were separate tests (maybe a theory?) and VerifyReservedWordsTable was embedded into the test. But this isn't mandatory for approval.
There was a problem hiding this comment.
Thanks @benrr101. It may be worth looking at #4362 before I start looking at changes. That PR is another approach which would also cover this PR's scope (and the rest of the SqlMetaData.xml scope's resources.)
I'd be happy enough to close this PR and tack tests on to the end of #4362, depending on the approach you want to work with.
Description
This PR populates the
ReservedWordscollection returned bySqlConnection.GetSchemain code, rather than by deserializing theSqlMetaData.xmlembedded resource.There are currently 393 reserved words in this embedded resource. To make this easier to review, I've broken it down commit-by-commit.
I'm aiming to do nothing more than migrate the data out of the XML file, with one exception: the "NATIONAL " keyword is now corrected to "NATIONAL", removing the trailing space. I believe this was an oversight.
Separately to this, I've noticed two things:
SERVERPROPERTY('EngineEdition'), or a simpler check against the DNS name.I've avoided pulling a response to either of them in this PR in order to prevent it drifting into the weeds and to keep it self-contained.
Issues
Contributes to #3372.
Testing
I've manually verified that reserved words match, and added a test which verifies that the data is correct. With adjustment to account for the "NATIONAL " case, this same test also passes against main.