henkmollema / Dommel

CRUD operations with Dapper made simple.
MIT License
634 stars 100 forks source link

Including table name on QueryCache #268

Closed SalatielBairros closed 3 years ago

SalatielBairros commented 3 years ago

I'm having problems with the Dommel query cache when using the same table on multiple schemas (when the database has multiple schemas and the same table in each). On my application I'm selecting the schema by user permissions from request, but the query is beeing generated always by the cache.

This PR fix it including the table name on QueryCache to avoid this kind of error.

codecov[bot] commented 3 years ago

Codecov Report

Merging #268 (b6cb698) into master (67cef37) will increase coverage by 0.12%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #268      +/-   ##
==========================================
+ Coverage   79.12%   79.24%   +0.12%     
==========================================
  Files          32       32              
  Lines        1188     1195       +7     
  Branches      136      136              
==========================================
+ Hits          940      947       +7     
  Misses        213      213              
  Partials       35       35              
Impacted Files Coverage Δ
src/Dommel/Any.cs 100.00% <100.00%> (ø)
src/Dommel/Cache.cs 100.00% <100.00%> (ø)
src/Dommel/Count.cs 100.00% <100.00%> (ø)
src/Dommel/Delete.cs 100.00% <100.00%> (ø)
src/Dommel/Get.cs 100.00% <100.00%> (ø)
src/Dommel/Insert.cs 100.00% <100.00%> (ø)
src/Dommel/Project.cs 100.00% <100.00%> (ø)
src/Dommel/Update.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 67cef37...b6cb698. Read the comment docs.

henkmollema commented 3 years ago

I understand the problem but by including the table name the purpose of the cache is pretty much defeated 😄 This can be solved by using different types for tables in different schemas, right?

SalatielBairros commented 3 years ago

I understand the problem but by including the table name the purpose of the cache is pretty much defeated 😄 This can be solved by using different types for tables in different schemas, right?

The problem is that the schema is dynamic. Each user belongs to a tenant and each tenant has its own schema. It could be hundreds of tenants, each one with its own schema with its tables. The same table exists in n schemas for the same rules on the application. The current schema is identified by info present on authorization token. I created a lib that integrate this repo with fluent migrator, so I'm using the table configuration to controll both queries and migrations for each schema.

henkmollema commented 3 years ago

I understand. But unfortunately I think Dommel is not suited for your scenario.