Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Output parameters of type varchar gives Size exception #836

Closed
kiwidude68 opened this issue Jun 1, 2021 · 9 comments
Closed

Bug: Output parameters of type varchar gives Size exception #836

kiwidude68 opened this issue Jun 1, 2021 · 9 comments
Assignees
Labels
bug Something isn't working fixed The bug, issue, incident has been fixed.

Comments

@kiwidude68
Copy link

Bug Description

Trying to call a stored procedure which has an output parameter of type varchar (sproc attached)
p_BreakglassRequest_Update.txt

Returning int or datetime output parameters seem to work fine, but varchar gives the exception below.

Exception Message:

An unhandled exception has occurred while executing the request. System.InvalidOperationException: String[22]: the Size property has an invalid size of 0.
   at Microsoft.Data.SqlClient.SqlParameter.Validate(Int32 index, Boolean isCommandProc)
   at Microsoft.Data.SqlClient.SqlCommand.SetUpRPCParameters(_SqlRPC rpc, Boolean inSchema, SqlParameterCollection parameters)
   at Microsoft.Data.SqlClient.SqlCommand.BuildRPC(Boolean inSchema, SqlParameterCollection parameters, _SqlRPC& rpc)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteReaderInternal(CommandBehavior behavior, AsyncCallback callback, Object stateObject, Int32 timeout, Boolean inRetry, Boolean asyncWrite)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteReaderAsyncCallback(AsyncCallback callback, Object stateObject)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncImpl(Func`3 beginMethod, Func`2 endFunction, Action`1 endAction, Object state, TaskCreationOptions creationOptions)
   at System.Threading.Tasks.TaskFactory`1.FromAsync(Func`3 beginMethod, Func`2 endMethod, Object state)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
--- End of stack trace from previous location ---
   at RepoDb.DbConnectionExtension.ExecuteScalarAsyncInternal[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, CancellationToken cancellationToken, Type entityType, IEnumerable`1 dbFields, Boolean skipCommandArrayParametersCheck)
   at LevelUp.Core.Services.Repositories.BreakglassRequestRepository.UpdateAsync(BreakglassRequest breakglassRequest) in D:\Dev\Infrastructure\LevelUp\LevelUp.Core\Services\Repositories\BreakglassRequestRepository.cs:line 124
   at LevelUp.Pages.Breakglass.EditRequestModel.DoPostAction(Action applyPropertyChangesAction, Boolean forceValidate) in D:\Dev\Infrastructure\LevelUp\LevelUp\Pages\Breakglass\EditRequest.cshtml.cs:line 185
   at LevelUp.Pages.Breakglass.EditRequestModel.OnPostSubmitAsync() in D:\Dev\Infrastructure\LevelUp\LevelUp\Pages\Breakglass\EditRequest.cshtml.cs:line 99

Schema and Model:

Please share to us the schema of the table (not actual) that could help us replicate the issue if necessary.

CREATE TABLE [dbo].[BreakglassRequest](
	[Id] [int] IDENTITY(1,1) NOT NULL,
	[RequestType] [varchar](20) NOT NULL,
	[BreakglassUser] [varchar](50) NOT NULL,
	[ClientCode] [varchar](3) NOT NULL,
	[Applications] [varchar](1000) NOT NULL,
	[StartDateTime] [datetime] NOT NULL,
	[EndDateTime] [datetime] NOT NULL,
	[Justification] [varchar](1500) NOT NULL,
	[Status] [varchar](20) NOT NULL,
	[ErrorMessage] [varchar](100) NULL,
	[CreatedBy] [varchar](50) NOT NULL,
	[CreatedDateTime] [datetime] NOT NULL,
	[ModifiedBy] [varchar](50) NULL,
	[ModifiedDateTime] [datetime] NULL,
	[ApprovedBy] [varchar](50) NULL,
	[ApprovedDateTime] [datetime] NULL,
	[CancelledBy] [varchar](50) NULL,
	[CancelledDateTime] [datetime] NULL,
	[RowVersion] [timestamp] NOT NULL
CONSTRAINT [PK_BreakglassRequest] PRIMARY KEY CLUSTERED 
(
	[Id] ASC
)

And also the model that corresponds the schema.

    public class BreakglassRequest
    {
        [Key]
        [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
        public int Id { get; set; }
        public BreakglassRequestType RequestType { get; set; }
        public string BreakglassUser { get; set; }
        public string ClientCode { get; set; }
        public string Applications { get; set; }
        public DateTime StartDateTime { get; set; }
        public DateTime EndDateTime { get; set; }
        public string Justification { get; set; }
        public BreakglassStatus Status { get; set; } = BreakglassStatus.Pending;
        public string ErrorMessage { get; set; }
        public string CreatedBy { get; set; }
        public DateTime CreatedDateTime { get; set; }
        public string ModifiedBy { get; set; }
        public DateTime? ModifiedDateTime { get; set; }
        public string ApprovedBy { get; set; }
        public DateTime? ApprovedDateTime { get; set; }
        public string CancelledBy { get; set; }
        public DateTime? CancelledDateTime { get; set; }
        [Timestamp]
        public byte[] RowVersion { get; set; }
    }

Library Version:

RepoDb v1.12.7 and RepoDb.SqlServer v1.1.3

Original Code Attempt:

In my own repository class (not inheriting from RepoDb repositories) my code looked like this:

        public async Task<ConcurrencyFailureInfo> UpdateAsync(BreakglassRequest breakglassRequest)
        {
            DirectionalQueryField lastUpdateUserField = new DirectionalQueryField("LastUpdateUser", typeof(string), ParameterDirection.Output);
            DirectionalQueryField lastUpdateDateTimeField = new DirectionalQueryField("LastUpdateDateTime", typeof(DateTime), ParameterDirection.Output);
            
            using (SqlConnection connection = new SqlConnection(m_connectionString))
            {
                int success = await connection.ExecuteScalarAsync<int>("p_BreakglassRequest_Update",
                    new []
                    {
                        new QueryField("Id", breakglassRequest.Id),
                        new QueryField("BreakglassUser", breakglassRequest.BreakglassUser),
                        new QueryField("Applications", breakglassRequest.Applications),
                        new QueryField("StartDateTime", breakglassRequest.StartDateTime),
                        new QueryField("EndDateTime", breakglassRequest.EndDateTime),
                        new QueryField("Justification", breakglassRequest.Justification ?? SqlString.Null),
                        new QueryField("Status", breakglassRequest.Status.ToString()),
                        new QueryField("ErrorMessage", breakglassRequest.ErrorMessage ?? SqlString.Null),
                        new QueryField("ModifiedBy", breakglassRequest.ModifiedBy ?? SqlString.Null),
                        new QueryField("ModifiedDateTime", breakglassRequest.ModifiedDateTime ?? SqlDateTime.Null),
                        new QueryField("ApprovedBy", breakglassRequest.ApprovedBy ?? SqlString.Null),
                        new QueryField("ApprovedDateTime", breakglassRequest.ApprovedDateTime ?? SqlDateTime.Null),
                        new QueryField("CancelledBy", breakglassRequest.CancelledBy ?? SqlString.Null),
                        new QueryField("CancelledDateTime", breakglassRequest.CancelledDateTime ?? SqlDateTime.Null),
                        new QueryField("RowVersion", breakglassRequest.RowVersion),
                        lastUpdateUserField,
                        lastUpdateDateTimeField
                    }, 
                    CommandType.StoredProcedure);
                if (success == 0)
                    return null;
            }

            ConcurrencyFailureInfo info = new ConcurrencyFailureInfo();
            info.LastUpdateUser = lastUpdateUserField.GetValue<string>();
            info.LastUpdateDateTime = lastUpdateDateTimeField.GetValue<DateTime>();

            m_logger.LogInformation($"Failed to update in DB as request {breakglassRequest.Id} was modified by user {info.LastUpdateUser} at {info.LastUpdateDateTime}");
            return info;
        }

Note that I found a second issue (mentioned on Gitter) to do with the above code in trying to pass that RowVersion property to the sproc. If you comment out the "lastUpdateUserField, LastUpdateDateTimeField" parameters and also comment out the related code in the sproc, I found I could not get the sproc to be invoked, with it complaining about a non matching number of parameters. If I also commented out the RowVersion QueryField above (and corresponding lines in the sproc) then the sproc works fine.

My workaround for all of this currently was to eliminate the output parameters completely from the sproc, requiring an additional db call in the situation where there is a concurrency issue (not a major deal given the likelihood of it happening but not ideal nonetheless). In making that change I was then able to eliminate using "QueryField" arguments to the sproc call, and instead use the code below, which incidentally solved my issue with the Timestamp parameter property to the sproc. So I "think" there is an issue with passing a QueryField parameter to a Timestamp property in the sproc given this now works for me, but I understand if you are sceptical...

...
            using (SqlConnection connection = new SqlConnection(m_connectionString))
            {
                int success = await connection.ExecuteScalarAsync<int>("p_BreakglassRequest_Update",
                    new
                    {
                        Id = breakglassRequest.Id,
                        BreakglassUser = breakglassRequest.BreakglassUser,
                        Applications = breakglassRequest.Applications,
                        StartDateTime = breakglassRequest.StartDateTime,
                        EndDateTime = breakglassRequest.EndDateTime,
                        Justification = breakglassRequest.Justification ?? SqlString.Null,
                        Status = breakglassRequest.Status.ToString(),
                        ErrorMessage = breakglassRequest.ErrorMessage ?? SqlString.Null,
                        ModifiedBy = breakglassRequest.ModifiedBy ?? SqlString.Null,
                        ModifiedDateTime = breakglassRequest.ModifiedDateTime ?? SqlDateTime.Null,
                        ApprovedBy = breakglassRequest.ApprovedBy ?? SqlString.Null,
                        ApprovedDateTime = breakglassRequest.ApprovedDateTime ?? SqlDateTime.Null,
                        CancelledBy = breakglassRequest.CancelledBy ?? SqlString.Null,
                        CancelledDateTime = breakglassRequest.CancelledDateTime ?? SqlDateTime.Null,
                        RowVersion = breakglassRequest.RowVersion,
                    }, 
                    CommandType.StoredProcedure);
                if (success == 0)
                    return null;
            }
...
@kiwidude68 kiwidude68 added the bug Something isn't working label Jun 1, 2021
@kiwidude68 kiwidude68 changed the title BUg: Output parameters of type varchar give exception Bug: Output parameters of type varchar gives Size exception Jun 1, 2021
@veenroid
Copy link

Confirmed. I'm seeing the same error message with a single output param of type string/nvarchar(6).

@mikependon
Copy link
Owner

Thanks gents. Apology for the late reply, been on a long vacation 😄 . I have this verified and it is confirmed as well. This will be a part of the next release.

There is alternative anyway, see below.

var commandText = @"DECLARE @returnValue NVARCHAR(MAX);
EXEC sp_whatever @returnValue OUT;
SELECT @returnValue;";
var result = connection.ExecuteScalar<string>(commandText);

@mikependon mikependon pinned this issue Jul 30, 2021
mikependon added a commit that referenced this issue Aug 28, 2021
mikependon added a commit that referenced this issue Aug 28, 2021
@mikependon
Copy link
Owner

The fixes for this issue has been delivered in our main line. It will be available to the next version of the library > RepoDB (v1.12.8-beta4).

@veenroid
Copy link

With 1.12.8-beta5 I'm still seeing this issue. I hope it's something I'm doing wrong.
Stored Proc test as expected. C# method as follows. Any ideas what I might be doing wrong?

        public Group Add(Group newGroup) {
            using var cn = Connection;
            cn.Open();
            var output = new DirectionalQueryField("GroupId", typeof(int), ParameterDirection.Output);
            var @params = new[] {
                new QueryField("Name", newGroup.Name),
                new QueryField("About", newGroup.About),
                new QueryField("OwnerId", newGroup.OwnerId),
                new QueryField("ProfileImage", newGroup.ProfileImage)
            };

            var rowsAffected = cn.ExecuteNonQuery("dbo.usp_GroupInsert", @params, commandType: CommandType.StoredProcedure);
            newGroup.GroupId = output.GetValue<int>();
            //newGroup.GroupId = (int)output.Parameter.Value;
            return newGroup;
        }

@mikependon
Copy link
Owner

This issue should be fixed there and what you are doing is correct. Can you try see if you can get the value on the QueryField.Parameter.Value instead of QueryField.GetValue()? We do this on our integration test as seen here. If that's the fix, then, this is something we need to revisit on our end.

@mikependon
Copy link
Owner

Well, it should still return the actual value for the newly introduced GetValue(). So what you did is really correct.

Let us check this.

image

@mikependon
Copy link
Owner

This one is working on the latest beta version of SQL Server (v1.1.4 -beta1).

SP:

DROP PROCEDURE IF EXISTS [dbo].[sp_TestOutput];
GO

CREATE PROCEDURE [dbo].[sp_TestOutput]
(
	@OutputInt INT OUTPUT
	, @OutputText NVARCHAR(256) OUTPUT
)
AS
BEGIN
	SET @OutputInt = 10045;
	SET @OutputText = 'The quick brown fox';
END

C#:

namespace RepoDbDirectionalQueryField
{
    class Program
    {
        static void Main(string[] args)
        {
            Initialize();
            TestDirectionalQueryField();
        }

        static void Initialize() =>
            SqlServerBootstrap.Initialize();

        static void TestDirectionalQueryField()
        {
            using (var connection = new SqlConnection("Server=.;Database=TestDB;Integrated Security=SSPI;"))
            {
                // Setup
                var outputInt = new DirectionalQueryField("OutputInt", typeof(int), ParameterDirection.Output);
                var outputText = new DirectionalQueryField("OutputText", typeof(string), 256, ParameterDirection.Output);
                var param = new QueryGroup(new[]
                { 
                    outputInt,
                    outputText
                });

                // Act
                connection.ExecuteNonQuery("sp_TestOutput",
                    param,
                    CommandType.StoredProcedure);

                // Assert
                var intValue = outputInt.GetValue<int>();
                var textValue = outputText.GetValue<string>();
            }
        }
    }
}

@mikependon
Copy link
Owner

In your case, you need to pass your variable output as part of the array in the param argument 😄 . That's the catch in your code.

@veenroid
Copy link

Works perfectly. Thank you!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed The bug, issue, incident has been fixed.
Projects
None yet
Development

No branches or pull requests

3 participants