Skip to content

Commit

Permalink
fix(hogql): Use project default mode value in toStartOfWeek (#17777)
Browse files Browse the repository at this point in the history
  • Loading branch information
Twixes authored Oct 4, 2023
1 parent be49959 commit b87d907
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 7 deletions.
5 changes: 3 additions & 2 deletions posthog/hogql/metadata.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.conf import settings
from posthog.hogql.context import HogQLContext
from posthog.hogql.errors import HogQLException
from posthog.hogql.filters import replace_filters
Expand Down Expand Up @@ -53,9 +54,9 @@ def get_hogql_metadata(
if "mismatched input '<EOF>' expecting" in error:
error = "Unexpected end of query"
response.errors.append(HogQLNotice(message=error, start=e.start, end=e.end))
else:
elif not settings.DEBUG:
# We don't want to accidentally expose too much data via errors
response.errors.append(HogQLNotice(message=f"Unexpected f{e.__class__.__name__}"))
response.errors.append(HogQLNotice(message=f"Unexpected {e.__class__.__name__}"))

return response

Expand Down
2 changes: 1 addition & 1 deletion posthog/hogql/printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ def visit_call(self, node: ast.Call):
if node.name == "toStartOfWeek" and len(node.args) == 1:
# If week mode hasn't been specified, use the project's default.
# For Monday-based weeks mode 3 is used (which is ISO 8601), for Sunday-based mode 0 (CH default)
args.insert(1, self._get_week_start_day().clickhouse_mode)
args.insert(1, WeekStartDay(self._get_week_start_day()).clickhouse_mode)

params = [self.visit(param) for param in node.params] if node.params is not None else None

Expand Down
10 changes: 8 additions & 2 deletions posthog/hogql/test/test_printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,15 @@ def test_concat_pipes(self):
)

def test_to_start_of_week_gets_mode(self):
sunday_week_context = HogQLContext(team_id=self.team.pk, database=Database(None, WeekStartDay.SUNDAY))
monday_week_context = HogQLContext(team_id=self.team.pk, database=Database(None, WeekStartDay.MONDAY))
# It's important we use ints and not WeekStartDay here, because it's the former that's actually in the DB
default_week_context = HogQLContext(team_id=self.team.pk, database=Database(None, None))
sunday_week_context = HogQLContext(team_id=self.team.pk, database=Database(None, 0)) # 0 == WeekStartDay.SUNDAY
monday_week_context = HogQLContext(team_id=self.team.pk, database=Database(None, 1)) # 1 == WeekStartDay.MONDAY

self.assertEqual(
self._expr("toStartOfWeek(timestamp)", default_week_context), # Sunday is the default
f"toStartOfWeek(toTimeZone(events.timestamp, %(hogql_val_0)s), 0)",
)
self.assertEqual(
self._expr("toStartOfWeek(timestamp)"), # Sunday is the default
f"toStartOfWeek(toTimeZone(events.timestamp, %(hogql_val_0)s), 0)",
Expand Down
6 changes: 4 additions & 2 deletions posthog/warehouse/api/saved_query.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.conf import settings
from posthog.permissions import OrganizationMemberPermissions
from rest_framework.exceptions import NotAuthenticated
from rest_framework.permissions import IsAuthenticated
Expand Down Expand Up @@ -69,8 +70,9 @@ def validate_query(self, query):
if isinstance(err, ValueError) or isinstance(err, HogQLException):
error = str(err)
raise exceptions.ValidationError(detail=f"Invalid query: {error}")
else:
raise exceptions.ValidationError(detail=f"Unexpected f{err.__class__.__name__}")
elif not settings.DEBUG:
# We don't want to accidentally expose too much data via errors
raise exceptions.ValidationError(detail=f"Unexpected {err.__class__.__name__}")

return query

Expand Down

0 comments on commit b87d907

Please sign in to comment.