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

Aggregated queries #1 #8345

Merged
merged 33 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4b51b58
wip
ijreilly Nov 5, 2024
3f929c9
Refactor code
ijreilly Nov 6, 2024
49bc25a
Add test for isUuid util
ijreilly Nov 6, 2024
10fdfd5
fix selectedFields and add number fields aggregations
ijreilly Nov 6, 2024
6b3e533
minor fixes
ijreilly Nov 6, 2024
3fb0fe0
Change way to filter out duplicates of fieldMetadataMap
ijreilly Nov 8, 2024
7adb8ce
Revert changes to tsconfig files
ijreilly Nov 8, 2024
ad441bd
Merge branch 'main' into aggregate-queries-1
charlesBochet Nov 8, 2024
fe53fb8
Merge branch 'main' into aggregate-queries-1
charlesBochet Nov 11, 2024
cd4465f
Fix
charlesBochet Nov 11, 2024
d26a1bd
Fix
charlesBochet Nov 11, 2024
870fc61
Fix ci
charlesBochet Nov 11, 2024
dc72de9
Try ci
charlesBochet Nov 11, 2024
d3dbf46
Fix tests
charlesBochet Nov 11, 2024
52e8a88
Fix
charlesBochet Nov 11, 2024
55ce64d
Fix
charlesBochet Nov 11, 2024
f8d4c2a
Fix
charlesBochet Nov 11, 2024
5af68f8
Fix
charlesBochet Nov 11, 2024
db02867
Fix
charlesBochet Nov 11, 2024
2c6549a
Add feature flag
charlesBochet Nov 11, 2024
b347f39
Refacto maps
charlesBochet Nov 11, 2024
6ca4628
Fix
charlesBochet Nov 11, 2024
7c7ee44
Fix
charlesBochet Nov 11, 2024
22d0fb9
Fix
charlesBochet Nov 11, 2024
b182252
Fix
charlesBochet Nov 11, 2024
2d7ff9c
Fix
charlesBochet Nov 11, 2024
fce5c81
Fixes
charlesBochet Nov 11, 2024
c1df0e5
Remove unused date time scalar
charlesBochet Nov 11, 2024
765d9a6
Fix tests
charlesBochet Nov 11, 2024
e5a9e3d
Merge branch 'main' into aggregate-queries-1
Weiko Nov 12, 2024
b44ceb9
Add aggregation to nested queries + totalCount + fix aggregate for pa…
Weiko Nov 14, 2024
1aee63c
fix
Weiko Nov 14, 2024
853126a
fix tests
Weiko Nov 14, 2024
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
31 changes: 11 additions & 20 deletions .github/workflows/ci-tinybird.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,24 @@ on:
push:
branches:
- main
paths:
- 'package.json'
- 'packages/twenty-tinybird/**'

