Potential Issue / Improvement with v7 on SQL Transport with default columns

Hi,

I’m looking at upgrading components that are combination of V4 and V6 to the latest versions of NServiceBus and NServiceBus SQL Transport versions.

I’ve got a prototype I think is nearly there minus distributed transactions.

However I’m getting the following error from the new native delay functionality.

“Column name or number of supplied values does not match table definition” being sent from the DueDelayProcessor

Example Exception

Exception thrown while moving matured delayed messages
Microsoft.Data.SqlClient.SqlException (0x80131904): Column name or number of supplied values does not match table definition.
at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action1 wrapCloseInAction) at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose) at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady) at Microsoft.Data.SqlClient.SqlDataReader.TryConsumeMetaData() at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData() at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted) at Microsoft.Data.SqlClient.SqlCommand.CompleteAsyncExecuteReader(Boolean isInternal, Boolean forDescribeParameterEncryption) at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteReader(IAsyncResult asyncResult, Boolean isInternal, String endMethod) at Microsoft.Data.SqlClient.SqlCommand.EndExecuteReaderInternal(IAsyncResult asyncResult) at Microsoft.Data.SqlClient.SqlCommand.EndExecuteReaderAsync(IAsyncResult asyncResult) at System.Threading.Tasks.TaskFactory1.FromAsyncCoreLogic(IAsyncResult iar, Func2 endFunction, Action1 endAction, Task`1 promise, Boolean requiresSynchronization)
— End of stack trace from previous location —
at NServiceBus.Transport.SqlServer.DelayedMessageTable.MoveDueMessages(Int32 batchSize, SqlConnectio

I’ve struggled to debug the SQL you are calling, but believe there may be an assumption which wasn’t required in the earlier versions which didn’t have this functionality.

I’ve reviewed the code on Github and the information from the following pages:

e.g.

;WITH message AS (
    SELECT TOP(@BatchSize) *
    FROM {0} WITH (UPDLOCK, READPAST, ROWLOCK)
    WHERE Due < GETUTCDATE())
DELETE FROM message
OUTPUT
    NEWID(),
    NULL,
    NULL,
    1,
    NULL,
    deleted.Headers,
    deleted.Body
INTO {1};

SELECT TOP 1 GETUTCDATE() as UtcNow, Due as NextDue
FROM {0} WITH (READPAST)
ORDER BY Due

I’m assuming because the OUTPUT command doesn’t specify columns, any additional columns aren’t allowed. I notice you are making use of this, but as RowVersion is IDENTITY it doesn’t fall foul of the problem. Default columns which are used by DBA’s or trigger columns are affected by this.

I’m hesitant to drop any columns that have been in a working Production system for 9 years or so without incident and this will be a wider issue if we remove them.

Given the order of the columns in this type of statement are implied anyway, can we not be specific with the column names and this problem goes away? I’ve supplied an example test case below.

I did think of logging this on the github board as a bug, but I think it’s more in the area of an improvement in how DBA’s / companies will deploy NServiceBus.

What do you think?

Cheers,
Jamie

Table

CREATE TABLE [dbo].[TestTable](
	[Id] [uniqueidentifier] NOT NULL,
	[CorrelationId] [varchar](255) NULL,
	[ReplyToAddress] [varchar](255) NULL,
	[Recoverable] [bit] NOT NULL,
	[Expires] [datetime] NULL,
	[Headers] [varchar](max) NOT NULL,
	[Body] [varbinary](max) NULL,
	[RowVersion] [bigint] IDENTITY(1,1) NOT NULL,
	[CreatedDate] [datetime2](7) NOT NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO

 CREATE TABLE [dbo].[TestTable.Delayed] (
        Headers nvarchar(max) NOT NULL,
        Body varbinary(max),
        Due datetime NOT NULL,
        RowVersion bigint IDENTITY(1,1) NOT NULL
    );

    CREATE NONCLUSTERED INDEX [Index_Due] ON  [dbo].[TestTable.Delayed]
    (
        [Due]
    )


DECLARE @NOCOUNT VARCHAR(3) = 'OFF';
IF ( (512 & @@OPTIONS) = 512 ) SET @NOCOUNT = 'ON'
SET NOCOUNT ON;

INSERT INTO [dbo].[TestTable.Delayed]
           ([Headers]
           ,[Body]
           ,[Due])
     VALUES
           ('Bla'
           ,CAST('sdsdsdsds' as VARBINARY(MAX))
           ,getutcdate())

IF(@NOCOUNT = 'ON') SET NOCOUNT ON;
IF(@NOCOUNT = 'OFF') SET NOCOUNT OFF;

GO

;WITH message AS (
    SELECT TOP(1) *
    FROM [TestTable.Delayed] WITH (UPDLOCK, READPAST, ROWLOCK)
    WHERE Due < GETUTCDATE()
)
DELETE FROM message
OUTPUT
    NEWID(),
    NULL,
    NULL,
    1,
    NULL,
    deleted.Headers,
    deleted.Body
INTO [dbo].[TestTable]
(Id, CorrelationId, ReplyToAddress, Recoverable, Expires, Headers, Body)

SELECT TOP 100 GETUTCDATE() as UtcNow, Due as NextDue
FROM [TestTable.Delayed] WITH (READPAST)
ORDER BY Due

I will reply on the GitHub issue you raised.

1 Like

Thanks David.

I’ve taken the version of this and so far so good!

Really appreciate this, especially the fast turnaround time. :clap: