Repository: tobymao/sqlglot

Issue:
two-argument version of clickhouse's `countIf` function incorrectly fails to parse
This used to work:

```
In [3]: import sqlglot as sg

In [4]: sg.__version__
Out[4]: '20.7.1'

In [5]: sg.parse_one("select countIf(x, y)", read="clickhouse")
...
ParseError: The number of provided arguments (2) is greater than the maximum number of supported arguments (1). Line 1, Col: 20.
  select countIf(x, y)
```

ClickHouse CLI:

```
localhost :) select countIf(1, false), countIf(1, true);

SELECT
    countIf(1, false),
    countIf(1, true)

Query id: cd5dea26-fae2-4bdc-a18b-edbf5b910fda

┌─countIf(1, false)─┬─countIf(1, true)─┐
│                 0 │                1 │
└───────────────────┴──────────────────┘
```


Unfortunately the clickhouse docs on `countIf`'s signature are nonexistent, with only a single mention of the function in the `If` combinators section here https://clickhouse.com/docs/en/sql-reference/aggregate-functions/combinators#-if

Discussion:
georgesittas: Could also just `pop` the `COUNTIF` entry from `FUNCTIONS` here and let all `countIf` variants be parsed as a `CombinedAggFunc`. The reason I overrode the entry was to enable the transpilation of the single-arg variant towards other dialects.
tobymao: can we add a test that checks every single one of these combinator functions to make sure they accept multiple args?
georgesittas: Sure, how about a for loop over [`AGG_FUNC_MAPPING`](https://github.com/tobymao/sqlglot/blob/main/sqlglot/dialects/clickhouse.py#L240) that ensures we roundtrip correctly if we call the function with, say, 4 arguments (`func(x, y, z, w)`)?
tobymao: sure, would it have caught this one?
georgesittas: > sure, would it have caught this one?

No, because the way Clickhouse's parser works is that it calls the superclass' `_parse_function` and then only constructs the Clickhouse-specific combined / parameterized expressions off of it if the result is an `Anonymous`. In this case `COUNTIF` was being parsed as `exp.CountIf` (which was a side-effect of https://github.com/tobymao/sqlglot/commit/2a94f2ba0d93ee1454a2d19cc8577e211fd5cbe0), so it would not hit that logic.

Tbh thinking about this again I'm not sure if it's gonna be worth the overhead of testing this because most functions in the mapping I mentioned are very Clickhouse-specific and they should already be parsed correctly (🤞) - `countIf` seems like an exception.
tobymao: ok looks good then

Patch:
diff --git a/sqlglot/dialects/clickhouse.py b/sqlglot/dialects/clickhouse.py
index eb3be0a9..795a04e5 100644
--- a/sqlglot/dialects/clickhouse.py
+++ b/sqlglot/dialects/clickhouse.py
@@ -23,16 +23,25 @@ def _lower_func(sql: str) -> str:
     return sql[:index].lower() + sql[index:]
 
 
-def _quantile_sql(self, e):
+def _quantile_sql(self: ClickHouse.Generator, e: exp.Quantile) -> str:
     quantile = e.args["quantile"]
     args = f"({self.sql(e, 'this')})"
+
     if isinstance(quantile, exp.Array):
         func = self.func("quantiles", *quantile)
     else:
         func = self.func("quantile", quantile)
+
     return func + args
 
 
+def _parse_count_if(args: t.List) -> exp.CountIf | exp.CombinedAggFunc:
+    if len(args) == 1:
+        return exp.CountIf(this=seq_get(args, 0))
+
+    return exp.CombinedAggFunc(this="countIf", expressions=args, parts=("count", "If"))
+
+
 class ClickHouse(Dialect):
     NORMALIZE_FUNCTIONS: bool | str = False
     NULL_ORDERING = "nulls_are_last"
@@ -92,6 +101,7 @@ class ClickHouse(Dialect):
         FUNCTIONS = {
             **parser.Parser.FUNCTIONS,
             "ANY": exp.AnyValue.from_arg_list,
+            "COUNTIF": _parse_count_if,
             "DATE_ADD": lambda args: exp.DateAdd(
                 this=seq_get(args, 2), expression=seq_get(args, 1), unit=seq_get(args, 0)
             ),
@@ -542,6 +552,7 @@ class ClickHouse(Dialect):
             exp.ArgMin: arg_max_or_min_no_count("argMin"),
             exp.Array: inline_array_sql,
             exp.CastToStrType: rename_func("CAST"),
+            exp.CountIf: rename_func("countIf"),
             exp.CurrentDate: lambda self, e: self.func("CURRENT_DATE"),
             exp.DateAdd: date_delta_sql("DATE_ADD"),
             exp.DateDiff: date_delta_sql("DATE_DIFF"),


Test patch:
diff --git a/tests/dialects/test_bigquery.py b/tests/dialects/test_bigquery.py
index 6844239e..71f2c196 100644
--- a/tests/dialects/test_bigquery.py
+++ b/tests/dialects/test_bigquery.py
@@ -292,8 +292,13 @@ class TestBigQuery(Validator):
         )
         self.validate_all(
             "SELECT COUNTIF(x)",
+            read={
+                "clickhouse": "SELECT countIf(x)",
+                "duckdb": "SELECT COUNT_IF(x)",
+            },
             write={
                 "bigquery": "SELECT COUNTIF(x)",
+                "clickhouse": "SELECT countIf(x)",
                 "duckdb": "SELECT COUNT_IF(x)",
             },
         )
diff --git a/tests/dialects/test_clickhouse.py b/tests/dialects/test_clickhouse.py
index fa40264c..2dfcad5c 100644
--- a/tests/dialects/test_clickhouse.py
+++ b/tests/dialects/test_clickhouse.py
@@ -26,6 +26,7 @@ class TestClickhouse(Validator):
         self.assertEqual(expr.sql(dialect="clickhouse"), "COUNT(x)")
         self.assertIsNone(expr._meta)
 
+        self.validate_identity("countIf(x, y)")
         self.validate_identity("x = y")
         self.validate_identity("x <> y")
         self.validate_identity("SELECT * FROM (SELECT a FROM b SAMPLE 0.01)")