DapperLib / DapperLib/Dapper

Invalid SQL for "WHERE ... IN" with parentheses

Open
#1,819 8 comments 0 reactions 0 assignees View on GitHub

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.