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

Automatic multi mapping not properly mapping on 3.3.3 update #308

Open
bromike opened this issue Aug 27, 2024 · 4 comments
Open

Automatic multi mapping not properly mapping on 3.3.3 update #308

bromike opened this issue Aug 27, 2024 · 4 comments
Assignees
Labels

Comments

@bromike
Copy link

bromike commented Aug 27, 2024

From 3.3.2 to 3.3.3, we noticed that some queries became flaky and started failing, at some times where it seemed random.

We are mostly using queries similar to:

[Table("elements")]
public class Element
{
    [Key][DatabaseGenerated(DatabaseGeneratedOption.None)]
    public Guid ElementKeyId { get; set; }

    public Guid OwnerId { get; set; }

    [ForeignKey(nameof(ElementTag.ElementKeyId))]
    public List<ElementTag> ElementTags { get; set; }
}

[Table("element_tags")]
public class ElementTag
{
    [Key][DatabaseGenerated(DatabaseGeneratedOption.None)]
    public Guid ElementKeyId { get; set; }
    [Key][DatabaseGenerated(DatabaseGeneratedOption.None)]
    public string Tag { get; set; }
}

public class ElementsService
{
    public List<Element> GetElementsAsync(Guid ownerId)
    {
        await using var connection = await _connectionFactory.CreateAsync();
        var elements = (await connection.SelectAsync<Element, ElementTag, Element>(i => i.OwnerId == ownerId)).ToList();
        // some logic here
        return elements;
    }
}

We noticed that sometimes, it would wrongly map element tags to an element.
It would return this:

[
  Element
  {
    ElementKeyId = {11111111-1111-1111-1111-111111111111},
    OwnerId = {00000000-0000-0000-0000-00000000000A},
    ElementTags = ElementTag
      {
        ElementKeyId = {11111111-1111-1111-1111-111111111111}, 
        Tag = "a"
      }, 
      ElementTag
      {
        ElementKeyId = {33333333-3333-3333-3333-333333333333}, <-- this should go with element of the same ElementKeyId
        Tag = "c"
      }
    },
  }, 
  Element
  {
    ElementKeyId = {22222222-2222-2222-2222-222222222222},
    OwnerId = {00000000-0000-0000-0000-00000000000A},
    ElementTags = ElementTag
    {
      {
        ElementKeyId = {22222222-2222-2222-2222-222222222222}, 
        Tag = "b"
      }
    },
  }
]

Rather than returning tags according to their ElementKeyIds:

[
  Element
  {
    ElementKeyId = {11111111-1111-1111-1111-111111111111},
    OwnerId = {00000000-0000-0000-0000-00000000000A},
    ElementTags = ElementTag
    {
      {
        ElementKeyId = {11111111-1111-1111-1111-111111111111}, 
        Tag = "a"
      }
    },
  },
  Element
  {
    ElementKeyId = {22222222-2222-2222-2222-222222222222}, 
    OwnerId = {00000000-0000-0000-0000-00000000000A},
    ElementTags = ElementTag
    {
      {
        ElementKeyId = {22222222-2222-2222-2222-222222222222}, 
        Tag = "b"
      }
    }
  },
  Element
  {
    ElementKeyId = {33333333-3333-3333-3333-333333333333},
    OwnerId = {00000000-0000-0000-0000-00000000000A},
    ElementTags = ElementTag
    {
      {
        ElementKeyId = {33333333-3333-3333-3333-333333333333}, 
        Tag = "c"
      }
    }
  }
]

However, looking at the logs from Dommel, It would actually print out the proper query in the logs:

...
MultiMapAsync<Element>: select * from `elements` left join `element_tags` on `elements`.`element_key_id` = `element_tags`.`element_key_id` where `elements`.`owner_id` = @p1
MultiMapAsync<Element>: select * from `elements` left join `element_tags` on `elements`.`element_key_id` = `element_tags`.`element_key_id` where `elements`.`owner_id` = @p1

It is very flaky, sometimes it will work, sometimes not. So I'm not sure if it's related to how it works with the cache key, but this actually blocks the upgrade. There is no issue on 3.3.2

@henkmollema henkmollema self-assigned this Sep 11, 2024
@henkmollema
Copy link
Owner

Thanks for reporting this.
Seems like the new way to combine primary key values for the cache key don't work correctly for non-numeric ID's. I'll look into it.

@henkmollema
Copy link
Owner

Are your ID's actually 11111111-1111-1111-1111-111111111111, 22222222-2222-2222-2222-222222222222 and 33333333-3333-3333-3333-333333333333? Because their hash codes are all 0. However I'd suspect that this would lead to flaky mapping before, since that logic also used the hash code.

@bromike
Copy link
Author

bromike commented Sep 16, 2024

Are your ID's actually 11111111-1111-1111-1111-111111111111, 22222222-2222-2222-2222-222222222222 and 33333333-3333-3333-3333-333333333333? Because their hash codes are all 0. However I'd suspect that this would lead to flaky mapping before, since that logic also used the hash code.

Not always, these were some examples but they are also usually generated via Guid.NewGuid() in the context of unit tests.

@henkmollema
Copy link
Owner

I can't reproduce the issue with actual generated GUID's (eg Guid.NewGuid()). Do you have an example of that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants