From 686f257f024eb61953a45bff1b4f426c9d21c303 Mon Sep 17 00:00:00 2001 From: Tristen Harr Date: Thu, 6 Jun 2024 21:20:44 -0500 Subject: [PATCH] add relationships, add filter relationships, add relationship ordering --- .gitignore | 5 +- CHANGELOG.md | 6 + connector-definition/connector-metadata.yaml | 4 +- generate-config.ts | 2 + package-lock.json | 18 +- package.json | 4 +- src/handlers/query.ts | 185 +++++++++++++------ 7 files changed, 157 insertions(+), 67 deletions(-) diff --git a/.gitignore b/.gitignore index 528bcd8..728a884 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,7 @@ configuration.json SECRET_REMOTE_CONFIGURATION.http node_modules .DS_Store -config.json \ No newline at end of file +config.json +TMP.md +chinook.db +chinook.db.wal \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 96b52e0..983d3ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # DuckDB Connector Changelog This changelog documents changes between release tags. +## [0.0.12] - 2024-05-6 +* Add Relationships support +* Fix OrderBy across relationships +* Add Filtering on Relationships +* Ship a multi-arch build + ## [0.0.11] - 2024-04-17 * Update generate-config to use the proper configuration directory diff --git a/connector-definition/connector-metadata.yaml b/connector-definition/connector-metadata.yaml index 77657e9..70e759e 100644 --- a/connector-definition/connector-metadata.yaml +++ b/connector-definition/connector-metadata.yaml @@ -1,13 +1,13 @@ packagingDefinition: type: PrebuiltDockerImage - dockerImage: ghcr.io/hasura/ndc-duckdb:v0.0.11 + dockerImage: ghcr.io/hasura/ndc-duckdb:v0.0.12 supportedEnvironmentVariables: - name: DUCKDB_URL description: The url for the DuckDB database commands: update: type: Dockerized - dockerImage: ghcr.io/hasura/ndc-duckdb:v0.0.11 + dockerImage: ghcr.io/hasura/ndc-duckdb:v0.0.12 commandArgs: - update dockerComposeWatch: diff --git a/generate-config.ts b/generate-config.ts index 5b4b3ac..bb75746 100644 --- a/generate-config.ts +++ b/generate-config.ts @@ -59,6 +59,8 @@ const determineType = (t: string): string => { return "String"; case "VARCHAR": return "String"; + case "HUGEINT": + return "String"; default: if (t.startsWith("DECIMAL")){ return "Float"; diff --git a/package-lock.json b/package-lock.json index 2bf6f39..f1d7e4c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,15 @@ { "name": "duckdb-sdk", - "version": "0.0.11", + "version": "0.0.12", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "duckdb-sdk", - "version": "0.0.11", + "version": "0.0.12", "dependencies": { "@hasura/ndc-sdk-typescript": "^4.5.0", - "duckdb": "^0.9.2", + "duckdb": "^1.0.0", "sql-formatter": "^13.0.4", "sqlstring-sqlite": "^0.1.1" }, @@ -1574,9 +1574,9 @@ "integrity": "sha512-c68LpLbO+7kP/b1Hr1qs8/BJ09F5khZGTxqxZuhzxpmwJKOgRFHJWIb9/KmqnqHhLdO55aOxFH/EGBvUQbL/RQ==" }, "node_modules/duckdb": { - "version": "0.9.2", - "resolved": "https://registry.npmjs.org/duckdb/-/duckdb-0.9.2.tgz", - "integrity": "sha512-POZ37Vf5cHVmk4W8z0PsdGi67VeQsr9HRrBbmivDt9AQ8C1XLESVE79facydbroNiqm7+n7fx6jqjyxyrBFYlQ==", + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/duckdb/-/duckdb-1.0.0.tgz", + "integrity": "sha512-QwpcIeN42A2lL19S70mUFibZgRcEcZpCkKHdzDgecHaYZhXj3+1i2cxSDyAk/RVg5CYnqj1Dp4jAuN4cc80udA==", "hasInstallScript": true, "dependencies": { "@mapbox/node-pre-gyp": "^1.0.0", @@ -4438,9 +4438,9 @@ "integrity": "sha512-c68LpLbO+7kP/b1Hr1qs8/BJ09F5khZGTxqxZuhzxpmwJKOgRFHJWIb9/KmqnqHhLdO55aOxFH/EGBvUQbL/RQ==" }, "duckdb": { - "version": "0.9.2", - "resolved": "https://registry.npmjs.org/duckdb/-/duckdb-0.9.2.tgz", - "integrity": "sha512-POZ37Vf5cHVmk4W8z0PsdGi67VeQsr9HRrBbmivDt9AQ8C1XLESVE79facydbroNiqm7+n7fx6jqjyxyrBFYlQ==", + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/duckdb/-/duckdb-1.0.0.tgz", + "integrity": "sha512-QwpcIeN42A2lL19S70mUFibZgRcEcZpCkKHdzDgecHaYZhXj3+1i2cxSDyAk/RVg5CYnqj1Dp4jAuN4cc80udA==", "requires": { "@mapbox/node-pre-gyp": "^1.0.0", "node-addon-api": "^7.0.0", diff --git a/package.json b/package.json index 1afc381..280d552 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "duckdb-sdk", - "version": "0.0.11", + "version": "0.0.12", "description": "", "main": "index.js", "scripts": { @@ -14,7 +14,7 @@ }, "dependencies": { "@hasura/ndc-sdk-typescript": "^4.5.0", - "duckdb": "^0.9.2", + "duckdb": "^1.0.0", "sql-formatter": "^13.0.4", "sqlstring-sqlite": "^0.1.1" } diff --git a/src/handlers/query.ts b/src/handlers/query.ts index 99d2409..08f7885 100644 --- a/src/handlers/query.ts +++ b/src/handlers/query.ts @@ -5,12 +5,13 @@ import { Query, RowSet, Forbidden, - Conflict + Conflict, + Relationship } from "@hasura/ndc-sdk-typescript"; import { Configuration, State } from ".."; const SqlString = require("sqlstring-sqlite"); -// import { format } from "sql-formatter"; import { MAX_32_INT } from "../constants"; +// import { format } from "sql-formatter"; const escape_single = (s: any) => SqlString.escape(s); const escape_double = (s: any) => `"${SqlString.escape(s).slice(1, -1)}"`; @@ -53,6 +54,20 @@ const json_replacer = (key: string, value: any): any => { return value; }; +const formatSQLWithArgs = (sql: string, args: any[]): string => { + let index = 0; + return sql.replace(/\?/g, () => { + const arg = args[index++]; + if (typeof arg === 'string') { + return `'${arg}'`; + } else if (arg === null) { + return 'NULL'; + } else { + return arg; + } + }); +}; + function wrap_data(s: string): string { return ` SELECT @@ -65,7 +80,7 @@ function wrap_data(s: string): string { function wrap_rows(s: string): string { return ` SELECT - JSON_OBJECT('rows', JSON_GROUP_ARRAY(JSON(r))) + JSON_OBJECT('rows', COALESCE(JSON_GROUP_ARRAY(JSON(r)), JSON('[]'))) FROM ( ${s} @@ -75,8 +90,13 @@ function wrap_rows(s: string): string { function build_where( expression: Expression, + collection_relationships: { + [k: string]: Relationship; + }, args: any[], - variables: QueryVariables + variables: QueryVariables, + prefix: string, + collection_aliases: {[k: string]: string} ): string { let sql = ""; switch (expression.type) { @@ -111,9 +131,6 @@ function build_where( sql = `${expression.column.name} = ?`; break; case "_like": - // TODO: Should this be setup like this? Or is this wrong because the % wildcard matches should be set by user? - // I.e. Should we let the user pass through their own % to more closely follow the sqlite spec, and create a new operator.. - // _contains => That does the LIKE %match% args[args.length - 1] = `%${args[args.length - 1]}%`; sql = `${expression.column.name} LIKE ?`; break; @@ -148,7 +165,7 @@ function build_where( } else { const clauses = []; for (const expr of expression.expressions) { - const res = build_where(expr, args, variables); + const res = build_where(expr, collection_relationships, args, variables, prefix, collection_aliases); clauses.push(res); } sql = `(${clauses.join(` AND `)})`; @@ -160,22 +177,38 @@ function build_where( } else { const clauses = []; for (const expr of expression.expressions) { - const res = build_where(expr, args, variables); + const res = build_where(expr, collection_relationships, args, variables, prefix, collection_aliases); clauses.push(res); } sql = `(${clauses.join(` OR `)})`; } break; case "not": - const not_result = build_where(expression.expression, args, variables); + const not_result = build_where(expression.expression, collection_relationships, args, variables, prefix, collection_aliases); sql = `NOT (${not_result})`; break; - // case "binary_array_comparison_operator": - // // IN - // throw new BadRequest("In not implemented", {}); case "exists": - // EXISTS - throw new Forbidden("Not implemented", {}); + const { in_collection, predicate } = expression; + let subquery_sql = ""; + let subquery_alias = `${prefix}_exists`; + + if (in_collection.type === "related") { + const relationship = collection_relationships[in_collection.relationship]; + let from_collection_alias = collection_aliases[relationship.target_collection]; + subquery_sql = ` + SELECT 1 + FROM ${from_collection_alias} AS ${escape_double(subquery_alias)} + WHERE ${predicate ? build_where(predicate, collection_relationships, args, variables, prefix, collection_aliases) : '1 = 1'} + AND ${Object.entries(relationship.column_mapping).map(([from, to]) => { + return `${escape_double(prefix)}.${escape_double(from)} = ${escape_double(subquery_alias)}.${escape_double(to)}`; + }).join(" AND ")} + `; + } else if (in_collection.type === "unrelated") { + throw new Forbidden("Unrelated collection type not supported!", {}); + } + + sql = `EXISTS (${subquery_sql})`; + break; default: throw new Forbidden("Unknown Expression Type!", {}); } @@ -190,9 +223,14 @@ function build_query( path: string[], variables: QueryVariables, args: any[], - agg_args: any[] + agg_args: any[], + relationship_key: string | null, + collection_relationships: { + [k: string]: Relationship; + }, + collection_aliases: {[k: string]: string} ): SQLQuery { - if (config.config === null || config.config === undefined) { + if (!config.config) { throw new Forbidden("Must supply config", {}); } let sql = ""; @@ -201,7 +239,6 @@ function build_query( let run_agg = false; path.push(collection); let collection_alias = path.join("_"); - // let indent = " ".repeat(path.length - 1); let limit_sql = ``; let offset_sql = ``; @@ -209,8 +246,6 @@ function build_query( let collect_rows = []; let where_conditions = ["WHERE 1"]; if (query.aggregates) { - // TODO: Add each aggregate to collectRows - // Aggregates need to be handled seperately. run_agg = true; agg_sql = "... todo"; throw new Forbidden("Aggregates not implemented yet!", {}); @@ -221,24 +256,31 @@ function build_query( collect_rows.push(escape_single(field_name)); switch (field_value.type) { case "column": - collect_rows.push(escape_double(field_value.column)); + collect_rows.push(`${escape_double(collection_alias)}.${escape_double(field_value.column)}`); break; case "relationship": + let relationship_collection = query_request.collection_relationships[field_value.relationship].target_collection; + let relationship_collection_alias = config.config.collection_aliases[relationship_collection]; collect_rows.push( - `(${ - build_query( - config, - query_request, - config.config.collection_aliases[field_value.relationship], // Lookup alias - field_value.query, - path, - variables, - args, - agg_args - ).sql - })` + `COALESCE(( + ${ + build_query( + config, + query_request, + relationship_collection_alias, + field_value.query, + path, + variables, + args, + agg_args, + field_value.relationship, + collection_relationships, + collection_aliases + ).sql + }), JSON('[]') + )` ); - path.pop(); // POST-ORDER search stack pop! + path.pop(); break; default: throw new Conflict("The types tricked me. 😭", {}); @@ -246,22 +288,49 @@ function build_query( } } let from_sql = `${collection} as ${escape_double(collection_alias)}`; - if (path.length > 1) { - throw new Forbidden("Relationships are not supported yet.", {}); + if (path.length > 1 && relationship_key !== null) { + let relationship = query_request.collection_relationships[relationship_key]; + let parent_alias = path.slice(0, -1).join("_"); + let relationship_alias = config.config.collection_aliases[relationship.target_collection]; + from_sql = `${relationship_alias} as ${escape_double(collection_alias)}`; + where_conditions.push( + ...Object.entries(relationship.column_mapping).map(([from, to]) => { + return `${escape_double(parent_alias)}.${escape_double(from)} = ${escape_double(collection_alias)}.${escape_double(to)}`; + }) + ); } + const filter_joins: string[] = []; + if (query.predicate) { - where_conditions.push(`(${build_where(query.predicate, args, variables)})`); + where_conditions.push(`(${build_where(query.predicate, query_request.collection_relationships, args, variables, collection_alias, config.config.collection_aliases)})`); } - if (query.order_by) { + if (query.order_by && config.config) { let order_elems: string[] = []; for (let elem of query.order_by.elements) { switch (elem.target.type) { case "column": - order_elems.push( - `${escape_double(elem.target.name)} ${elem.order_direction}` - ); + if (elem.target.path.length === 0){ + order_elems.push( + `${escape_double(collection_alias)}.${escape_double(elem.target.name)} ${elem.order_direction}` + ); + } else { + let currentAlias = collection_alias; + for (let path_elem of elem.target.path) { + const relationship = collection_relationships[path_elem.relationship]; + const nextAlias = `${currentAlias}_${relationship.target_collection}`; + const target_collection_alias = collection_aliases[relationship.target_collection]; + const join_str = `JOIN ${target_collection_alias} AS ${escape_double(nextAlias)} ON ${Object.entries(relationship.column_mapping).map(([from, to]) => `${escape_double(currentAlias)}.${escape_double(from)} = ${escape_double(nextAlias)}.${escape_double(to)}`).join(" AND ")}`; + if (!filter_joins.includes(join_str)) { + filter_joins.push(join_str); + } + currentAlias = nextAlias; + } + order_elems.push( + `LOWER(${escape_double(currentAlias)}.${escape_double(elem.target.name)}) ${elem.order_direction}` + ); + } break; case "single_column_aggregate": throw new Forbidden( @@ -294,18 +363,19 @@ function build_query( } sql = wrap_rows(` - SELECT - JSON_OBJECT(${collect_rows.join(",")}) as r - FROM ${from_sql} - ${where_conditions.join(" AND ")} - ${order_by_sql} - ${limit_sql} - ${offset_sql} - `); +SELECT +JSON_OBJECT(${collect_rows.join(",")}) as r +FROM ${from_sql} +${filter_joins.join(" ")} +${where_conditions.join(" AND ")} +${order_by_sql} +${limit_sql} +${offset_sql} +`); if (path.length === 1) { sql = wrap_data(sql); - // console.log(format(sql, { language: "sqlite" })); + // console.log(format(formatSQLWithArgs(sql, args), { language: "sqlite" })); } return { @@ -331,6 +401,7 @@ export async function plan_queries( if (query.variables) { let promises = query.variables.map((varSet) => { let query_variables: QueryVariables = varSet; + if (configuration.config){ return build_query( configuration, query, @@ -339,8 +410,14 @@ export async function plan_queries( [], query_variables, [], - [] + [], + null, + query.collection_relationships, + configuration.config.collection_aliases ); + } else { + throw new Forbidden("Config must be defined", {}); + } }); query_plan = await Promise.all(promises); } else { @@ -352,7 +429,10 @@ export async function plan_queries( [], {}, [], - [] + [], + null, + query.collection_relationships, + configuration.config.collection_aliases ); query_plan = [promise]; } @@ -390,7 +470,6 @@ export async function do_query( state: State, query: QueryRequest ): Promise { - // console.log(JSON.stringify(query, null, 4)); let query_plans = await plan_queries(configuration, query); return await perform_query(state, query_plans); }