Parameter limitation on prefetch_related

[techknowfile]

This is an issue that of course also exists in the ESSolution library that this repo is forked from.

When using prefetch_related to optimize queries for many-to-one and many-to-many relationships, Django generates an IN clause containing all of the primary keys that a M:1 model's foreign key should be filtered on, thus solving the N+1 problem.

However, MS SQL Server has a 2100 parameter limit, which causes the rdbms to fail when trying to prefetch objects with more than 2100 possible foreign keys. It would be awesome if the driver could add support for handling this stark limitation.

The official MS SQL Server IN clause documentation recommends creating a temp table and then joining over that. Without being too familiar with Django ORM internals, I don't know how feasible it would be to override the prefetch_related as_sql query to implement this solution.

There is a related issue in the ESSolutions/django-mssql-backend repo, where a suggested workaround is to override Django's In lookup class to basically pass in a single giant parameter string and then have SQL Server split it via {lhs} IN ( SELECT * FROM STRING_SPLIT(CONVERT(nvarchar(max), %s),','))). While this seems to work when filtering a queryset explicitly with __in, the overridden lookup class is not used for prefetch_related calls.

A committed solution would be awesome, but I'd also love any advice you may have as a temporary workaround.

[vwarchu]

@techknowfile - Thanks for the issue report. We'll take a look and get back to you regarding a potential solution and/or temp workaround. Cheers

[techknowfile]

Thanks, @vwarchu. I think we're going to temporarily hack around this issue by monkey-patching the base.execute() function to identify when we have too many parameters with a query that contains a single IN clause and move those parameters into a temp table instead.

[absci]

@techknowfile I'm able to reproduce the issue, I think Django has some code to split parameters. But it doesn't work with SQL Server. I'll try to provide a solution for that.

[absci]

I have committed a fix in dev branch. Let me know if you encountered any problems.

[techknowfile]

Thank you, @absci! We had come to a similar approach of creating a temp table, but this is a much cleaner place of putting it

One issue we have rn is that your solution doesn't work when UUIDs are used as the PK. Added comments for suggested changes to your commit

[absci]

I see. If that's the case, it might be better to use different SQL data types for each Django models.

[absci]

This issue has been fixed.