Merge branch 'master' into source-for-persons-query

PostHog · Oct 18, 2023 · 8ea5b87 · 8ea5b87
2 parents 589b26c + f476f67
commit 8ea5b87
Show file tree

Hide file tree

Showing 20 changed files with 348 additions and 73 deletions.
diff --git a/frontend/__snapshots__/components-sharing--insight-sharing-licensed.png b/frontend/__snapshots__/components-sharing--insight-sharing-licensed.png
diff --git a/frontend/__snapshots__/components-sharing--insight-sharing.png b/frontend/__snapshots__/components-sharing--insight-sharing.png
diff --git a/frontend/__snapshots__/components-sharing--recording-sharing-licensed.png b/frontend/__snapshots__/components-sharing--recording-sharing-licensed.png
diff --git a/frontend/src/lib/components/Sharing/SharingModal.tsx b/frontend/src/lib/components/Sharing/SharingModal.tsx
@@ -121,28 +121,6 @@ export function SharingModalContent({
                             </div>
 
                             <Form logic={sharingLogic} props={logicProps} formKey="embedConfig" className="space-y-2">
-                                {previewIframe && (
-                                    <div className="rounded border">
-                                        <LemonButton
-                                            fullWidth
-                                            status="stealth"
-                                            sideIcon={showPreview ? <IconUnfoldLess /> : <IconUnfoldMore />}
-                                            onClick={togglePreview}
-                                        >
-                                            Preview
-                                            {showPreview && !iframeLoaded ? <Spinner className="ml-2" /> : null}
-                                        </LemonButton>
-                                        {showPreview && (
-                                            <div className="SharingPreview border-t">
-                                                <iframe
-                                                    className="block"
-                                                    {...iframeProperties}
-                                                    onLoad={() => setIframeLoaded(true)}
-                                                />
-                                            </div>
-                                        )}
-                                    </div>
-                                )}
                                 <Field name="whitelabel">
                                     {({ value, onChange }) => (
                                         <LemonSwitch
@@ -203,6 +181,29 @@ export function SharingModalContent({
                                         )}
                                     </Field>
                                 )}
+
+                                {previewIframe && (
+                                    <div className="rounded border">
+                                        <LemonButton
+                                            fullWidth
+                                            status="stealth"
+                                            sideIcon={showPreview ? <IconUnfoldLess /> : <IconUnfoldMore />}
+                                            onClick={togglePreview}
+                                        >
+                                            Preview
+                                            {showPreview && !iframeLoaded ? <Spinner className="ml-2" /> : null}
+                                        </LemonButton>
+                                        {showPreview && (
+                                            <div className="SharingPreview border-t">
+                                                <iframe
+                                                    className="block"
+                                                    {...iframeProperties}
+                                                    onLoad={() => setIframeLoaded(true)}
+                                                />
+                                            </div>
+                                        )}
+                                    </div>
+                                )}
                             </Form>
                         </>
                     ) : null}

diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json
@@ -1235,6 +1235,10 @@
             "additionalProperties": false,
             "description": "HogQL Query Options are automatically set per team. However, they can be overriden in the query.",
             "properties": {
+                "inCohortVia": {
+                    "enum": ["leftjoin", "subquery"],
+                    "type": "string"
+                },
                 "personsArgMaxVersion": {
                     "enum": ["auto", "v1", "v2"],
                     "type": "string"

diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts
@@ -133,6 +133,7 @@ export interface DataNode extends Node {
 export interface HogQLQueryModifiers {
     personsOnEventsMode?: 'disabled' | 'v1_enabled' | 'v2_enabled'
     personsArgMaxVersion?: 'auto' | 'v1' | 'v2'
+    inCohortVia?: 'leftjoin' | 'subquery'
 }
 
 export interface HogQLQueryResponse {

diff --git a/frontend/src/scenes/debug/HogQLDebug.tsx b/frontend/src/scenes/debug/HogQLDebug.tsx
@@ -32,7 +32,7 @@ export function HogQLDebug({ query, setQuery, queryKey }: HogQLDebugProps): JSX.
                 </div>
                 <div className="flex gap-2">
                     <LemonLabel>
-                        POE Version:
+                        POE:
                         <LemonSelect
                             options={[
                                 { value: 'disabled', label: 'Disabled' },
@@ -49,7 +49,7 @@ export function HogQLDebug({ query, setQuery, queryKey }: HogQLDebugProps): JSX.
                         />
                     </LemonLabel>
                     <LemonLabel>
-                        Persons ArgMax Version
+                        Persons ArgMax:
                         <LemonSelect
                             options={[
                                 { value: 'v1', label: 'V1' },
@@ -64,6 +64,22 @@ export function HogQLDebug({ query, setQuery, queryKey }: HogQLDebugProps): JSX.
                             value={query.modifiers?.personsArgMaxVersion ?? response?.modifiers?.personsArgMaxVersion}
                         />
                     </LemonLabel>
+                    <LemonLabel>
+                        In Cohort Via:
+                        <LemonSelect
+                            options={[
+                                { value: 'join', label: 'join' },
+                                { value: 'subquery', label: 'subquery' },
+                            ]}
+                            onChange={(value) =>
+                                setQuery({
+                                    ...query,
+                                    modifiers: { ...query.modifiers, inCohortVia: value },
+                                } as HogQLQuery)
+                            }
+                            value={query.modifiers?.inCohortVia ?? response?.modifiers?.inCohortVia}
+                        />
+                    </LemonLabel>{' '}
                 </div>
                 {dataLoading ? (
                     <>

diff --git a/frontend/src/scenes/plugins/tabs/apps/AppView.tsx b/frontend/src/scenes/plugins/tabs/apps/AppView.tsx
@@ -1,18 +1,11 @@
 import { Link, LemonButton, LemonBadge } from '@posthog/lemon-ui'
 import { useActions, useValues } from 'kea'
 import { LemonMenuItem, LemonMenu } from 'lib/lemon-ui/LemonMenu'
-import {
-    IconLink,
-    IconCloudDownload,
-    IconSettings,
-    IconEllipsis,
-    IconLegend,
-    IconErrorOutline,
-} from 'lib/lemon-ui/icons'
+import { IconLink, IconSettings, IconEllipsis, IconLegend, IconErrorOutline } from 'lib/lemon-ui/icons'
 import { PluginImage } from 'scenes/plugins/plugin/PluginImage'
 import { SuccessRateBadge } from 'scenes/plugins/plugin/SuccessRateBadge'
 import { pluginsLogic } from 'scenes/plugins/pluginsLogic'
-import { PluginTypeWithConfig, PluginRepositoryEntry, PluginInstallationType } from 'scenes/plugins/types'
+import { PluginTypeWithConfig, PluginRepositoryEntry } from 'scenes/plugins/types'
 import { urls } from 'scenes/urls'
 import { PluginType } from '~/types'
 import { PluginTags } from './components'
@@ -23,8 +16,8 @@ export function AppView({
 }: {
     plugin: PluginTypeWithConfig | PluginType | PluginRepositoryEntry
 }): JSX.Element {
-    const { installingPluginUrl, showAppMetricsForPlugin, loading, sortableEnabledPlugins } = useValues(pluginsLogic)
-    const { installPlugin, editPlugin, toggleEnabled, openReorderModal } = useActions(pluginsLogic)
+    const { showAppMetricsForPlugin, sortableEnabledPlugins } = useValues(pluginsLogic)
+    const { editPlugin, toggleEnabled, openReorderModal } = useActions(pluginsLogic)
 
     const pluginConfig = 'pluginConfig' in plugin ? plugin.pluginConfig : null
     const isConfigured = !!pluginConfig?.id
@@ -112,9 +105,9 @@ export function AppView({
             </div>
 
             <div className="flex gap-2 whitespace-nowrap items-center justify-end">
-                {'id' in plugin && pluginConfig ? (
+                {'id' in plugin && (
                     <>
-                        {!pluginConfig.enabled && isConfigured && (
+                        {pluginConfig && !pluginConfig.enabled && isConfigured && (
                             <LemonButton
                                 type="secondary"
                                 size="small"
@@ -129,7 +122,8 @@ export function AppView({
                             </LemonButton>
                         )}
 
-                        {pluginConfig.id &&
+                        {pluginConfig &&
+                            pluginConfig.id &&
                             (pluginConfig.error ? (
                                 <LemonButton
                                     type="secondary"
@@ -160,18 +154,6 @@ export function AppView({
                             Configure
                         </LemonButton>
                     </>
-                ) : (
-                    <LemonButton
-                        type="primary"
-                        loading={loading && installingPluginUrl === plugin.url}
-                        icon={<IconCloudDownload />}
-                        size="small"
-                        onClick={() =>
-                            plugin.url ? installPlugin(plugin.url, PluginInstallationType.Repository) : undefined
-                        }
-                    >
-                        Install
-                    </LemonButton>
                 )}
 
                 <LemonMenu items={menuItems} placement="left">

diff --git a/posthog/hogql/database/schema/cohort_people.py b/posthog/hogql/database/schema/cohort_people.py
@@ -51,7 +51,7 @@ def to_printed_clickhouse(self, context):
         return "cohortpeople"
 
     def to_printed_hogql(self):
-        return "cohort_people"
+        return "raw_cohort_people"
 
 
 class CohortPeople(LazyTable):

diff --git a/posthog/hogql/database/schema/groups.py b/posthog/hogql/database/schema/groups.py
@@ -69,7 +69,7 @@ def to_printed_clickhouse(self, context):
         return "groups"
 
     def to_printed_hogql(self):
-        return "groups"
+        return "raw_groups"
 
 
 class GroupsTable(LazyTable):

diff --git a/posthog/hogql/database/schema/person_overrides.py b/posthog/hogql/database/schema/person_overrides.py
@@ -62,7 +62,7 @@ def to_printed_clickhouse(self, context):
         return "person_overrides"
 
     def to_printed_hogql(self):
-        return "person_overrides"
+        return "raw_person_overrides"
 
 
 class PersonOverridesTable(Table):

diff --git a/posthog/hogql/functions/test/test_cohort.py b/posthog/hogql/functions/test/test_cohort.py
@@ -7,6 +7,7 @@
 from posthog.models import Cohort
 from posthog.models.cohort.util import recalculate_cohortpeople
 from posthog.models.utils import UUIDT
+from posthog.schema import HogQLQueryModifiers
 from posthog.test.base import BaseTest, _create_person, _create_event, flush_persons_and_events
 
 elements_chain_match = lambda x: parse_expr("match(elements_chain, {regex})", {"regex": ast.Constant(value=str(x))})
@@ -38,14 +39,15 @@ def test_in_cohort_dynamic(self):
         response = execute_hogql_query(
             f"SELECT event FROM events WHERE person_id IN COHORT {cohort.pk} AND event='{random_uuid}'",
             self.team,
+            modifiers=HogQLQueryModifiers(inCohortVia="subquery"),
         )
         self.assertEqual(
             response.clickhouse,
             f"SELECT events.event FROM events WHERE and(equals(events.team_id, {self.team.pk}), in(events.person_id, (SELECT cohortpeople.person_id FROM cohortpeople WHERE and(equals(cohortpeople.team_id, {self.team.pk}), equals(cohortpeople.cohort_id, {cohort.pk})) GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), equals(events.event, %(hogql_val_0)s)) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1",
         )
         self.assertEqual(
             response.hogql,
-            f"SELECT event FROM events WHERE and(in(person_id, (SELECT person_id FROM cohort_people WHERE equals(cohort_id, {cohort.pk}) GROUP BY person_id, cohort_id, version HAVING greater(sum(sign), 0))), equals(event, '{random_uuid}')) LIMIT 100",
+            f"SELECT event FROM events WHERE and(in(person_id, (SELECT person_id FROM raw_cohort_people WHERE equals(cohort_id, {cohort.pk}) GROUP BY person_id, cohort_id, version HAVING greater(sum(sign), 0))), equals(event, '{random_uuid}')) LIMIT 100",
         )
         self.assertEqual(len(response.results), 1)
         self.assertEqual(response.results[0][0], random_uuid)
@@ -59,6 +61,7 @@ def test_in_cohort_static(self):
         response = execute_hogql_query(
             f"SELECT event FROM events WHERE person_id IN COHORT {cohort.pk}",
             self.team,
+            modifiers=HogQLQueryModifiers(inCohortVia="subquery"),
         )
         self.assertEqual(
             response.clickhouse,
@@ -79,6 +82,7 @@ def test_in_cohort_strings(self):
         response = execute_hogql_query(
             f"SELECT event FROM events WHERE person_id IN COHORT 'my cohort'",
             self.team,
+            modifiers=HogQLQueryModifiers(inCohortVia="subquery"),
         )
         self.assertEqual(
             response.clickhouse,

diff --git a/posthog/hogql/modifiers.py b/posthog/hogql/modifiers.py
@@ -19,4 +19,7 @@ def create_default_modifiers_for_team(
     if modifiers.personsArgMaxVersion is None:
         modifiers.personsArgMaxVersion = "auto"
 
+    if modifiers.inCohortVia is None:
+        modifiers.inCohortVia = "subquery"
+
     return modifiers
diff --git a/posthog/hogql/parser.py b/posthog/hogql/parser.py
@@ -46,7 +46,7 @@ def parse_expr(
         node = RULE_TO_PARSE_FUNCTION[backend]["expr"](expr, start)
         if placeholders:
             with timings.measure("replace_placeholders"):
-                return replace_placeholders(node, placeholders)
+                node = replace_placeholders(node, placeholders)
     return node
 
 
@@ -63,7 +63,7 @@ def parse_order_expr(
         node = RULE_TO_PARSE_FUNCTION[backend]["order_expr"](order_expr)
         if placeholders:
             with timings.measure("replace_placeholders"):
-                return replace_placeholders(node, placeholders)
+                node = replace_placeholders(node, placeholders)
     return node
 
 

diff --git a/posthog/hogql/printer.py b/posthog/hogql/printer.py
@@ -31,6 +31,7 @@
 )
 from posthog.hogql.functions.mapping import validate_function_args
 from posthog.hogql.resolver import ResolverException, lookup_field_by_name, resolve_types
+from posthog.hogql.transforms.in_cohort import resolve_in_cohorts
 from posthog.hogql.transforms.lazy_tables import resolve_lazy_tables
 from posthog.hogql.transforms.property_types import resolve_property_types
 from posthog.hogql.visitor import Visitor, clone_expr
@@ -83,6 +84,9 @@ def prepare_ast_for_printing(
 
     with context.timings.measure("resolve_types"):
         node = resolve_types(node, context, scopes=[node.type for node in stack] if stack else None)
+    if context.modifiers.inCohortVia == "leftjoin":
+        with context.timings.measure("resolve_in_cohorts"):
+            resolve_in_cohorts(node, stack, context)
     if dialect == "clickhouse":
         with context.timings.measure("resolve_property_types"):
             node = resolve_property_types(node, context)
@@ -489,7 +493,7 @@ def visit_compare_operation(self, node: ast.CompareOperation):
                 lambda left_op, right_op: left_op <= right_op if left_op is not None and right_op is not None else False
             )
         else:
-            raise HogQLException(f"Unknown CompareOperationOp: {type(node.op).__name__}")
+            raise HogQLException(f"Unknown CompareOperationOp: {node.op.name}")
 
         # Try to see if we can take shortcuts
 

diff --git a/posthog/hogql/resolver.py b/posthog/hogql/resolver.py
@@ -4,11 +4,10 @@
 
 from posthog.hogql import ast
 from posthog.hogql.ast import FieldTraverserType, ConstantType
-from posthog.hogql.functions import HOGQL_POSTHOG_FUNCTIONS
+from posthog.hogql.functions import HOGQL_POSTHOG_FUNCTIONS, cohort
 from posthog.hogql.context import HogQLContext
 from posthog.hogql.database.models import StringJSONDatabaseField, FunctionCallTable, LazyTable, SavedQuery
 from posthog.hogql.errors import ResolverException
-from posthog.hogql.functions.cohort import cohort
 from posthog.hogql.functions.mapping import validate_function_args
 from posthog.hogql.functions.sparkline import sparkline
 from posthog.hogql.parser import parse_select
@@ -508,22 +507,23 @@ def visit_compare_operation(self, node: ast.CompareOperation):
                 )
             )
 
-        if node.op == ast.CompareOperationOp.InCohort:
-            return self.visit(
-                ast.CompareOperation(
-                    op=ast.CompareOperationOp.In,
-                    left=node.left,
-                    right=cohort(node=node.right, args=[node.right], context=self.context),
+        if self.context.modifiers.inCohortVia != "leftjoin":
+            if node.op == ast.CompareOperationOp.InCohort:
+                return self.visit(
+                    ast.CompareOperation(
+                        op=ast.CompareOperationOp.In,
+                        left=node.left,
+                        right=cohort(node=node.right, args=[node.right], context=self.context),
+                    )
                 )
-            )
-        elif node.op == ast.CompareOperationOp.NotInCohort:
-            return self.visit(
-                ast.CompareOperation(
-                    op=ast.CompareOperationOp.NotIn,
-                    left=node.left,
-                    right=cohort(node=node.right, args=[node.right], context=self.context),
+            elif node.op == ast.CompareOperationOp.NotInCohort:
+                return self.visit(
+                    ast.CompareOperation(
+                        op=ast.CompareOperationOp.NotIn,
+                        left=node.left,
+                        right=cohort(node=node.right, args=[node.right], context=self.context),
+                    )
                 )
-            )
 
         node = super().visit_compare_operation(node)
         node.type = ast.BooleanType()

diff --git a/posthog/hogql/test/test_modifiers.py b/posthog/hogql/test/test_modifiers.py
@@ -1,5 +1,6 @@
 from posthog.hogql.modifiers import create_default_modifiers_for_team
 from posthog.hogql.query import execute_hogql_query
+from posthog.models import Cohort
 from posthog.schema import HogQLQueryModifiers
 from posthog.test.base import BaseTest
 from django.test import override_settings
@@ -71,3 +72,20 @@ def test_modifiers_persons_argmax_version_auto(self):
             modifiers=HogQLQueryModifiers(personsArgMaxVersion="auto"),
         )
         assert "in(tuple(person.id, person.version)" not in response.clickhouse
+
+    def test_modifiers_in_cohort_join(self):
+        cohort = Cohort.objects.create(team=self.team, name="test")
+        response = execute_hogql_query(
+            f"select * from persons where id in cohort {cohort.pk}",
+            team=self.team,
+            modifiers=HogQLQueryModifiers(inCohortVia="subquery"),
+        )
+        assert "LEFT JOIN" not in response.clickhouse
+
+        # Use the v1 query when not selecting any properties
+        response = execute_hogql_query(
+            f"select * from persons where id in cohort {cohort.pk}",
+            team=self.team,
+            modifiers=HogQLQueryModifiers(inCohortVia="leftjoin"),
+        )
+        assert "LEFT JOIN" in response.clickhouse