pull_request:
paths:
- 'package.json'
- 'packages/twenty-tinybird/**'

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
ci:
timeout-minutes: 10
runs-on: ubuntu-latest
uses: tinybirdco/ci/.github/workflows/ci.yml@main
steps:
- name: Check for changed files
id: changed-files
uses: tj-actions/changed-files@v11
with:
files: |
package.json
packages/twenty-tinybird/**

- name: Skip if no relevant changes
if: steps.changed-files.outputs.any_changed == 'false'
run: echo "No relevant changes. Skipping CI."

- name: Check twenty-tinybird package
with:
data_project_dir: packages/twenty-tinybird
tb_admin_token: ${{ secrets.TB_ADMIN_TOKEN }}
tb_host: https://api.eu-central-1.aws.tinybird.co
with:
data_project_dir: packages/twenty-tinybird
secrets:
tb_admin_token: ${{ secrets.TB_ADMIN_TOKEN }}
tb_host: https://api.eu-central-1.aws.tinybird.co
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { isNavigationDrawerExpandedState } from '@/ui/navigation/states/isNaviga
import { navigationDrawerExpandedMemorizedState } from '@/ui/navigation/states/navigationDrawerExpandedMemorizedState';
import { navigationMemorizedUrlState } from '@/ui/navigation/states/navigationMemorizedUrlState';
import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile';
import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled';
import styled from '@emotion/styled';

const StyledMainSection = styled(NavigationDrawerSection)`
Expand All @@ -27,9 +26,7 @@ export const MainNavigationDrawerItems = () => {
const setNavigationMemorizedUrl = useSetRecoilState(
navigationMemorizedUrlState,
);
const isWorkspaceFavoriteEnabled = useIsFeatureEnabled(
'IS_WORKSPACE_FAVORITE_ENABLED',
);

const [isNavigationDrawerExpanded, setIsNavigationDrawerExpanded] =
useRecoilState(isNavigationDrawerExpandedState);
const setNavigationDrawerExpandedMemorized = useSetRecoilState(
Expand Down Expand Up @@ -58,18 +55,9 @@ export const MainNavigationDrawerItems = () => {
/>
</StyledMainSection>
)}

{isWorkspaceFavoriteEnabled && <NavigationDrawerOpenedSection />}

<NavigationDrawerOpenedSection />
<CurrentWorkspaceMemberFavorites />

{isWorkspaceFavoriteEnabled ? (
<WorkspaceFavorites />
) : (
<NavigationDrawerSectionForObjectMetadataItemsWrapper
isRemote={false}
/>
)}
<WorkspaceFavorites />
<NavigationDrawerSectionForObjectMetadataItemsWrapper isRemote={true} />
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ export type FeatureFlagKey =
| 'IS_FREE_ACCESS_ENABLED'
| 'IS_MESSAGE_THREAD_SUBSCRIBER_ENABLED'
| 'IS_WORKFLOW_ENABLED'
| 'IS_WORKSPACE_FAVORITE_ENABLED'
| 'IS_QUERY_RUNNER_TWENTY_ORM_ENABLED'
| 'IS_GMAIL_SEND_EMAIL_SCOPE_ENABLED'
| 'IS_ANALYTICS_V2_ENABLED'
| 'IS_SSO_ENABLED'
| 'IS_UNIQUE_INDEXES_ENABLED'
| 'IS_ARRAY_AND_JSON_FILTER_ENABLED'
| 'IS_MICROSOFT_SYNC_ENABLED'
| 'IS_ADVANCED_FILTERS_ENABLED';
| 'IS_ADVANCED_FILTERS_ENABLED'
| 'IS_AGGREGATE_QUERY_ENABLED';
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ export const seedFeatureFlags = async (
workspaceId: workspaceId,
value: false,
},
{
key: FeatureFlagKey.IsWorkspaceFavoriteEnabled,
workspaceId: workspaceId,
value: true,
},
{
key: FeatureFlagKey.IsAnalyticsV2Enabled,
workspaceId: workspaceId,
Expand Down Expand Up @@ -85,6 +80,11 @@ export const seedFeatureFlags = async (
workspaceId: workspaceId,
value: false,
},
{
key: FeatureFlagKey.IsAggregateQueryEnabled,
workspaceId: workspaceId,
value: false,
},
])
.execute();
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,27 @@ import {
WhereExpressionBuilder,
} from 'typeorm';

import { RecordFilter } from 'src/engine/api/graphql/workspace-query-builder/interfaces/record.interface';
import { ObjectRecordFilter } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface';

import { FieldMetadataMap } from 'src/engine/metadata-modules/utils/generate-object-metadata-map.util';
import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map';

import { GraphqlQueryFilterFieldParser } from './graphql-query-filter-field.parser';

export class GraphqlQueryFilterConditionParser {
private fieldMetadataMap: FieldMetadataMap;
private fieldMetadataMapByName: FieldMetadataMap;
private queryFilterFieldParser: GraphqlQueryFilterFieldParser;

constructor(fieldMetadataMap: FieldMetadataMap) {
this.fieldMetadataMap = fieldMetadataMap;
constructor(fieldMetadataMapByName: FieldMetadataMap) {
this.fieldMetadataMapByName = fieldMetadataMapByName;
this.queryFilterFieldParser = new GraphqlQueryFilterFieldParser(
this.fieldMetadataMap,
this.fieldMetadataMapByName,
);
}

public parse(
queryBuilder: SelectQueryBuilder<any>,
objectNameSingular: string,
filter: Partial<RecordFilter>,
filter: Partial<ObjectRecordFilter>,
): SelectQueryBuilder<any> {
if (!filter || Object.keys(filter).length === 0) {
return queryBuilder;
Expand All @@ -50,7 +50,7 @@ export class GraphqlQueryFilterConditionParser {
switch (key) {
case 'and': {
const andWhereCondition = new Brackets((qb) => {
value.forEach((filter: RecordFilter, index: number) => {
value.forEach((filter: ObjectRecordFilter, index: number) => {
const whereCondition = new Brackets((qb2) => {
Object.entries(filter).forEach(
([subFilterkey, subFilterValue], index) => {
Expand Down Expand Up @@ -82,7 +82,7 @@ export class GraphqlQueryFilterConditionParser {
}
case 'or': {
const orWhereCondition = new Brackets((qb) => {
value.forEach((filter: RecordFilter, index: number) => {
value.forEach((filter: ObjectRecordFilter, index: number) => {
const whereCondition = new Brackets((qb2) => {
Object.entries(filter).forEach(
([subFilterkey, subFilterValue], index) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ import {
import { computeWhereConditionParts } from 'src/engine/api/graphql/graphql-query-runner/utils/compute-where-condition-parts';
import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
import { FieldMetadataMap } from 'src/engine/metadata-modules/utils/generate-object-metadata-map.util';
import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map';
import { CompositeFieldMetadataType } from 'src/engine/metadata-modules/workspace-migration/factories/composite-column-action.factory';
import { capitalize } from 'src/utils/capitalize';

const ARRAY_OPERATORS = ['in', 'contains', 'not_contains'];

export class GraphqlQueryFilterFieldParser {
private fieldMetadataMap: FieldMetadataMap;
private fieldMetadataMapByName: FieldMetadataMap;

constructor(fieldMetadataMap: FieldMetadataMap) {
this.fieldMetadataMap = fieldMetadataMap;
constructor(fieldMetadataMapByName: FieldMetadataMap) {
this.fieldMetadataMapByName = fieldMetadataMapByName;
}

public parse(
Expand All @@ -29,7 +29,7 @@ export class GraphqlQueryFilterFieldParser {
filterValue: any,
isFirst = false,
): void {
const fieldMetadata = this.fieldMetadataMap[`${key}`];
const fieldMetadata = this.fieldMetadataMapByName[`${key}`];

if (!fieldMetadata) {
throw new Error(`Field metadata not found for field: ${key}`);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
ObjectRecordOrderBy,
OrderByDirection,
RecordOrderBy,
} from 'src/engine/api/graphql/workspace-query-builder/interfaces/record.interface';
} from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface';
import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';

import {
Expand All @@ -10,25 +10,25 @@ import {
} from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception';
import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
import { FieldMetadataMap } from 'src/engine/metadata-modules/utils/generate-object-metadata-map.util';
import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map';
import { CompositeFieldMetadataType } from 'src/engine/metadata-modules/workspace-migration/factories/composite-column-action.factory';
import { capitalize } from 'src/utils/capitalize';
export class GraphqlQueryOrderFieldParser {
private fieldMetadataMap: FieldMetadataMap;
private fieldMetadataMapByName: FieldMetadataMap;

constructor(fieldMetadataMap: FieldMetadataMap) {
this.fieldMetadataMap = fieldMetadataMap;
constructor(fieldMetadataMapByName: FieldMetadataMap) {
this.fieldMetadataMapByName = fieldMetadataMapByName;
}

parse(
orderBy: RecordOrderBy,
orderBy: ObjectRecordOrderBy,
objectNameSingular: string,
isForwardPagination = true,
): Record<string, string> {
return orderBy.reduce(
(acc, item) => {
Object.entries(item).forEach(([key, value]) => {
const fieldMetadata = this.fieldMetadataMap[key];
const fieldMetadata = this.fieldMetadataMapByName[key];

if (!fieldMetadata || value === undefined) {
throw new GraphqlQueryRunnerException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metada

import { GraphqlQuerySelectedFieldsParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-selected-fields/graphql-selected-fields.parser';
import { getRelationObjectMetadata } from 'src/engine/api/graphql/graphql-query-runner/utils/get-relation-object-metadata.util';
import { ObjectMetadataMap } from 'src/engine/metadata-modules/utils/generate-object-metadata-map.util';
import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps';

export class GraphqlQuerySelectedFieldsRelationParser {
private objectMetadataMap: ObjectMetadataMap;
private objectMetadataMaps: ObjectMetadataMaps;

constructor(objectMetadataMap: ObjectMetadataMap) {
this.objectMetadataMap = objectMetadataMap;
constructor(objectMetadataMaps: ObjectMetadataMaps) {
this.objectMetadataMaps = objectMetadataMaps;
}

parseRelationField(
Expand All @@ -25,12 +25,12 @@ export class GraphqlQuerySelectedFieldsRelationParser {

const referencedObjectMetadata = getRelationObjectMetadata(
fieldMetadata,
this.objectMetadataMap,
this.objectMetadataMaps,
);

const relationFields = referencedObjectMetadata.fields;
const relationFields = referencedObjectMetadata.fieldsByName;
const fieldParser = new GraphqlQuerySelectedFieldsParser(
this.objectMetadataMap,
this.objectMetadataMaps,
);
const subResult = fieldParser.parse(fieldValue, relationFields);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import { GraphqlQuerySelectedFieldsRelationParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-selected-fields/graphql-selected-fields-relation.parser';
import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
import { ObjectMetadataMap } from 'src/engine/metadata-modules/utils/generate-object-metadata-map.util';
import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps';
import { CompositeFieldMetadataType } from 'src/engine/metadata-modules/workspace-migration/factories/composite-column-action.factory';
import { isRelationFieldMetadataType } from 'src/engine/utils/is-relation-field-metadata-type.util';
import { capitalize } from 'src/utils/capitalize';
Expand All @@ -16,14 +16,14 @@ import { isPlainObject } from 'src/utils/is-plain-object';
export class GraphqlQuerySelectedFieldsParser {
private graphqlQuerySelectedFieldsRelationParser: GraphqlQuerySelectedFieldsRelationParser;

constructor(objectMetadataMap: ObjectMetadataMap) {
constructor(objectMetadataMaps: ObjectMetadataMaps) {
this.graphqlQuerySelectedFieldsRelationParser =
new GraphqlQuerySelectedFieldsRelationParser(objectMetadataMap);
new GraphqlQuerySelectedFieldsRelationParser(objectMetadataMaps);
}

parse(
graphqlSelectedFields: Partial<Record<string, any>>,
fieldMetadataMap: Record<string, FieldMetadataInterface>,
fieldMetadataMapByName: Record<string, FieldMetadataInterface>,
): { select: Record<string, any>; relations: Record<string, any> } {
const result: {
select: Record<string, any>;
Expand All @@ -33,21 +33,27 @@ export class GraphqlQuerySelectedFieldsParser {
relations: {},
};

const hasEdges = Object.keys(graphqlSelectedFields).includes('edges');
Weiko marked this conversation as resolved.
Show resolved Hide resolved

for (const [fieldKey, fieldValue] of Object.entries(
graphqlSelectedFields,
)) {
if (hasEdges && fieldKey !== 'edges') {
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: skipping non-edges fields when edges exist could prevent aggregation fields from being processed at the root level

if (this.shouldNotParseField(fieldKey)) {
continue;
}

if (this.isConnectionField(fieldKey, fieldValue)) {
const subResult = this.parse(fieldValue, fieldMetadataMap);
const subResult = this.parse(fieldValue, fieldMetadataMapByName);

Object.assign(result.select, subResult.select);
Object.assign(result.relations, subResult.relations);
continue;
}

const fieldMetadata = fieldMetadataMap[fieldKey];
const fieldMetadata = fieldMetadataMapByName[fieldKey];

if (!fieldMetadata) {
throw new GraphqlQueryRunnerException(
Expand Down Expand Up @@ -83,9 +89,7 @@ export class GraphqlQuerySelectedFieldsParser {
}

private shouldNotParseField(fieldKey: string): boolean {
return ['__typename', 'totalCount', 'pageInfo', 'cursor'].includes(
Copy link
Member

Choose a reason for hiding this comment

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

Should we replace totalCount everywhere in the app by countUniqueId? We'll need to give a heads up for deprecation but we can start replacing it in our frontend at least. We plan to email people about API Key deprecation so we can let them know about this at the same time.
If I'm not mistaken we're doing a separate query for totalCount now while this could fit in the same query with all other aggregated fields?

fieldKey,
);
return ['__typename', 'cursor'].includes(fieldKey);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: removing 'totalCount' and 'pageInfo' from shouldNotParseField may cause these fields to be incorrectly included in the select statement


private parseCompositeField(
Expand Down
Loading
Loading