Skip to content

Commit

Permalink
Fix LimitContext causing too high default limit with pagination
Browse files Browse the repository at this point in the history
  • Loading branch information
webjunkie committed Dec 21, 2023
1 parent 8865be4 commit cc08544
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
4 changes: 2 additions & 2 deletions posthog/clickhouse/client/execute_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ def enqueue_process_query_task(
if bypass_celery:
# Call directly ( for testing )
process_query_task(
team_id, query_id, query_json, limit_context=LimitContext.EXPORT, refresh_requested=refresh_requested
team_id, query_id, query_json, limit_context=LimitContext.QUERY_ASYNC, refresh_requested=refresh_requested
)
else:
task = process_query_task.delay(
team_id, query_id, query_json, limit_context=LimitContext.EXPORT, refresh_requested=refresh_requested
team_id, query_id, query_json, limit_context=LimitContext.QUERY_ASYNC, refresh_requested=refresh_requested
)
query_status.task_id = task.id
manager.store_query_status(query_status)
Expand Down
5 changes: 3 additions & 2 deletions posthog/hogql/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,15 @@

class LimitContext(str, Enum):
QUERY = "query"
QUERY_ASYNC = "query_async"
EXPORT = "export"
COHORT_CALCULATION = "cohort_calculation"


def get_max_limit_for_context(limit_context: LimitContext) -> int:
if limit_context == LimitContext.EXPORT:
return MAX_SELECT_RETURNED_ROWS # 10k
elif limit_context == LimitContext.QUERY:
elif limit_context in (LimitContext.QUERY, LimitContext.QUERY_ASYNC):
return MAX_SELECT_RETURNED_ROWS # 10k
elif limit_context == LimitContext.COHORT_CALCULATION:
return MAX_SELECT_COHORT_CALCULATION_LIMIT # 1b
Expand All @@ -58,7 +59,7 @@ def get_default_limit_for_context(limit_context: LimitContext) -> int:
"""Limit used if no limit is provided"""
if limit_context == LimitContext.EXPORT:
return MAX_SELECT_RETURNED_ROWS # 10k
elif limit_context == LimitContext.QUERY:
elif limit_context in (LimitContext.QUERY, LimitContext.QUERY_ASYNC):
return DEFAULT_RETURNED_ROWS # 100
elif limit_context == LimitContext.COHORT_CALCULATION:
return MAX_SELECT_COHORT_CALCULATION_LIMIT # 1b
Expand Down
6 changes: 3 additions & 3 deletions posthog/hogql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from posthog.client import sync_execute
from posthog.schema import HogQLQueryResponse, HogQLFilters, HogQLQueryModifiers

EXPORT_CONTEXT_MAX_EXECUTION_TIME = 600
INCREASED_MAX_EXECUTION_TIME = 600


def execute_hogql_query(
Expand Down Expand Up @@ -114,8 +114,8 @@ def execute_hogql_query(
)

settings = settings or HogQLGlobalSettings()
if limit_context == LimitContext.EXPORT or limit_context == LimitContext.COHORT_CALCULATION:
settings.max_execution_time = EXPORT_CONTEXT_MAX_EXECUTION_TIME
if limit_context in (LimitContext.EXPORT, LimitContext.COHORT_CALCULATION, LimitContext.QUERY_ASYNC):
settings.max_execution_time = INCREASED_MAX_EXECUTION_TIME

# Print the ClickHouse SQL query
with timings.measure("print_ast"):
Expand Down

0 comments on commit cc08544

Please sign in to comment.