chore(insights): address review feedback

- add extractAll/extractAllGroupsHorizontal/extractAllGroupsVertical/extractGroups to coerced functions
- exclude StringArrayType (not StringJSONType) from nullable string coercion
- parameterize the nullable-arg test across all array-returning functions, assert ifNull is emitted
- add a non-nullable-arg case asserting no spurious ifNull

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sampennington

posthog/hogql/printer/clickhouse.py

@@ -81,7 +81,11 @@ def team_id_guard_for_table(table_type: ast.TableOrSelectType, context: HogQLCon
         "splitByWhitespace",
         "splitByNonAlpha",
         "alphaTokens",
+        "extractAll",
         "extractAllGroups",
+        "extractAllGroupsHorizontal",
+        "extractAllGroupsVertical",
+        "extractGroups",
         "ngrams",
         "tokens",
     }
@@ -172,14 +176,21 @@ def _render_function_call(self, node: ast.Call, func_meta) -> str:
                 else:
                     args.append(f"ifNull(toString({self.visit(arg)}), '')")
         elif node.name in _ARRAY_RETURNING_STRING_FUNCTIONS:
-            # These return Array(...). A Nullable string argument would make the
-            # result Nullable(Array(...)), which ClickHouse rejects, so coerce
-            # nullable string args to a non-nullable empty string.
+            # A Nullable string arg would make the result Nullable(Array(...)),
+            # which ClickHouse rejects, so coerce nullable string args to a
+            # non-nullable empty string. Coercion is position-agnostic on
+            # purpose — a nullable separator would be just as illegal.
             args = []
             for arg in node_args:
                 visited = self.visit(arg)
                 arg_type = arg.type.resolve_constant_type(self.context) if arg.type is not None else None
-                if isinstance(arg_type, ast.StringType) and arg_type.nullable:
+                # StringArrayType is a StringType subclass but resolves to an
+                # Array — coercing it to a bare '' would be invalid, so exclude it.
+                if (
+                    isinstance(arg_type, ast.StringType)
+                    and not isinstance(arg_type, ast.StringArrayType)
+                    and arg_type.nullable
+                ):
                     args.append(f"ifNull({visited}, '')")
                 else:
                     args.append(visited)

posthog/hogql/printer/test/test_printer.py

@@ -4813,11 +4813,37 @@ def test_can_call_parametric_function(self):
         )
         assert query_response.results == [(6,)]
 
-    def test_split_function_on_nullable_property(self):
-        # splitByChar on a nullable property would produce an illegal Nullable(Array)
-        query = parse_select("SELECT splitByChar('@', properties.email) FROM events")
+    @parameterized.expand(
+        [
+            ("splitByChar", "splitByChar('@', properties.email)"),
+            ("splitByString", "splitByString('@@', properties.email)"),
+            ("splitByRegexp", "splitByRegexp('[^a-z]+', properties.email)"),
+            ("splitByWhitespace", "splitByWhitespace(properties.email)"),
+            ("splitByNonAlpha", "splitByNonAlpha(properties.email)"),
+            ("alphaTokens", "alphaTokens(properties.email)"),
+            ("ngrams", "ngrams(properties.email, 3)"),
+            ("tokens", "tokens(properties.email)"),
+            ("extractAll", "extractAll(properties.email, '[a-z]+')"),
+            ("extractAllGroups", "extractAllGroups(properties.email, '([a-z]+)')"),
+            ("extractAllGroupsHorizontal", "extractAllGroupsHorizontal(properties.email, '([a-z]+)')"),
+            ("extractAllGroupsVertical", "extractAllGroupsVertical(properties.email, '([a-z]+)')"),
+            ("extractGroups", "extractGroups(properties.email, '([a-z]+)')"),
+        ]
+    )
+    def test_array_returning_string_function_on_nullable_property(self, _name, func_call):
+        # A nullable string arg to an array-returning function would produce an
+        # illegal Nullable(Array(...)) — ClickHouse rejects it (exception_code=43).
+        query = parse_select(f"SELECT {func_call} FROM events")
+        query_response = execute_hogql_query(team=self.team, query=query)
+        assert query_response.clickhouse is not None
+        assert "ifNull(" in query_response.clickhouse
+
+    def test_array_returning_string_function_no_spurious_ifnull_on_constant(self):
+        # A non-nullable (constant) string arg must not be wrapped in ifNull.
+        query = parse_select("SELECT splitByChar('@', 'a@b') FROM events")
         query_response = execute_hogql_query(team=self.team, query=query)
-        assert query_response.results is not None
+        assert query_response.clickhouse is not None
+        assert "ifNull(" not in query_response.clickhouse
 
 
 class TestPostgresPrinter(BaseTest):