convertQuestionMark function improperly handles single quotes in comments #1954

New Issue

antialiasis · 2021-06-14T10:32:32Z

antialiasis commented

2021-06-14 10:32:32 +00:00

The convertQuestionMark function converts ? SQL parameters to $1, $2 and so on. It tries to ignore question marks inside single-quoted strings. However, it does this very simplisticially, simply alternating each time a single quote occurs in the SQL string. This is not always correct, as SQL comments may contain single quotes, and this makes the parsing of the rest of the string incorrect.

These tests both fail with the current implementation:

func TestSeqFilterLineComment(t *testing.T) {
	var kases = map[string]string{
		`SELECT *
		FROM TABLE1
		WHERE a=? -- a comment with a single quote'
		AND b=?`: `SELECT *
		FROM TABLE1
		WHERE a=$1 -- a comment with a single quote'
		AND b=$2`,
	}
	for sql, result := range kases {
		assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1))
	}
}

func TestSeqFilterComment(t *testing.T) {
	var kases = map[string]string{
		`SELECT *
		FROM TABLE1
		WHERE a=? /* it's a comment */
		AND b=?`: `SELECT *
		FROM TABLE1
		WHERE a=$1 /* it's a comment */
		AND b=$2`,
	}
	for sql, result := range kases {
		assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1))
	}
}

The `convertQuestionMark` function converts ? SQL parameters to $1, $2 and so on. It tries to ignore question marks inside single-quoted strings. However, it does this very simplisticially, simply alternating each time a single quote occurs in the SQL string. This is not always correct, as SQL comments may contain single quotes, and this makes the parsing of the rest of the string incorrect. These tests both fail with the current implementation: ``` func TestSeqFilterLineComment(t *testing.T) { var kases = map[string]string{ `SELECT * FROM TABLE1 WHERE a=? -- a comment with a single quote' AND b=?`: `SELECT * FROM TABLE1 WHERE a=$1 -- a comment with a single quote' AND b=$2`, } for sql, result := range kases { assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1)) } } func TestSeqFilterComment(t *testing.T) { var kases = map[string]string{ `SELECT * FROM TABLE1 WHERE a=? /* it's a comment */ AND b=?`: `SELECT * FROM TABLE1 WHERE a=$1 /* it's a comment */ AND b=$2`, } for sql, result := range kases { assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1)) } } ```

antialiasis changed title from ~~convertQuestionMark function improperly handles single quotes~~ to convertQuestionMark function improperly handles single quotes in comments

2021-06-14 11:17:04 +00:00

lunny added the

kind

bug

label 2021-06-14 14:13:26 +00:00

lunny commented

2021-06-14 14:13:34 +00:00

Could you send a PR to fix that?

antialiasis referenced this issue

2021-06-14 16:16:51 +00:00

Ignore comments when deciding when to replace question marks. #1954 #1955

lunny referenced this issue from a commit