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: Different lambdas in QueryAsync generate the same SQL expression (cache problem) #782

Closed
Eyescream opened this issue Mar 10, 2021 · 10 comments
Assignees
Labels
bug Something isn't working priority Top priority feature or things to do todo Things to be done in the future

Comments

@Eyescream
Copy link

Bug Description

It seems that two different lambda expression passed to QueryAsync generate the same SQL expression, because the second expression gets wrongly matched to the first one in the CommandTextCache.

Schema and Model:

CREATE TABLE [dbo].[TB_MY_SET](
	[id_set] [int] IDENTITY(1,1) NOT NULL,
	[country] [int] NOT NULL,
	[id_my] [varchar](46) NOT NULL,
	[part_my] [int] NOT NULL,
	[ver_my] [int] NOT NULL,
	[key_my] [varchar](50) NOT NULL,
	[type_set] [int] NOT NULL,
	[descr] [varchar](255) NULL,
	[comment] [text] NULL,
	[is_deleted] [bit] NOT NULL,
 CONSTRAINT [PK_TB_MY_SET] PRIMARY KEY CLUSTERED 
(
	[id_set] ASC
)WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO
  using RepoDb.Attributes;
    [Map("[dbo].[TB_MY_SET]")]
    public class MyParSet
    {
        [Primary]
        [Map("id_set")]
        public int Id { get; set; }
        [Map("country")]
        public int Country { get; set; }
        [Map("id_my")]
        public string IdMy { get; set; }
        [Map("part_my")]
        public int PartMy { get; set; }
        [Map("ver_my")]
        public int VerMy { get; set; }
        [Map("key_my")]
        public string KeyMy { get; set; }
        [Map("type_set")]
        public int TypeSet { get; set; }
        [Map("descr")]
        public string Descr { get; set; }
        [Map("comment")]
        public string Comment { get; set; }
        [Map("is_deleted")]
        public bool IsDeleted { get; set; }
    }

Bug reproduction:

var idMy = "abc";
var idCountry = 1;
var partMy = -1;

var z = await Context
	.GetConnection()
	.QueryAsync<MyParSet>(x => x.IdMy == idMy && x.PartMy != 0 && x.VerMy == 0 && x.Country == idCountry && x.isDeleted == false,
			transaction: Context.GetTransaction(),
			trace: Trace);
var z2 = await Context
	.GetConnection()
	.QueryAsync<MyParSet>(x => x.IdMy == idMy && x.PartMy == partMy && x.VerMy != 0 && x.Country == idCountry && x.isDeleted == false,
			transaction: Context.GetTransaction(),
			trace: Trace);

Comment:

The generated SQL query is for both the lambda this one:

SELECT [id_set], [country], [id_my], [part_my], [ver_my], [key_my], [type_set], [descr], [comment], [is_deleted] FROM [dbo].[TB_MY_SET] WHERE ((((([id_my] = @id_my) AND ([part_my] <> @part_my)) AND ([ver_my] = @ver_my)) AND ([country] = @country)) AND ([is_deleted] = @is_deleted)) ;

As you see the first and the second lambdas differ from the equality sign between some arguments, but that is not reflected in the cache lookup in the CommandTextCache.cs file, method GetQueryTextAsync,

if (cache.TryGetValue(request, out var commandText) == false)

This lookup is matched the second time, so the generated string for the first lambda is wrongly returned.

Library Version:

Example: RepoDb v1.12.4 and RepoDb.SqlServer v1.1.1, I also tried version 1.1.3 bug the bug was still there.

@Eyescream Eyescream added the bug Something isn't working label Mar 10, 2021
@mikependon mikependon added priority Top priority feature or things to do todo Things to be done in the future labels Mar 10, 2021
@mikependon
Copy link
Owner

Thanks for reporting this issue and also for pinpointing the problem. Is this an urgent issue in your side, or has this been rectified by a different approach?

Note: We are yet to investigate this.

@Eyescream
Copy link
Author

It is a bit urgent (has anyone ever said no? 😅) as the only workaround I've found around this problem was to include a custom built repodb dll with the internal lookup cache completely disabled

@mikependon
Copy link
Owner

Had you tried using the QueryGroup approach as a temporary solution?

@Eyescream
Copy link
Author

I think that would work, but the existing codebase uses the QueryAsync approach. We sadly didn't notice the bug until now. Thank you.

@mikependon mikependon pinned this issue Mar 12, 2021
@mikependon
Copy link
Owner

RepoDB caching mechanism is being driven by the GetHashCode() method of the object. This issue can as well replicated via a simplied call to the GetHashCode() method of the QueryGroup object (as below).

var qg1 = QueryGroup.Parse<MyParSet>(
            x => x.IdMy == "idMy" && x.PartMy != 0 && x.VerMy == 0 && x.Country == 1 && x.IsDeleted == false);
var qg2 = QueryGroup.Parse<MyParSet>(
            x => x.IdMy == "idMy" && x.PartMy == 0 && x.VerMy != 0 && x.Country == 1 && x.IsDeleted == false);
var equals = qg1 == qg2;
Console.WriteLine(qg1.GetHashCode());
Console.WriteLine(qg2.GetHashCode());

@mikependon
Copy link
Owner

The issue can also be replicated this very simplified version of collisions.

var qg1 = QueryGroup.Parse<MyParSet>(x => x.PartMy != 0 && x.VerMy == 0);
var qg2 = QueryGroup.Parse<MyParSet>(x => x.PartMy == 0 && x.VerMy != 0);

@mikependon
Copy link
Owner

The following are the fixes. It was accidentally tag to a wrong ticket number.

@mikependon mikependon unpinned this issue Mar 12, 2021
@mikependon
Copy link
Owner

The fix is now available at RepoDb v1.12.8-beta4.

@Eyescream
Copy link
Author

The fix is now available at RepoDb v1.12.8-beta4.

I tried it and now the two hashcodes are indeed different, so it works fine! Thank you!

@mikependon
Copy link
Owner

Thank you for verifying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority Top priority feature or things to do todo Things to be done in the future
Projects
None yet
Development

No branches or pull requests

2 participants