microsoft / azuredatastudio

Azure Data Studio is a data management and development tool with connectivity to popular cloud and on-premises databases. Azure Data Studio supports Windows, macOS, and Linux, with immediate capability to connect to Azure SQL and SQL Server. Browse the extension library for more database support options including MySQL, PostgreSQL, and MongoDB.
https://learn.microsoft.com/sql/azure-data-studio
MIT License
7.57k stars 900 forks source link

System-Versioned (Temporal) History Table Index Fail on Build. Not reproducible. SDK-style SQLProj #22734

Open bryan-goodrich opened 1 year ago

bryan-goodrich commented 1 year ago

This is pretty simple. I can't reproduce this error, but I had a clean project. Using the GUI to build the Project. Not connected to any systems (offline development). Using SDK-style project file. Made a change, and rebuilt just to confirm. Got 2 identical errors thrown

Build error SQL71508: The model already has an element that has the same name dbo.tablename.IX_indexname

I investigated the table and index. Realized it was referring to the history table. Knew nothing could actually be wrong. Reran the build command and it succeeded just fine.

Random bug?

Version: 1.43.0 (system setup) Commit: b790d700898b1095d83e62f0de14678a58222520 Date: 2023-04-10T23:14:27.236Z VS Code: 1.67.0 Electron: 19.1.8 Chromium: 102.0.5005.167 Node.js: 16.14.2 V8: 10.2.154.15-electron.0 OS: Windows_NT x64 10.0.19045

SQL Database Projects v1.0.1

kisantia commented 1 year ago

if you're able to repro this again, please share the repro steps and we can try to investigate

kisantia commented 1 year ago

Please reopen this issue if repro steps can be provided

HughMartin-OMES commented 5 months ago

I am getting this same error on a project that has literally not changed between when it build successfully a week ago and when it is failing now. Exact same situation - two identical SQL71508 errors referring to the same index on a history table that is automatically build from a table definition. That table certainly has not changed (no table definitions changed). It builds locally in Visual Studio (latest version 17), but it fails in Azure now. And it fails every time I retry it.

I am trying to see if there are any new options I need to include, or if anything about my project definition changed at all somehow. No changes other than two minor changes to stored procedures (decision logic, not even affecting the output row structure of those procedures).

bryan-goodrich commented 5 months ago

As far as I can tell it's not reproducible (race condition?). Part of the build process when it comes to system versioned tables may fail for no apparent reason. Whether local or in the CI pipeline, I can just rerun the build and it'll work the next time. It doesn't fail often, but if you build enough it will happen at some point I've found. "Try again" works, but does hamper automation!

HughMartin-OMES commented 5 months ago

I have been able to isolate something, but it makes absolutely no sense. And I mean that in the most literal sense.

We have several loader scripts that are very cookie cutter. One of the most basic ones loads our application version history table. This table consists of basic columns:

The version loader stored procedure just MERGEs a manual list of values into this table. The values are just pairs of the version number string and the date/time string in a mm/dd/yyyy hh:mmAM format.

For the current working release branch that is under development, it always has GETUTCDATE() in its date entry instead of a hard coded date/time string like all the others.

So here is the utterly baffling thing. I changed that working version entry that looks like this: ('2.20.0', GETUTCDATE()) to the following that represents the upcoming finalized release date: ('2.20.0', '05/29/2024 12:00PM')

This is the exact same format and the exact same procedure we have done time after time. However, this is the difference that apparently is causing the build failure. Again, note that the build failure is two identical errors with that error SQL71508 referring to the same history table index on each error line. This table hasn't been touched in months, just to be clear.

I changed that back to using GETUTCDATE() and it build. I changed it to use a different date/time and it did NOT build (same error). I then removed the 2.20.0 line altogether, leaving our last version number item as the last one in the list (2.19.2). This means that this procedure, this merge statement, is the exact script that was used when 2.19.2 was published last month. Now, however, it does NOT build (same unrelated index error). Exact same script that worked a month ago. Then I changed that 2.19.2 line to use GETUTCDATE() and it now does build.

So then I left that with GETUTCDATE() in place and added the 2.20.0 entry after it again with the same hard-coded date/time as before. This time it DID build (only difference being that the second to last line had GETUTCDATE() in it instead of its previous literal value).

Next I restored the 2.19.2 and 2.20.0 lines to use the literal hard-coded date/time strings they had before (when the initial build problem was seen). This time, I added another line for a future/fake 2.21.0 version and used GETUTCDATE() for it. At this point, I have this as those last lines in the merge's USING values list: ('2.19.2', '04/23/2024 14:00PM'), ('2.20.0', '05/29/2024 12:00PM'), ('2.21.0', GETUTCDATE()) This time, the project DID build.

