Invalid SQL for "WHERE ... IN" with parentheses
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 18.4k
- Forks
- 3.7k
- Avg merge
- 5h 8m
- Merged PRs (30d)
- 1
Description
Using Dapper 2.0.90 and
> select @@version;
Microsoft SQL Azure (RTM) - 12.0.2000.8
May 12 2022 23:11:24
Copyright (C) 2022 Microsoft Corporation
Given
SELECT 1 FROM foo WHERE bar IN (@ids)
Dapper produces either
SELECT 1 FROM foo WHERE bar IN (@ids)
or
SELECT 1 FROM foo WHERE bar IN ((@ids1,@ids2))
when the value for @ids is new int[] { 1 } respectively new int[] { 1, 2 }; that is, when the count exceeds 1. The second case produces the error
Microsoft.Data.SqlClient.SqlException : Incorrect syntax near ','.
because SQL Server does not recognize ... IN ( ( ... , ... ) ). Neither does MySQL 8. Postgres 14 also errors in this case but that's because it lacks an equality operator between integers and its non-standard tuple extension (... (bar, bar) IN ((@ids1, @ids2)) should work).
The inconsistency in Dapper's behaviour may be explained by Dapper in actuality doing nothing when the count does not exceed 1.
I would argue that Dapper's behaviour in this case should be strictly consistent, and that its inconsistency is an error.
Whether Dapper should support the parenthesized form in the first place or exclusively the unparenthesized form (as demonstrated in the README) seems to me more of a stylistic decisions. Personally, I would have said that Dapper should never have inserted the parentheses because those patently are not part of the parameter(s), however, obviously Dapper cannot now remove that form. Whether Dapper should preserve support for the parenthesized form is less clear; such code works purely by coincidence. Perhaps Dapper could learn to detect existing parentheses and transparently overwrite them (i.e. keep generating parentheses but avoiding nesting), however, this is a substantially more complex solution to build and I don't know if that would be backwards compatible with Postgres' tuples.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in Dapper/SqlMapper.cs at the linked parameter-expansion code and compare it with the unparenthesized form shown in the README. Reproduce the one-item and multi-item cases against the documented SQL dialects; done means the behavior is consistent and does not generate invalid nested parentheses, while compatibility implications are resolved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, sql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100