Incrementing revision numbers in table's compo

Question

I'm running SQL Server 2014 locally for a database that will be deployed to an Azure SQL V12 database.

I have a table that stores values of extensible properties for a business-entity object, in this case the three tables look like this:

CREATE TABLE Widgets (
    WidgetId bigint IDENTITY(1,1),
    ...
)

CREATE TABLE WidgetProperties (
    PropertyId int IDENTITY(1,1),
    Name       nvarchar(50)
    Type       int -- 0 = int, 1 = string, 2 = date, etc
)

CREATE TABLE WidgetPropertyValues (
    WidgetId   bigint,
    PropertyId int,
    Revision   int,
    DateTime   datetimeoffset(7),
    Value      varbinary(255)

    CONSTRAINT [PK_WidgetPropertyValues] PRIMARY KEY CLUSTERED (
        [WidgetId] ASC,
        [PropertyIdId] ASC,
        [Revision] ASC
    )
)

ALTER TABLE dbo.WidgetPropertyValues WITH CHECK ADD CONSTRAINT FK_WidgetPropertyValues_WidgetProperties FOREIGN KEY( PropertyId )
REFERENCES dbo.WidgetProperties ( PropertyId )

ALTER TABLE dbo.WidgetPropertyValues WITH CHECK ADD  CONSTRAINT FK_WidgetPropertyValues_Widgets FOREIGN KEY( WidgetId )
REFERENCES dbo.Widgets ( WidgetId )

So you see how WidgetId, PropertyId, Revision is a composite key and the table stores the entire history of Values (the current values are obtained by getting the rows with the biggest Revision number for each WidgetId + PropertyId.

I want to know how I can set-up the Revision column to increment by 1 for each WidgetId + PropertyId. I want data like this:

WidgetId, PropertyId, Revision, DateTime, Value
------------------------------------------------
       1           1         1               123
       1           1         2               456
       1           1         3               789
       1           2         1               012

IDENTITY wouldn't work because it's global to the table and the same applies with SEQUENCE objects.

Update I can think of a possible solution using an INSTEAD OF INSERT trigger:

CREATE TRIGGER WidgetPropertyValueInsertTrigger ON WidgetPropertyValues
    INSTEAD OF INSERT
AS
BEGIN
    DECLARE @maxRevision int
    SELECT @maxRevision = ISNULL( MAX( Revision ), 0 ) FROM WidgetPropertyValues WHERE WidgetId = INSERTED.WidgetId AND PropertyId = INSERTED.PropertyId

    INSERT INTO WidgetPropertyValues VALUES (
        INSERTED.WidgetId,
        INSERTED.PropertyId,
        @maxRevision + 1,
        INSERTED.DateTime,
        INSERTED.Value,
    )
END

(For the uninitiated, INSTEAD OF INSERT triggers run instead of any INSERT operation on the table, compared to a normal INSERT-trigger which runs before or after an INSERT operation)

I think this would be concurrency-safe because all INSERT operations have an implicit transaction, and any associated triggers are executed in the same transaction context, which should mean it's safe. Unless anyone can claim otherwise?

Answer 1

You code has a race condition - a concurrent transaction might select and insert the same Revision between your SELECT and your INSERT. That could cause occasional (primary) key violations in concurrent environment (forcing you to retry the entire transaction).

Instead of retrying the whole transaction, a better strategy is to retry only the INSERT. Simply put your code in a loop, and if key violation (and only key violation) happens, increment the Revision and try again.

Something like this (writing from my head):

DECLARE @maxRevision int = (
    SELECT
        @maxRevision = ISNULL(MAX(Revision), 0)
    FROM
        WidgetPropertyValues
    WHERE
        WidgetId = INSERTED.WidgetId
        AND PropertyId = INSERTED.PropertyId
);

WHILE 0 = 0 BEGIN

    SET @maxRevision = @maxRevision + 1;

    BEGIN TRY

        INSERT INTO WidgetPropertyValues
        VALUES (
            INSERTED.WidgetId,
            INSERTED.PropertyId,
            @maxRevision,
            INSERTED.DateTime,
            INSERTED.Value,
        );

        BREAK;

    END TRY
    BEGIN CATCH

        -- The error was different from key violation,
        -- in which case we just pass it back to caller.
        IF ERROR_NUMBER() <> 2627
            THROW;

        -- Otherwise, this was a key violation, and we can let the loop 
        -- enter the next iteration (to retry with the incremented value).

    END CATCH

END