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

feat(event-explorer): column aliases #15861

Closed
wants to merge 15 commits into from
Closed
4 changes: 2 additions & 2 deletions frontend/src/lib/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {
import { IconCopy } from 'lib/lemon-ui/icons'
import { lemonToast } from 'lib/lemon-ui/lemonToast'
import { BehavioralFilterKey } from 'scenes/cohorts/CohortFilters/types'
import { extractExpressionComment } from '~/queries/nodes/DataTable/utils'
import { extractCommentOrAlias } from '~/queries/nodes/DataTable/utils'
import { urls } from 'scenes/urls'
import { isFunnelsFilter } from 'scenes/insights/sharedUtils'

Expand Down Expand Up @@ -371,7 +371,7 @@ export function formatPropertyLabel(
valueFormatter: (value: PropertyFilterValue | undefined) => string | string[] | null = (s) => [String(s)]
): string {
if (isHogQLPropertyFilter(item as AnyFilterLike)) {
return extractExpressionComment(item.key)
return extractCommentOrAlias(item.key)
}
const { value, key, operator, type } = item
return type === 'cohort'
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/queries/nodes/DataNode/dataNodeLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { subscriptions } from 'kea-subscriptions'
import { objectsEqual, shouldCancelQuery, uuid } from 'lib/utils'
import clsx from 'clsx'
import api, { ApiMethodOptions } from 'lib/api'
import { removeExpressionComment } from '~/queries/nodes/DataTable/utils'
import { removeCommentOrAlias } from '~/queries/nodes/DataTable/utils'
import { userLogic } from 'scenes/userLogic'
import { UNSAVED_INSIGHT_MIN_REFRESH_INTERVAL_MINUTES } from 'scenes/insights/insightLogic'
import { teamLogic } from 'scenes/teamLogic'
Expand Down Expand Up @@ -291,7 +291,7 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
const sortKey = query.orderBy?.[0] ?? 'timestamp DESC'
if (sortKey === 'timestamp DESC') {
const sortColumnIndex = query.select
.map((hql) => removeExpressionComment(hql))
.map((hql) => removeCommentOrAlias(hql))
.indexOf('timestamp')
if (sortColumnIndex !== -1) {
const typedResults = (response as EventsQuery['response'])?.results
Expand Down Expand Up @@ -325,7 +325,7 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
const typedResults = (response as EventsQuery['response'])?.results
if (sortKey === 'timestamp DESC') {
const sortColumnIndex = query.select
.map((hql) => removeExpressionComment(hql))
.map((hql) => removeCommentOrAlias(hql))
.indexOf('timestamp')
if (sortColumnIndex !== -1) {
const lastTimestamp = typedResults?.[typedResults.length - 1]?.[sortColumnIndex]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { AutoSizer } from 'react-virtualized/dist/es/AutoSizer'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { useState } from 'react'
import { columnConfiguratorLogic, ColumnConfiguratorLogicProps } from './columnConfiguratorLogic'
import { defaultDataTableColumns, extractExpressionComment, removeExpressionComment } from '../utils'
import { defaultDataTableColumns, extractCommentOrAlias, removeCommentOrAlias } from '../utils'
import { DataTableNode, NodeKind } from '~/queries/schema'
import { LemonModal } from 'lib/lemon-ui/LemonModal'
import { isEventsQuery, taxonomicFilterToHogQl, trimQuotes } from '~/queries/utils'
Expand Down Expand Up @@ -45,11 +45,11 @@ export function ColumnConfigurator({ query, setQuery }: ColumnConfiguratorProps)
if (isEventsQuery(query.source)) {
let orderBy = query.source.orderBy
if (orderBy && orderBy.length > 0) {
const orderColumn = removeExpressionComment(
const orderColumn = removeCommentOrAlias(
orderBy[0].endsWith(' DESC') ? orderBy[0].replace(/ DESC$/, '') : orderBy[0]
)
// the old orderBy column was removed, so remove it from the new query
if (!columns.some((c) => removeExpressionComment(c) === orderColumn)) {
if (!columns.some((c) => removeCommentOrAlias(c) === orderColumn)) {
orderBy = undefined
}
}
Expand Down Expand Up @@ -112,7 +112,7 @@ function ColumnConfiguratorModal({ query }: ColumnConfiguratorProps): JSX.Elemen
columnKey = column.substring(11)
}

columnKey = trimQuotes(extractExpressionComment(columnKey))
columnKey = trimQuotes(extractCommentOrAlias(columnKey))

return (
<div className={clsx(['SelectedColumn', 'selected'])} style={{ height: rowItemHeight }}>
Expand Down
20 changes: 9 additions & 11 deletions frontend/src/queries/nodes/DataTable/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { DateRange } from '~/queries/nodes/DataNode/DateRange'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { TaxonomicPopover } from 'lib/components/TaxonomicPopover/TaxonomicPopover'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { extractExpressionComment, removeExpressionComment } from '~/queries/nodes/DataTable/utils'
import { extractCommentOrAlias, removeCommentOrAlias } from '~/queries/nodes/DataTable/utils'
import { InsightEmptyState, InsightErrorState } from 'scenes/insights/EmptyStates'
import { EventType } from '~/types'
import { SavedQueries } from '~/queries/nodes/DataTable/SavedQueries'
Expand Down Expand Up @@ -81,9 +81,8 @@ export function DataTable({ query, setQuery, context, cachedResults }: DataTable
} = useValues(builtDataNodeLogic)

const dataTableLogicProps: DataTableLogicProps = { query, key, context }
const { dataTableRows, columnsInQuery, columnsInResponse, queryWithDefaults, canSort } = useValues(
dataTableLogic(dataTableLogicProps)
)
const { dataTableRows, columnsInLemonTable, columnsInQuery, columnsInResponse, queryWithDefaults, canSort } =
useValues(dataTableLogic(dataTableLogicProps))

const {
showActions,
Expand All @@ -104,7 +103,6 @@ export function DataTable({ query, setQuery, context, cachedResults }: DataTable
const isReadOnly = setQuery === undefined

const actionsColumnShown = showActions && isEventsQuery(query.source) && columnsInResponse?.includes('*')
const columnsInLemonTable = isHogQLQuery(query.source) ? columnsInResponse ?? columnsInQuery : columnsInQuery

const lemonColumns: LemonTableColumn<DataTableRow, any>[] = [
...columnsInLemonTable.map((key, index) => ({
Expand Down Expand Up @@ -132,9 +130,9 @@ export function DataTable({ query, setQuery, context, cachedResults }: DataTable
!isReadOnly && showActions && isEventsQuery(query.source) ? (
<>
<div className="px-2 py-1">
<div className="font-mono font-bold">{extractExpressionComment(key)}</div>
{extractExpressionComment(key) !== removeExpressionComment(key) && (
<div className="font-mono">{removeExpressionComment(key)}</div>
<div className="font-mono font-bold">{extractCommentOrAlias(key)}</div>
{extractCommentOrAlias(key) !== removeCommentOrAlias(key) && (
<div className="font-mono">{removeCommentOrAlias(key)}</div>
)}
</div>
<LemonDivider />
Expand Down Expand Up @@ -265,15 +263,15 @@ export function DataTable({ query, setQuery, context, cachedResults }: DataTable
status="danger"
data-attr="datatable-remove-column"
onClick={() => {
const cleanColumnKey = removeExpressionComment(key)
const cleanColumnKey = removeCommentOrAlias(key)
const newSource: EventsQuery = {
...(query.source as EventsQuery),
select: (query.source as EventsQuery).select.filter((_, i) => i !== index),
// remove the current column from orderBy if it's there
orderBy: (query.source as EventsQuery).orderBy?.find(
(orderKey) =>
removeExpressionComment(orderKey) === cleanColumnKey ||
removeExpressionComment(orderKey) === `-${cleanColumnKey}`
removeCommentOrAlias(orderKey) === cleanColumnKey ||
removeCommentOrAlias(orderKey) === `-${cleanColumnKey}`
)
? undefined
: (query.source as EventsQuery).orderBy,
Expand Down
15 changes: 10 additions & 5 deletions frontend/src/queries/nodes/DataTable/dataTableLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
QueryContext,
TimeToSeeDataSessionsQuery,
} from '~/queries/schema'
import { getColumnsForQuery, removeExpressionComment } from './utils'
import { getColumnsForQuery, removeCommentOrAlias } from './utils'
import { objectsEqual, sortedKeys } from 'lib/utils'
import { isDataTableNode, isEventsQuery } from '~/queries/utils'
import { isDataTableNode, isEventsQuery, isHogQLQuery } from '~/queries/utils'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { FEATURE_FLAGS } from 'lib/constants'
import { dataNodeLogic } from '~/queries/nodes/DataNode/dataNodeLogic'
Expand Down Expand Up @@ -69,7 +69,12 @@ export const dataTableLogic = kea<dataTableLogicType>([
columnsInResponse: [
(s) => [s.response],
(response: AnyDataNode['response']): string[] | null =>
response && 'columns' in response && Array.isArray(response.columns) ? response?.columns : null,
response && 'columns' in response && Array.isArray(response.columns) ? response.columns : null,
],
columnsInLemonTable: [
(s, p) => [p.query, s.columnsInResponse, s.columnsInQuery],
(query, columnsInResponse, columnsInQuery) =>
isHogQLQuery(query.source) ? columnsInResponse ?? columnsInQuery : columnsInQuery,
],
dataTableRows: [
(s) => [s.sourceKind, s.orderBy, s.response, s.columnsInQuery, s.columnsInResponse],
Expand All @@ -95,8 +100,8 @@ export const dataTableLogic = kea<dataTableLogicType>([
const orderKeyIndex =
columnsInResponse?.findIndex(
(column) =>
removeExpressionComment(column) === orderKey ||
removeExpressionComment(column) === `-${orderKey}`
removeCommentOrAlias(column) === orderKey ||
removeCommentOrAlias(column) === `-${orderKey}`
) ?? -1

// Add a label between results if the day changed
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/queries/nodes/DataTable/renderColumnMeta.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { PropertyFilterType } from '~/types'
import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
import { QueryContext, DataTableNode } from '~/queries/schema'
import { isEventsQuery, isHogQLQuery, trimQuotes } from '~/queries/utils'
import { extractExpressionComment } from '~/queries/nodes/DataTable/utils'
import { extractCommentOrAlias } from '~/queries/nodes/DataTable/utils'
import { SortingIndicator } from 'lib/lemon-ui/LemonTable/sorting'

export interface ColumnMeta {
Expand Down Expand Up @@ -41,7 +41,7 @@ export function renderColumnMeta(key: string, query: DataTableNode, context?: Qu
// NOTE: PropertyFilterType.Event is not a mistake. PropertyKeyInfo only knows events vs elements ¯\_(ツ)_/¯
title = <PropertyKeyInfo value={trimQuotes(key.substring(18))} type={PropertyFilterType.Event} disableIcon />
} else {
title = isEventsQuery(query.source) ? extractExpressionComment(key) : key
title = isEventsQuery(query.source) ? extractCommentOrAlias(key) : key
}

if (isEventsQuery(query.source) && !query.allowSorting) {
Expand Down
30 changes: 19 additions & 11 deletions frontend/src/queries/nodes/DataTable/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,32 @@
import {
defaultDataTableEventColumns,
defaultDataTablePersonColumns,
extractExpressionComment,
extractCommentOrAlias,
getColumnsForQuery,
removeExpressionComment,
removeCommentOrAlias,
} from '~/queries/nodes/DataTable/utils'
import { NodeKind } from '~/queries/schema'

describe('DataTable utils', () => {
it('extractExpressionComment', () => {
expect(extractExpressionComment('')).toBe('')
expect(extractExpressionComment('asd -- bla')).toBe('bla')
expect(extractExpressionComment('asd -- asd -- bla ')).toBe('bla')
it('extractCommentOrAlias', () => {
expect(extractCommentOrAlias('')).toBe('')
expect(extractCommentOrAlias('asd -- bla')).toBe('bla')
expect(extractCommentOrAlias('asd -- asd -- bla ')).toBe('bla')
expect(extractCommentOrAlias('asd as `hello`')).toBe('hello')
expect(extractCommentOrAlias('asd as "hello world"')).toBe('hello world')
expect(extractCommentOrAlias('asd as $bandana')).toBe('$bandana')
expect(extractCommentOrAlias('asd as $bandana -- bla')).toBe('bla')
})

it('removeExpressionComment', () => {
expect(removeExpressionComment('')).toBe('')
expect(removeExpressionComment('asd -- bla')).toBe('asd')
expect(removeExpressionComment('asd -- asd -- bla ')).toBe('asd -- asd')
expect(removeExpressionComment(' hoho trim -- trim')).toBe('hoho trim')
it('removeCommentOrAlias', () => {
expect(removeCommentOrAlias('')).toBe('')
expect(removeCommentOrAlias('asd -- bla')).toBe('asd')
expect(removeCommentOrAlias('asd -- asd -- bla ')).toBe('asd -- asd')
expect(removeCommentOrAlias(' hoho trim -- trim')).toBe('hoho trim')
expect(removeCommentOrAlias('asd as `hello`')).toBe('asd')
expect(removeCommentOrAlias('asd as "hello world"')).toBe('asd')
expect(removeCommentOrAlias('asd as $bandana')).toBe('asd')
expect(removeCommentOrAlias('asd as $bandana -- bla')).toBe('asd')
})

it('getColumnsForQuery', () => {
Expand Down
28 changes: 25 additions & 3 deletions frontend/src/queries/nodes/DataTable/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,38 @@ export function getColumnsForQuery(query: DataTableNode): HogQLExpression[] {
return query.columns ?? getDataNodeDefaultColumns(query.source)
}

export function extractExpressionComment(query: string): string {
/**
* Extracts the comment and/or alias from a query. For display only!
*
* This is a rather naive implementation using basic string operations.
* Its output is not cleaned for usage in queries!
*/
export function extractCommentOrAlias(query: string): string {
if (query.match(/ as (`[^`]+`|"[^"]+"|[a-zA-Z$][a-zA-Z0-9_$]*)\s*$/)) {
const comment = query.split(' as ').pop()?.trim() || query
if ((comment.startsWith('`') || comment.startsWith('"')) && comment.endsWith(comment[0])) {
return comment.slice(1, -1)
}
return comment
}
if (query.includes('--')) {
return query.split('--').pop()?.trim() || query
}
return query
}

export function removeExpressionComment(query: string): string {
/**
* Removes the comment and/or alias from a query, keeping just the SQL. For display only!
*
* This is a rather naive implementation using basic string operations.
* Its output is not cleaned for usage in queries!
*/
export function removeCommentOrAlias(query: string): string {
if (query.includes('--')) {
return query.split('--').slice(0, -1).join('--').trim()
query = query.split('--').slice(0, -1).join('--').trim()
}
if (query.match(/ as (`[^`]+`|"[^"]+"|[a-zA-Z\$][a-zA-Z0-9\_\$]*)$/)) {
Comment on lines +43 to +66
Copy link
Member

Choose a reason for hiding this comment

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

These two regexes only seem to differ by "_" and "$" being escaped in character sets, and whitespace being allowed at the end of the extraction one. Are these differences important, or should these be the same? I think this would be clearer if the regexes were put in file-level constants.
BTW I think the case insensitivity flag is missing, this wouldn't match the valid "AS" or "As"

query = query.split(' as ').slice(0, -1).join(' as ').trim()
}
return query.trim()
}
4 changes: 2 additions & 2 deletions posthog/api/test/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def test_safe_clickhouse_error_passed_through(self):
response_get.json(),
self.validation_error_response(
"Illegal types DateTime64(6, 'UTC') and String of arguments of function plus: "
"While processing toTimeZone(timestamp, 'UTC') + 'string'.",
"While processing toTimeZone(timestamp, 'UTC') + 'string' AS `timestamp + 'string'`.",
"illegal_type_of_argument",
),
)
Expand All @@ -263,7 +263,7 @@ def test_safe_clickhouse_error_passed_through(self):
response_post.json(),
self.validation_error_response(
"Illegal types DateTime64(6, 'UTC') and String of arguments of function plus: "
"While processing toTimeZone(timestamp, 'UTC') + 'string'.",
"While processing toTimeZone(timestamp, 'UTC') + 'string' AS `timestamp + 'string'`.",
"illegal_type_of_argument",
),
)
Expand Down
2 changes: 0 additions & 2 deletions posthog/hogql/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,6 @@ def visit_alias(self, node: ast.Alias):
raise ResolverException("Aliases are allowed only within SELECT queries")

scope = self.scopes[-1]
if node.alias in scope.aliases:
raise ResolverException(f"Cannot redefine an alias with the name: {node.alias}")
if node.alias == "":
raise ResolverException("Alias cannot be empty")

Expand Down
Loading