Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions posthog/hogql/printer/clickhouse.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,26 @@ def team_id_guard_for_table(table_type: ast.TableOrSelectType, context: HogQLCon
)


# String functions that return an Array — a Nullable string arg would produce an
# illegal Nullable(Array(...)) type, so their nullable string args are coerced.
_ARRAY_RETURNING_STRING_FUNCTIONS = frozenset(
{
"splitByChar",
"splitByString",
"splitByRegexp",
"splitByWhitespace",
"splitByNonAlpha",
"alphaTokens",
"extractAll",
"extractAllGroups",
"extractAllGroupsHorizontal",
"extractAllGroupsVertical",
"extractGroups",
"ngrams",
"tokens",
}
)

# In non-nullable materialized columns, these values are treated as NULL
MAT_COL_NULL_SENTINELS = ["", "null"]

Expand Down Expand Up @@ -155,6 +175,25 @@ def _render_function_call(self, node: ast.Call, func_meta) -> str:
args.append(f"ifNull({self.visit(arg)}, '')")
else:
args.append(f"ifNull(toString({self.visit(arg)}), '')")
elif node.name in _ARRAY_RETURNING_STRING_FUNCTIONS:
# 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
# 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)
else:
args = [self.visit(arg) for arg in node_args]

Expand Down
32 changes: 32 additions & 0 deletions posthog/hogql/printer/test/test_printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -4813,6 +4813,38 @@ def test_can_call_parametric_function(self):
)
assert query_response.results == [(6,)]

@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.clickhouse is not None
assert "ifNull(" not in query_response.clickhouse


class TestPostgresPrinter(BaseTest):
maxDiff = None
Expand Down
Loading