Skip to content

fix($conditionalTest): preserve commas inside SQL_if (quotes + \, escape)#903

Open
oplehto wants to merge 1 commit into
Altinity:masterfrom
oplehto:fix/conditionaltest-comma
Open

fix($conditionalTest): preserve commas inside SQL_if (quotes + \, escape)#903
oplehto wants to merge 1 commit into
Altinity:masterfrom
oplehto:fix/conditionaltest-comma

Conversation

@oplehto

@oplehto oplehto commented Jun 8, 2026

Copy link
Copy Markdown

The new conditionalTest 3-parameter format caused some existing queries to break where a comma was a part of the first parameter value. These are somewhat common if we extend GROUP BY or ORDER BY lists with a conditionalTest macro: Often there is a need to have a leading comma in the string that conditionalTest produces.

Furthermore it would be good IMO to disambiguate if a comma is a delimiter or part of a string properly so there should also be a "proper" form where the commas are escaped.

The 2- vs 3-parameter form of $conditionalTest was decided purely by counting top-level commas, and the comma scan only skipped commas inside parentheses. Any 2-parameter macro whose SQL_if contained a comma was therefore misparsed and the comma dropped, producing broken SQL. Affected cases:

  • leading-comma idiom: $conditionalTest(, expr, $var)
  • trailing-comma idiom: $conditionalTest(expr,, $var)
  • a comma inside a string literal, e.g. AND t = 'a,b', was treated as an
    argument separator and truncated the query.

Fix which maintains the compatibility with the existing style and also provides a better, escaped format:

  • Treat a 2-comma macro as 3-parameter only when BOTH branches are non-empty; an empty first/middle segment means the comma belongs to SQL_if.
  • Make the comma scan quote-aware ('...', "...", ...) and backslash-aware, so commas inside string literals are never treated as separators.
  • Add \, as an explicit escape for a literal comma in SQL_if.
  • Document the \, escape in the macro autocomplete tooltip.

Adds regression tests for the leading/trailing-comma idioms, the 3-parameter form, quoted commas (single/double/backtick), escaped quotes, and \, escaping.

…ape)

The 2- vs 3-parameter form of $conditionalTest was decided purely by counting
top-level commas, and the comma scan only skipped commas inside parentheses.
Any 2-parameter macro whose SQL_if contained a comma was therefore misparsed
and the comma dropped, producing broken SQL. Affected cases:

  - leading-comma idiom:   $conditionalTest(, expr, $var)
  - trailing-comma idiom:  $conditionalTest(expr,, $var)
  - a comma inside a string literal, e.g. AND t = 'a,b', was treated as an
    argument separator and truncated the query.

Fix:
  - Treat a 2-comma macro as 3-parameter only when BOTH branches are non-empty;
    an empty first/middle segment means the comma belongs to SQL_if.
  - Make the comma scan quote-aware ('...', "...", `...`) and backslash-aware,
    so commas inside string literals are never treated as separators.
  - Add `\,` as an explicit escape for a literal comma in SQL_if.
  - Document the `\,` escape in the macro autocomplete tooltip.

Adds regression tests for the leading/trailing-comma idioms, the 3-parameter
form, quoted commas (single/double/backtick), escaped quotes, and `\,` escaping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant