Skip to content

Commit

Permalink
feat(debug): clearly show materialized properties (#20126)
Browse files Browse the repository at this point in the history
  • Loading branch information
mariusandra authored Feb 6, 2024
1 parent 19b52ae commit 0441e2b
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 25 deletions.
15 changes: 14 additions & 1 deletion frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1852,6 +1852,10 @@
"HogQLMetadata": {
"additionalProperties": false,
"properties": {
"debug": {
"description": "Enable more verbose output, usually run from the /debug page",
"type": "boolean"
},
"expr": {
"description": "HogQL expression to validate (use `select` or `expr`, but not both)",
"type": "string"
Expand All @@ -1861,7 +1865,8 @@
"description": "Query within which \"expr\" is validated. Defaults to \"select * from events\""
},
"filters": {
"$ref": "#/definitions/HogQLFilters"
"$ref": "#/definitions/HogQLFilters",
"description": "Extra filters applied to query via {filters}"
},
"kind": {
"const": "HogQLMetadata",
Expand Down Expand Up @@ -2046,6 +2051,10 @@
"limit": {
"type": "integer"
},
"metadata": {
"$ref": "#/definitions/HogQLMetadataResponse",
"description": "Query metadata output"
},
"modifiers": {
"$ref": "#/definitions/HogQLQueryModifiers",
"description": "Modifiers used when performing the query"
Expand Down Expand Up @@ -3365,6 +3374,10 @@
"limit": {
"type": "integer"
},
"metadata": {
"$ref": "#/definitions/HogQLMetadataResponse",
"description": "Query metadata output"
},
"modifiers": {
"$ref": "#/definitions/HogQLQueryModifiers",
"description": "Modifiers used when performing the query"
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ export interface HogQLQueryResponse {
timings?: QueryTiming[]
/** Query explanation output */
explain?: string[]
/** Query metadata output */
metadata?: HogQLMetadataResponse
/** Modifiers used when performing the query */
modifiers?: HogQLQueryModifiers
hasMore?: boolean
Expand Down Expand Up @@ -242,7 +244,10 @@ export interface HogQLMetadata extends DataNode {
exprSource?: AnyDataNode
/** Table to validate the expression against */
table?: string
/** Extra filters applied to query via {filters} */
filters?: HogQLFilters
/** Enable more verbose output, usually run from the /debug page */
debug?: boolean
response?: HogQLMetadataResponse
}

Expand Down
68 changes: 66 additions & 2 deletions frontend/src/scenes/debug/HogQLDebug.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { CodeEditor } from 'lib/components/CodeEditors'
import { CodeSnippet, Language } from 'lib/components/CodeSnippet'
import { LemonLabel } from 'lib/lemon-ui/LemonLabel'
import { LemonSelect } from 'lib/lemon-ui/LemonSelect'
import { LemonTable } from 'lib/lemon-ui/LemonTable'

import { dataNodeLogic, DataNodeLogicProps } from '~/queries/nodes/DataNode/dataNodeLogic'
import { DateRange } from '~/queries/nodes/DataNode/DateRange'
Expand All @@ -17,10 +18,40 @@ interface HogQLDebugProps {
query: HogQLQuery
setQuery: (query: DataNode) => void
}

function toLineColumn(hogql: string, position: number): { line: number; column: number } {
const lines = hogql.split('\n')
let line = 0
let column = 0
for (let i = 0; i < lines.length; i++) {
if (position < lines[i].length) {
line = i + 1
column = position + 1
break
}
position -= lines[i].length + 1
}
return { line, column }
}

function toLine(hogql: string, position: number): number {
return toLineColumn(hogql, position).line
}

function toColumn(hogql: string, position: number): number {
return toLineColumn(hogql, position).column
}

export function HogQLDebug({ query, setQuery, queryKey }: HogQLDebugProps): JSX.Element {
const dataNodeLogicProps: DataNodeLogicProps = { query, key: queryKey }
const { dataLoading, response, responseErrorObject, elapsedTime } = useValues(dataNodeLogic(dataNodeLogicProps))
const clickHouseTime = (response as HogQLQueryResponse)?.timings?.find(({ k }) => k === './clickhouse_execute')?.t
const {
dataLoading,
response: _response,
responseErrorObject,
elapsedTime,
} = useValues(dataNodeLogic(dataNodeLogicProps))
const response = _response as HogQLQueryResponse | null
const clickHouseTime = response?.timings?.find(({ k }) => k === './clickhouse_execute')?.t

return (
<BindLogic logic={dataNodeLogic} props={dataNodeLogicProps}>
Expand Down Expand Up @@ -141,6 +172,39 @@ export function HogQLDebug({ query, setQuery, queryKey }: HogQLDebugProps): JSX.
</CodeSnippet>
</>
) : null}
{response?.metadata ? (
<>
<h2>Metadata</h2>
<LemonTable
dataSource={[
...response.metadata.errors.map((error) => ({
type: 'error',
line: toLine(response.hogql ?? '', error.start ?? 0),
column: toColumn(response.hogql ?? '', error.start ?? 0),
...error,
})),
...response.metadata.warnings.map((warn) => ({
type: 'warning',
line: toLine(response.hogql ?? '', warn.start ?? 0),
column: toColumn(response.hogql ?? '', warn.start ?? 0),
...warn,
})),
...response.metadata.notices.map((notice) => ({
type: 'notice',
line: toLine(response.hogql ?? '', notice.start ?? 0),
column: toColumn(response.hogql ?? '', notice.start ?? 0),
...notice,
})),
].sort((a, b) => (a.start ?? 0) - (b.start ?? 0))}
columns={[
{ title: 'Line', dataIndex: 'line', key: 'line', width: '40px' },
{ title: 'Column', dataIndex: 'column', key: 'column', width: '40px' },
{ title: 'Type', dataIndex: 'type', key: 'type', width: '80px' },
{ title: 'Message', dataIndex: 'message', key: 'message' },
]}
/>
</>
) : null}
{response?.explain ? (
<>
<h2>Explained ClickHouseSQL</h2>
Expand Down
21 changes: 10 additions & 11 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,6 @@ posthog/hogql/filters.py:0: note: PEP 484 prohibits implicit Optional. According
posthog/hogql/filters.py:0: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
posthog/hogql/query.py:0: error: Incompatible types in assignment (expression has type "None", variable has type "str | SelectQuery | SelectUnionQuery") [assignment]
posthog/hogql/query.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "SelectQuery | SelectUnionQuery") [assignment]
posthog/hogql/query.py:0: error: Argument 2 to "replace_filters" has incompatible type "HogQLFilters | None"; expected "HogQLFilters" [arg-type]
posthog/hogql/query.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "SelectQuery | SelectUnionQuery") [assignment]
posthog/hogql/query.py:0: error: Argument 1 to "get_default_limit_for_context" has incompatible type "LimitContext | None"; expected "LimitContext" [arg-type]
posthog/hogql/query.py:0: error: "SelectQuery" has no attribute "select_queries" [attr-defined]
Expand Down Expand Up @@ -788,6 +787,16 @@ posthog/api/property_definition.py:0: error: Item "AnonymousUser" of "User | Ano
posthog/api/property_definition.py:0: error: Item "None" of "Organization | Any | None" has no attribute "is_feature_available" [union-attr]
posthog/api/dashboards/dashboard_templates.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/temporal/tests/batch_exports/test_batch_exports.py:0: error: TypedDict key must be a string literal; expected one of ("_timestamp", "created_at", "distinct_id", "elements", "elements_chain", ...) [literal-required]
posthog/queries/app_metrics/test/test_app_metrics.py:0: error: Argument 3 to "AppMetricsErrorDetailsQuery" has incompatible type "AppMetricsRequestSerializer"; expected "AppMetricsErrorsRequestSerializer" [arg-type]
posthog/queries/app_metrics/test/test_app_metrics.py:0: error: Argument 3 to "AppMetricsErrorDetailsQuery" has incompatible type "AppMetricsRequestSerializer"; expected "AppMetricsErrorsRequestSerializer" [arg-type]
posthog/queries/app_metrics/test/test_app_metrics.py:0: error: Argument 3 to "AppMetricsErrorDetailsQuery" has incompatible type "AppMetricsRequestSerializer"; expected "AppMetricsErrorsRequestSerializer" [arg-type]
posthog/batch_exports/http.py:0: error: Unsupported right operand type for in ("object") [operator]
posthog/api/plugin.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/api/plugin.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/warehouse/external_data_source/source.py:0: error: Incompatible types in assignment (expression has type "int", target has type "str") [assignment]
posthog/warehouse/external_data_source/source.py:0: error: Incompatible types in assignment (expression has type "int", target has type "str") [assignment]
posthog/warehouse/external_data_source/source.py:0: error: Incompatible types in assignment (expression has type "dict[str, Collection[str]]", variable has type "StripeSourcePayload") [assignment]
posthog/warehouse/external_data_source/source.py:0: error: Argument 1 to "_create_source" has incompatible type "StripeSourcePayload"; expected "dict[Any, Any]" [arg-type]
posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py:0: error: Need type annotation for "_execute_calls" (hint: "_execute_calls: List[<type>] = ...") [var-annotated]
posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py:0: error: Need type annotation for "_execute_async_calls" (hint: "_execute_async_calls: List[<type>] = ...") [var-annotated]
posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py:0: error: Need type annotation for "_cursors" (hint: "_cursors: List[<type>] = ...") [var-annotated]
Expand Down Expand Up @@ -854,16 +863,6 @@ posthog/temporal/tests/batch_exports/test_s3_batch_export_workflow.py:0: error:
posthog/temporal/tests/batch_exports/test_backfill_batch_export.py:0: error: Argument "name" to "acreate_batch_export" has incompatible type "object"; expected "str" [arg-type]
posthog/temporal/tests/batch_exports/test_backfill_batch_export.py:0: error: Argument "destination_data" to "acreate_batch_export" has incompatible type "object"; expected "dict[Any, Any]" [arg-type]
posthog/temporal/tests/batch_exports/test_backfill_batch_export.py:0: error: Argument "interval" to "acreate_batch_export" has incompatible type "object"; expected "str" [arg-type]
posthog/queries/app_metrics/test/test_app_metrics.py:0: error: Argument 3 to "AppMetricsErrorDetailsQuery" has incompatible type "AppMetricsRequestSerializer"; expected "AppMetricsErrorsRequestSerializer" [arg-type]
posthog/queries/app_metrics/test/test_app_metrics.py:0: error: Argument 3 to "AppMetricsErrorDetailsQuery" has incompatible type "AppMetricsRequestSerializer"; expected "AppMetricsErrorsRequestSerializer" [arg-type]
posthog/queries/app_metrics/test/test_app_metrics.py:0: error: Argument 3 to "AppMetricsErrorDetailsQuery" has incompatible type "AppMetricsRequestSerializer"; expected "AppMetricsErrorsRequestSerializer" [arg-type]
posthog/batch_exports/http.py:0: error: Unsupported right operand type for in ("object") [operator]
posthog/api/plugin.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/api/plugin.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/warehouse/external_data_source/source.py:0: error: Incompatible types in assignment (expression has type "int", target has type "str") [assignment]
posthog/warehouse/external_data_source/source.py:0: error: Incompatible types in assignment (expression has type "int", target has type "str") [assignment]
posthog/warehouse/external_data_source/source.py:0: error: Incompatible types in assignment (expression has type "dict[str, Collection[str]]", variable has type "StripeSourcePayload") [assignment]
posthog/warehouse/external_data_source/source.py:0: error: Argument 1 to "_create_source" has incompatible type "StripeSourcePayload"; expected "dict[Any, Any]" [arg-type]
posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py:0: error: Unsupported left operand type for + ("None") [operator]
posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py:0: note: Left operand is of type "Any | None"
posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py:0: error: Incompatible types in assignment (expression has type "str | int", variable has type "int") [assignment]
Expand Down
2 changes: 2 additions & 0 deletions posthog/hogql/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class HogQLContext:
timings: HogQLTimings = field(default_factory=HogQLTimings)
# Modifications requested by the HogQL client
modifiers: HogQLQueryModifiers = field(default_factory=HogQLQueryModifiers)
# Enables more verbose output for debugging
debug: bool = False

def add_value(self, value: Any) -> str:
key = f"hogql_val_{len(self.values)}"
Expand Down
2 changes: 1 addition & 1 deletion posthog/hogql/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from posthog.utils import relative_date_parse


def replace_filters(node: ast.Expr, filters: HogQLFilters, team: Team) -> ast.Expr:
def replace_filters(node: ast.Expr, filters: Optional[HogQLFilters], team: Team) -> ast.Expr:
return ReplaceFilters(filters, team).visit(node)


Expand Down
3 changes: 2 additions & 1 deletion posthog/hogql/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def get_hogql_metadata(

try:
if isinstance(query.expr, str):
context = HogQLContext(team_id=team.pk, modifiers=query_modifiers)
context = HogQLContext(team_id=team.pk, modifiers=query_modifiers, debug=query.debug or False)
if query.exprSource is not None:
source_query = get_query_runner(query.exprSource, team).to_query()
translate_hogql(query.expr, context=context, metadata_source=source_query)
Expand All @@ -41,6 +41,7 @@ def get_hogql_metadata(
team_id=team.pk,
modifiers=query_modifiers,
enable_select_queries=True,
debug=query.debug or False,
)

select_ast = parse_select(query.select)
Expand Down
8 changes: 7 additions & 1 deletion posthog/hogql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from posthog.models.team import Team
from posthog.clickhouse.query_tagging import tag_queries
from posthog.client import sync_execute
from posthog.schema import HogQLQueryResponse, HogQLFilters, HogQLQueryModifiers
from posthog.schema import HogQLQueryResponse, HogQLFilters, HogQLQueryModifiers, HogQLMetadata, HogQLMetadataResponse

INCREASED_MAX_EXECUTION_TIME = 600

Expand Down Expand Up @@ -169,6 +169,7 @@ def execute_hogql_query(
else:
raise e

metadata: Optional[HogQLMetadataResponse] = None
if explain and error is None: # If the query errored, explain will fail as well.
with timings.measure("explain"):
explain_results = sync_execute(
Expand All @@ -180,6 +181,10 @@ def execute_hogql_query(
readonly=True,
)
explain_output = [str(r[0]) for r in explain_results[0]]
with timings.measure("metadata"):
from posthog.hogql.metadata import get_hogql_metadata

metadata = get_hogql_metadata(HogQLMetadata(select=hogql, debug=True), team)
else:
explain_output = None

Expand All @@ -194,4 +199,5 @@ def execute_hogql_query(
types=types,
modifiers=query_modifiers,
explain=explain_output,
metadata=metadata,
)
45 changes: 41 additions & 4 deletions posthog/hogql/test/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
class TestMetadata(ClickhouseTestMixin, APIBaseTest):
maxDiff = None

def _expr(self, query: str, table: str = "events") -> HogQLMetadataResponse:
def _expr(self, query: str, table: str = "events", debug=True) -> HogQLMetadataResponse:
return get_hogql_metadata(
query=HogQLMetadata(
kind="HogQLMetadata",
expr=query,
exprSource=HogQLQuery(kind="HogQLQuery", query=f"select * from {table}"),
response=None,
debug=debug,
),
team=self.team,
)
Expand Down Expand Up @@ -190,7 +191,7 @@ def test_metadata_in_cohort(self):
},
)

def test_metadata_property_type_notice(self):
def test_metadata_property_type_notice_debug(self):
try:
from ee.clickhouse.materialized_columns.analyze import materialize
except ModuleNotFoundError:
Expand All @@ -211,13 +212,49 @@ def test_metadata_property_type_notice(self):
"inputSelect": None,
"notices": [
{
"message": "Event property 'string' is of type 'String'",
"message": "Event property 'string' is of type 'String'. This property is not materialized 🐢.",
"start": 11,
"end": 17,
"fix": None,
},
{
"message": "⚡️Event property 'number' is of type 'Float'",
"message": "Event property 'number' is of type 'Float'. This property is materialized ⚡️.",
"start": 32,
"end": 38,
"fix": None,
},
],
},
)

def test_metadata_property_type_notice_no_debug(self):
try:
from ee.clickhouse.materialized_columns.analyze import materialize
except ModuleNotFoundError:
# EE not available? Assume we're good
self.assertEqual(1 + 2, 3)
return
materialize("events", "number")

PropertyDefinition.objects.create(team=self.team, name="string", property_type="String")
PropertyDefinition.objects.create(team=self.team, name="number", property_type="Numeric")
metadata = self._expr("properties.string || properties.number", debug=False)
self.assertEqual(
metadata.dict(),
metadata.dict()
| {
"isValid": True,
"inputExpr": "properties.string || properties.number",
"inputSelect": None,
"notices": [
{
"message": "Event property 'string' is of type 'String'.",
"start": 11,
"end": 17,
"fix": None,
},
{
"message": "Event property 'number' is of type 'Float'.",
"start": 32,
"end": 38,
"fix": None,
Expand Down
Loading

0 comments on commit 0441e2b

Please sign in to comment.