These experiments strongly imply that if I have any one of those value lines with GETUTCDATE() in them instead of a hard-coded date like all the others, it builds. But if I have all hard-coded date/time strings, it does NOT build. Even though it built just one month ago. Again note that the error that is given concerning a history table index has absolutely nothing to do with this stored procedure, what it does, and what table it operates on: this stored proc is even for a completely different table than the one that the error would be related to.

I'm going to have to try some kind of bizarre workaround that has GETUTCDATE() in the list without upsetting any of the actual real versions somehow. But I cannot for the life of me understand why this makes any difference with relation to the error we are seeing. The only actual difference is that one month ago I build this project and it works. Today I can build the exact same project in Azure and it does not build with this very bizarre error.

HughMartin-OMES commented 5 months ago

By the way, to try and eliminate any chance of random occurrence, I ran several of these scenarios again during this trial and error process. It was absolutely consistent in that it ALWAYS failed if I had all literal string dates in that stored procedure, and it ALWAYS succeeded if I had one of them with GETUTCDATE() in it.

@kisantia I am not sure what I can do about this issue; I haven't encountered something this baffling in a very long time (if at all). In case I didn't mention it, all of these attempts build perfectly fine locally. I am running the latest update of Visual Studio 17. I even tried running the same msbuild command line that the Azure pipeline uses (minus the logging parameter), and it was totally happy on my computer. :(

HughMartin-OMES commented 5 months ago

I have now also tried using a CONVERT() on the hard-coded date to explicitly cast it as DATETIME2, DATETIME, and SMALLDATETIME. None of these things worked.

I ended up adding a '0.0.0' version row at the top with DATEADD(YEAR, -100, GETUTCDATE()) to create basically a dummy entry when the procedure is called. This use of DATEADD() with GETUTCDATE() does work still, so this is what I am stuck with...

kisantia commented 5 months ago

@ssreerama fyi

HughMartin-OMES commented 5 months ago

Update: additional information.

I was creating the template for our new working version. It has a last line in the values of ('2.21.0', GETUTCDATE()) (indicating it is still in development). When I added this and Azure attempted to build it, I got the same error described originally above. I had to comment out the line I added for version 0.0.0 described in my previous message in order for it to build.

So the conclusion seems to be that I must have one row with GETUTCDATE() in it, but I can't have two rows. This is the most bizarre thing I have ever seen...

HughMartin-OMES commented 4 months ago

I have had another random incident of the same index errors appearing again; I haven't been able to identify any change in the database project between when it worked before, and when it does not work now.

2024-06-11T19:53:22.1422197Z ##[error]D:\a\1\s\xxxxxxxx\MSSQL::history.Notification_History(18,5): Error SQL71508: The model already has an element that has the same name history.Notification_History.IX_Notification_EntityId.
2024-06-11T19:53:22.1433565Z      1>MSSQL::history.Notification_History(18,5,18,5): Build error SQL71508: The model already has an element that has the same name history.Notification_History.IX_Notification_EntityId. [D:\a\1\s\xxxxxxxx\xxxx-Database.sqlproj]
2024-06-11T19:53:22.1437076Z ##[error]D:\a\1\s\xxxxxxxx\MSSQL::history.Notification_History(18,5): Error SQL71508: The model already has an element that has the same name history.Notification_History.IX_Notification_EntityId.
2024-06-11T19:53:22.1440736Z      1>MSSQL::history.Notification_History(18,5,18,5): Build error SQL71508: The model already has an element that has the same name history.Notification_History.IX_Notification_EntityId. [D:\a\1\s\xxxxxxxx\xxxx-Database.sqlproj]

I was reviewing the database definitions, and the "offending" database is very cookie cutter. It has this index for a foreign key ID column, pretty simple. I do notice that the history tables don't actually seem to have these same indexes and constraints that the parent table do. As a result, I don't understand why it would even be trying to create an index like the one showing in the error messages above.

It also occurs to me that the indexes wouldn't necessarily be needed for the history table's existence, so I am not surprised to see that they are not present in the history table definition for the Notification table (as well as others; I did a quick spot check to see if it was an aberration).

I just now noticed one thing that is unique for the Notification table's SSDT definition: It is the only table definition that uses the INDEX declarations inside the CREATE TABLE statement block. All our other table definitions (built in the SSDT project) have the INDEXes created in the table definition SQL but using CREATE INDEX statements after the CREATE TABLE statement block!

The resulting table is identical in its creation regardless of whether the table SQL definition uses the INDEX lines inside the CREATE TABLE vs the table SQL definition using CREATE INDEX in lines following the CREATE TABLE block.

This is definitely a bug; my local SSDT project build produces the same table in the database either way.

cc @kisantia @ssreerama