From 576418df07ee34a7f91cf6e7c13167c6be721d5f Mon Sep 17 00:00:00 2001 From: Ola Okelola Date: Tue, 5 Sep 2023 15:40:14 -0700 Subject: [PATCH] use join for 2-way connection + we get the edges returned correctly with the right id2 --- ts/src/core/base.ts | 9 ++++ ts/src/core/clause.ts | 51 ++++++++++++------- ts/src/core/ent.ts | 47 +++++------------ ts/src/core/loaders/assoc_edge_loader.test.ts | 10 ++-- ts/src/core/query_impl.ts | 12 ++++- 5 files changed, 68 insertions(+), 61 deletions(-) diff --git a/ts/src/core/base.ts b/ts/src/core/base.ts index 03e8955ef..7bdab2037 100644 --- a/ts/src/core/base.ts +++ b/ts/src/core/base.ts @@ -172,6 +172,14 @@ export interface QueryableDataOptions extends SelectBaseDataOptions, QueryDataOptions {} +// for now, no complicated joins or no need to support multiple joins +// just one simple join +interface JoinOptions { + tableName: string; + alias?: string; + clause: clause.Clause; +} + export interface QueryDataOptions { distinct?: boolean; clause: clause.Clause; @@ -179,6 +187,7 @@ export interface QueryDataOptions { groupby?: K; limit?: number; disableTransformations?: boolean; + join?:JoinOptions; } // For loading data from database diff --git a/ts/src/core/clause.ts b/ts/src/core/clause.ts index 931f3a5b6..61323bc67 100644 --- a/ts/src/core/clause.ts +++ b/ts/src/core/clause.ts @@ -105,6 +105,8 @@ class simpleClause implements Clause { } } +// NB: we're not using alias in this class in clause method +// if we end up with a subclass that does, we need to handle it class queryClause implements Clause { constructor( protected dependentQueryOptions: QueryableDataOptions, // private value: any, // private op: string, // private handleNull?: Clause, @@ -143,18 +145,6 @@ class existsQueryClause extends queryClause { } } -class columnInQueryClause extends queryClause< - T, - K -> { - constructor( - protected dependentQueryOptions: QueryableDataOptions, - protected col: K, - ) { - super(dependentQueryOptions, `${col} IN`); - } -} - class isNullClause implements Clause { constructor(protected col: K) {} @@ -203,6 +193,30 @@ class isNotNullClause implements Clause { } } +class simpleExpression implements Clause { + constructor(protected expression: string) {} + + clause(idx: number, alias?: string): string { + return this.expression; + } + + columns(): K[] { + return []; + } + + values(): any[] { + return []; + } + + logValues(): any[] { + return []; + } + + instanceKey(): string { + return `${this.expression}`; + } +} + class arraySimpleClause implements Clause { constructor(protected col: K, private value: any, private op: string) {} @@ -833,13 +847,6 @@ export function DBTypeNotIn( return new notInClause(col, values, typ); } -export function ColInQuery( - col: K, - queryOptions: QueryableDataOptions, -): Clause { - return new columnInQueryClause(queryOptions, col); -} - interface TsQuery { // todo lang ::reconfig language: "english" | "french" | "german" | "simple"; @@ -1210,3 +1217,9 @@ export function getCombinedClause( } return cls; } + +export function Expression( + expression: string, +): Clause { + return new simpleExpression(expression); +} \ No newline at end of file diff --git a/ts/src/core/ent.ts b/ts/src/core/ent.ts index 61b808c6e..c119e6077 100644 --- a/ts/src/core/ent.ts +++ b/ts/src/core/ent.ts @@ -1370,17 +1370,11 @@ export function getDefaultLimit() { } function defaultEdgeQueryOptions( - edgeData: AssocEdgeData, id1: ID, edgeType: string, id2?: ID, - id1Clause?: (edgeData: AssocEdgeData) => clause.Clause, ): Required> { - let id1cls: clause.Clause = clause.Eq("id1", id1); - if (id1Clause) { - id1cls = id1Clause(edgeData); - } - let cls = clause.And(id1cls, clause.Eq("edge_type", edgeType)); + let cls = clause.And(clause.Eq("id1", id1), clause.Eq("edge_type", edgeType)); if (id2) { cls = clause.And(cls, clause.Eq("id2", id2)); } @@ -1448,7 +1442,6 @@ export async function loadCustomEdges( async function loadEdgesInfo( options: loadCustomEdgesOptions, id2?: ID, - id1Clause?: (edgeData: AssocEdgeData) => clause.Clause, ) { const { id1, edgeType } = options; const edgeData = await loadEdgeData(edgeType); @@ -1456,13 +1449,7 @@ async function loadEdgesInfo( throw new Error(`error loading edge data for ${edgeType}`); } - const defaultOptions = defaultEdgeQueryOptions( - edgeData, - id1, - edgeType, - id2, - id1Clause, - ); + const defaultOptions = defaultEdgeQueryOptions(id1, edgeType, id2); let cls = defaultOptions.clause; if (options.queryOptions?.clause) { cls = clause.And(cls, options.queryOptions.clause); @@ -1572,31 +1559,23 @@ export async function loadEdgeForID2( export async function loadTwoWayEdges( opts: loadCustomEdgesOptions, ): Promise { - const { - cls: actualClause, - fields, - tableName, - } = await loadEdgesInfo(opts, undefined, (edgeData: AssocEdgeData) => { - const { clause: subClause } = defaultEdgeQueryOptions( - edgeData, - opts.id1, - opts.edgeType, - ); - - const { cls } = getEdgeClauseAndFields(subClause, opts); - const subquery: QueryableDataOptions = { - tableName: edgeData.edgeTable, - fields: ["id2"], - clause: cls, - }; - return clause.ColInQuery("id1", subquery); - }); + const { cls: actualClause, fields, tableName } = await loadEdgesInfo(opts); const rows = await loadRows({ tableName, + alias: "t1", fields, clause: actualClause, context: opts.context, + join: { + tableName, + alias: "t2", + clause: clause.And( + // these are not values so need this to not think they're values... + clause.Expression("t1.id1 = t2.id2"), + clause.Expression("t1.id2 = t2.id1"), + ), + }, }); return rows as T[]; } diff --git a/ts/src/core/loaders/assoc_edge_loader.test.ts b/ts/src/core/loaders/assoc_edge_loader.test.ts index 71fa0e180..c42b5832d 100644 --- a/ts/src/core/loaders/assoc_edge_loader.test.ts +++ b/ts/src/core/loaders/assoc_edge_loader.test.ts @@ -479,8 +479,7 @@ function commonTests() { } const edges = await loader.load(user.id); const twoWay = await loader.loadTwoWay(user.id); - // TODO need to flip the edges probably. shouldn't have the source id always be id2 here... - expect(twowayIds.sort()).toEqual(twoWay.map((e) => e.id1).sort()); + expect(twowayIds.sort()).toEqual(twoWay.map((e) => e.id2).sort()); expect(edges.length).toBe(10); expect(twoWay.length).toBe(5); @@ -497,10 +496,10 @@ function commonTests() { twowayIds = []; for await (const edge of twoWay) { if (i % 2 === 0) { - twowayIds.push(edge.id1); + twowayIds.push(edge.id2); } else { action.builder.orchestrator.removeOutboundEdge( - edge.id1, + edge.id2, EdgeType.UserToFollowing, ); } @@ -514,8 +513,7 @@ function commonTests() { const edges2 = await loader.load(user.id); const twoWay2 = await loader.loadTwoWay(user.id); - // TODO need to flip the edges probably. shouldn't have the source id always be id2 here... - expect(twowayIds.sort()).toEqual(twoWay2.map((e) => e.id1).sort()); + expect(twowayIds.sort()).toEqual(twoWay2.map((e) => e.id2).sort()); // deleted some things here which shouldn't show up here expect(edges2.length).toBe(8); diff --git a/ts/src/core/query_impl.ts b/ts/src/core/query_impl.ts index eb94b8eb8..07f0bdb3d 100644 --- a/ts/src/core/query_impl.ts +++ b/ts/src/core/query_impl.ts @@ -40,12 +40,20 @@ export function buildQuery(options: QueryableDataOptions): string { : options.fields.join(", "); // always start at 1 - const whereClause = options.clause.clause(1, options.alias); const parts: string[] = []; const tableName = options.alias ? `${options.tableName} AS ${options.alias}` : options.tableName; - parts.push(`SELECT ${fields} FROM ${tableName} WHERE ${whereClause}`); + parts.push(`SELECT ${fields} FROM ${tableName}`); + + let whereStart = 1; + if (options.join) { + const joinTable = options.join.alias ? `${options.join.tableName} ${options.join.alias}` : options.join.tableName; + parts.push(`JOIN ${joinTable} ON ${options.join.clause.clause(1)}`); + whereStart += options.join.clause.values().length; + } + + parts.push(`WHERE ${options.clause.clause(whereStart, options.alias)}`); if (options.groupby) { parts.push(`GROUP BY ${options.groupby}`); }