Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(data-warehouse): Allowed saving of views without querying again #26580

Merged
merged 3 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2230,15 +2230,15 @@ const api = {
async get(viewId: DataWarehouseSavedQuery['id']): Promise<DataWarehouseSavedQuery> {
return await new ApiRequest().dataWarehouseSavedQuery(viewId).get()
},
async create(data: Partial<DataWarehouseSavedQuery>): Promise<DataWarehouseSavedQuery> {
async create(data: Partial<DataWarehouseSavedQuery> & { types: string[][] }): Promise<DataWarehouseSavedQuery> {
return await new ApiRequest().dataWarehouseSavedQueries().create({ data })
},
async delete(viewId: DataWarehouseSavedQuery['id']): Promise<void> {
await new ApiRequest().dataWarehouseSavedQuery(viewId).delete()
},
async update(
viewId: DataWarehouseSavedQuery['id'],
data: Partial<DataWarehouseSavedQuery>
data: Partial<DataWarehouseSavedQuery> & { types: string[][] }
): Promise<DataWarehouseSavedQuery> {
return await new ApiRequest().dataWarehouseSavedQuery(viewId).update({ data })
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ function InternalDataTableVisualization(props: DataTableVisualizationProps): JSX
<>
<HogQLQueryEditor
query={query.source}
queryResponse={response ?? undefined}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my finest hour, but I wasn't expecting the "saved query" functionality to be tied to the HogQLQueryEditor component. Not sure on a better way to do this without tearing apart this component and logic 🤷

setQuery={setQuerySource}
embedded
onChange={setEditorQuery}
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/queries/nodes/HogQLQuery/HogQLQueryEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface HogQLQueryEditorProps {
onChange?: (query: string) => void
embedded?: boolean
editorFooter?: (hasErrors: boolean, errors: string | null, isValidView: boolean) => JSX.Element
queryResponse?: Record<string, any>
}

let uniqueNode = 0
Expand Down Expand Up @@ -60,6 +61,7 @@ export function HogQLQueryEditor(props: HogQLQueryEditorProps): JSX.Element {
key,
editor,
monaco,
queryResponse: props.queryResponse,
}
const logic = hogQLQueryEditorLogic(hogQLQueryEditorLogicProps)
const { queryInput, prompt, aiAvailable, promptError, promptLoading, multitab } = useValues(logic)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface HogQLQueryEditorLogicProps {
monaco?: Monaco | null
editor?: editor.IStandaloneCodeEditor | null
metadataSource?: DataNode
queryResponse?: Record<string, any>
}

export const hogQLQueryEditorLogic = kea<hogQLQueryEditorLogicType>([
Expand Down Expand Up @@ -139,10 +140,13 @@ export const hogQLQueryEditorLogic = kea<hogQLQueryEditorLogicType>([
kind: NodeKind.HogQLQuery,
query: values.queryInput,
}
await dataWarehouseViewsLogic.asyncActions.createDataWarehouseSavedQuery({ name, query })
const types = props.queryResponse?.types ?? []

await dataWarehouseViewsLogic.asyncActions.createDataWarehouseSavedQuery({ name, query, types })
},
onUpdateView: async () => {
actions.updateView(values.queryInput)
const types = props.queryResponse?.types ?? []
actions.updateView(values.queryInput, types)
},
})),
])
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ export const dataWarehouseViewsLogic = kea<dataWarehouseViewsLogicType>([
}
return savedQueries.results
},
createDataWarehouseSavedQuery: async (view: Partial<DatabaseSchemaViewTable>) => {
createDataWarehouseSavedQuery: async (
view: Partial<DatabaseSchemaViewTable> & { types: string[][] }
) => {
const newView = await api.dataWarehouseSavedQueries.create(view)

lemonToast.success(`${newView.name ?? 'View'} successfully created`)
Expand All @@ -46,7 +48,9 @@ export const dataWarehouseViewsLogic = kea<dataWarehouseViewsLogicType>([
await api.dataWarehouseSavedQueries.delete(viewId)
return values.dataWarehouseSavedQueries.filter((view) => view.id !== viewId)
},
updateDataWarehouseSavedQuery: async (view: Partial<DatabaseSchemaViewTable> & { id: string }) => {
updateDataWarehouseSavedQuery: async (
view: Partial<DatabaseSchemaViewTable> & { id: string; types: string[][] }
) => {
const newView = await api.dataWarehouseSavedQueries.update(view.id, view)
return values.dataWarehouseSavedQueries.map((savedQuery) => {
if (savedQuery.id === view.id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import {
DatabaseSchemaMaterializedViewTable,
DatabaseSchemaTable,
DatabaseSchemaViewTable,
DatabaseSerializedFieldType,
HogQLQuery,
NodeKind,
Expand Down Expand Up @@ -55,7 +56,7 @@
deleteDataWarehouseTable: (tableId: string) => ({ tableId }),
toggleSchemaModal: true,
setEditingView: (id: string | null) => ({ id }),
updateView: (query: string) => ({ query }),
updateView: (query: string, types: string[][]) => ({ query, types }),
})),
reducers({
selectedRow: [
Expand Down Expand Up @@ -278,18 +279,19 @@
})
}
},
updateView: ({ query }) => {
updateView: ({ query, types }) => {
if (values.editingView) {
const newViewQuery: HogQLQuery = {
kind: NodeKind.HogQLQuery,
query: query,
}
const oldView = values.viewsMapById[values.editingView]
const newView = {
const newView: DatabaseSchemaViewTable = {
...oldView,
query: newViewQuery,
types,

Check failure on line 292 in frontend/src/scenes/data-warehouse/settings/dataWarehouseSceneLogic.ts

View workflow job for this annotation

GitHub Actions / Code quality checks

Type '{ query: HogQLQuery; types: string[][]; type: "view"; id: string; name: string; fields: Record<string, DatabaseSchemaField>; }' is not assignable to type 'DatabaseSchemaViewTable'.
}
actions.updateDataWarehouseSavedQuery(newView)

Check failure on line 294 in frontend/src/scenes/data-warehouse/settings/dataWarehouseSceneLogic.ts

View workflow job for this annotation

GitHub Actions / Code quality checks

Argument of type 'DatabaseSchemaViewTable' is not assignable to parameter of type 'Partial<DatabaseSchemaViewTable> & { id: string; types: string[][]; }'.
}
},
})),
Expand Down
6 changes: 6 additions & 0 deletions posthog/hogql/constants.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from datetime import date, datetime
from enum import StrEnum
import sys
from typing import Optional, Literal, TypeAlias
from uuid import UUID
from pydantic import ConfigDict, BaseModel
Expand Down Expand Up @@ -53,6 +54,7 @@ class LimitContext(StrEnum):
EXPORT = "export"
COHORT_CALCULATION = "cohort_calculation"
HEATMAPS = "heatmaps"
SAVED_QUERY = "saved_query"


def get_max_limit_for_context(limit_context: LimitContext) -> int:
Expand All @@ -62,6 +64,8 @@ def get_max_limit_for_context(limit_context: LimitContext) -> int:
return MAX_SELECT_HEATMAPS_LIMIT # 1M
elif limit_context == LimitContext.COHORT_CALCULATION:
return MAX_SELECT_COHORT_CALCULATION_LIMIT # 1b
elif limit_context == LimitContext.SAVED_QUERY:
return sys.maxsize # Max python int
else:
raise ValueError(f"Unexpected LimitContext value: {limit_context}")

Expand All @@ -76,6 +80,8 @@ def get_default_limit_for_context(limit_context: LimitContext) -> int:
return MAX_SELECT_HEATMAPS_LIMIT # 1M
elif limit_context == LimitContext.COHORT_CALCULATION:
return MAX_SELECT_COHORT_CALCULATION_LIMIT # 1b
elif limit_context == LimitContext.SAVED_QUERY:
return sys.maxsize # Max python int
else:
raise ValueError(f"Unexpected LimitContext value: {limit_context}")

Expand Down
6 changes: 4 additions & 2 deletions posthog/temporal/data_modeling/run_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from django.conf import settings
from dlt.common.libs.deltalake import get_delta_tables

from posthog.hogql.constants import HogQLGlobalSettings
from posthog.hogql.constants import HogQLGlobalSettings, LimitContext
from posthog.hogql.database.database import create_hogql_database
from posthog.hogql.query import execute_hogql_query
from posthog.models import Team
Expand Down Expand Up @@ -347,7 +347,9 @@ def hogql_table(query: str, team: Team, table_name: str, table_columns: dlt_typi
async def get_hogql_rows():
settings = HogQLGlobalSettings(max_execution_time=60 * 10) # 10 mins, same as the /query endpoint async workers

response = await asyncio.to_thread(execute_hogql_query, query, team, settings=settings)
response = await asyncio.to_thread(
execute_hogql_query, query, team, settings=settings, limit_context=LimitContext.SAVED_QUERY
)

if not response.columns:
raise EmptyHogQLResponseColumnsError()
Expand Down
23 changes: 21 additions & 2 deletions posthog/warehouse/api/saved_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@
from posthog.hogql.printer import print_ast
from posthog.temporal.common.client import sync_connect
from posthog.temporal.data_modeling.run_workflow import RunWorkflowInputs, Selector
from posthog.warehouse.models import DataWarehouseJoin, DataWarehouseModelPath, DataWarehouseSavedQuery
from posthog.warehouse.models import (
CLICKHOUSE_HOGQL_MAPPING,
DataWarehouseJoin,
DataWarehouseModelPath,
DataWarehouseSavedQuery,
clean_type,
)
import uuid


Expand Down Expand Up @@ -70,7 +76,20 @@ def create(self, validated_data):
view = DataWarehouseSavedQuery(**validated_data)
# The columns will be inferred from the query
try:
view.columns = view.get_columns()
client_types = self.context["request"].data.get("types", [])
if len(client_types) == 0:
view.columns = view.get_columns()
else:
columns = {
str(item[0]): {
"hogql": CLICKHOUSE_HOGQL_MAPPING[clean_type(str(item[1]))].__name__,
"clickhouse": item[1],
"valid": True,
}
for item in client_types
}
view.columns = columns

view.external_tables = view.s3_tables
except Exception as err:
raise serializers.ValidationError(str(err))
Expand Down
33 changes: 32 additions & 1 deletion posthog/warehouse/api/test/test_saved_query.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from unittest.mock import patch
import uuid

from posthog.test.base import APIBaseTest
from posthog.warehouse.models import DataWarehouseModelPath
from posthog.warehouse.models import DataWarehouseModelPath, DataWarehouseSavedQuery


class TestSavedQuery(APIBaseTest):
Expand Down Expand Up @@ -34,6 +35,36 @@ def test_create(self):
],
)

def test_create_with_types(self):
with patch.object(DataWarehouseSavedQuery, "get_columns") as mock_get_columns:
response = self.client.post(
f"/api/projects/{self.team.id}/warehouse_saved_queries/",
{
"name": "event_view",
"query": {
"kind": "HogQLQuery",
"query": "select event as event from events LIMIT 100",
},
"types": [["event", "Nullable(String)"]],
},
)
assert response.status_code == 201
saved_query = response.json()
assert saved_query["name"] == "event_view"
assert saved_query["columns"] == [
{
"key": "event",
"name": "event",
"type": "string",
"schema_valid": True,
"fields": None,
"table": None,
"chain": None,
}
]

mock_get_columns.assert_not_called()

def test_create_name_overlap_error(self):
response = self.client.post(
f"/api/projects/{self.team.id}/warehouse_saved_queries/",
Expand Down
Loading