From 05770baba3e9fd5199a9492ea6b6c8b276ba9e5d Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Tue, 26 Nov 2024 19:45:22 -0700 Subject: [PATCH 1/2] Correctly handle NativeQueries in Relationships --- .../kotlin/io/hasura/mysql/JSONGenerator.kt | 20 +--- .../hasura/ndc/sqlgen/BaseQueryGenerator.kt | 106 +++++++++++++++++- 2 files changed, 107 insertions(+), 19 deletions(-) diff --git a/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt b/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt index db1f61f..08f96a3 100644 --- a/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt +++ b/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt @@ -30,23 +30,11 @@ object JsonQueryGenerator : BaseQueryGenerator() { fun queryRequestToSQLInternal( request: QueryRequest, ): SelectSelectStep<*> { - // If the QueryRequest "collection" references the name of a Native Query defined in the configuration.json, - // we need to prefix the generated query with a CTE named identically to the Native Query, containing the Native Query itself - val isNativeQuery = ConnectorConfiguration.Loader.config.nativeQueries.containsKey(request.collection) - - return if (isNativeQuery) { - mkNativeQueryCTE(request).select( - jsonArrayAgg( - buildJSONSelectionForQueryRequest(request) - ) - ) - } else { - DSL.select( - jsonArrayAgg( - buildJSONSelectionForQueryRequest(request) - ) + return mkNativeQueryCTEs(request).select( + jsonArrayAgg( + buildJSONSelectionForQueryRequest(request) ) - } + ) } fun buildJSONSelectionForQueryRequest( diff --git a/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt b/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt index 270b149..d949617 100644 --- a/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt +++ b/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt @@ -25,6 +25,108 @@ abstract class BaseQueryGenerator : BaseGenerator { throw NotImplementedError("Mutation not supported for this data source") } + protected fun findAllNativeQueries(request: QueryRequest): Set { + val nativeQueries = mutableSetOf() + val config = ConnectorConfiguration.Loader.config + + // Helper function to check if a collection is a native query + fun checkAndAddNativeQuery(collection: String) { + if (config.nativeQueries.containsKey(collection)) { + nativeQueries.add(collection) + } + } + + // Check main collection + checkAndAddNativeQuery(request.collection) + + // Check relationships + request.collection_relationships.values.forEach { rel -> + checkAndAddNativeQuery(rel.target_collection) + } + + // Recursive function to check predicates + fun checkPredicates(expression: Expression?) { + when (expression) { + is Expression.Exists -> { + when (val collection = expression.in_collection) { + is ExistsInCollection.Related -> { + // Check related collection from relationship + val rel = request.collection_relationships[collection.relationship] + ?: error("Relationship ${collection.relationship} not found") + checkAndAddNativeQuery(rel.target_collection) + } + is ExistsInCollection.Unrelated -> { + checkAndAddNativeQuery(collection.collection) + } + } + // Recursively check the predicate within exists + checkPredicates(expression.predicate) + } + is Expression.And -> expression.expressions.forEach { checkPredicates(it) } + is Expression.Or -> expression.expressions.forEach { checkPredicates(it) } + is Expression.Not -> checkPredicates(expression.expression) + else -> {} // Other expression types don't reference collections + } + } + + // Check predicates in the main query + checkPredicates(request.query.predicate) + + // Check predicates in relationship fields + request.query.fields?.values?.forEach { field -> + if (field is IRField.RelationshipField) { + checkPredicates(field.query.predicate) + } + } + + return nativeQueries + } + + fun mkNativeQueryCTEs( + request: QueryRequest + ): org.jooq.WithStep { + val config = ConnectorConfiguration.Loader.config + var nativeQueries = findAllNativeQueries(request) + + if (nativeQueries.isEmpty()) { + return DSL.with() + } + + fun renderNativeQuerySQL( + nativeQuery: NativeQueryInfo, + arguments: Map + ): String { + val sql = nativeQuery.sql + val parts = sql.parts + + return parts.joinToString("") { part -> + when (part) { + is NativeQueryPart.Text -> part.value + is NativeQueryPart.Parameter -> { + val argument = arguments[part.value] ?: error("Argument ${part.value} not found") + when (argument) { + is Argument.Literal -> argument.value.toString() + else -> error("Only literals are supported in Native Queries in this version") + } + } + } + } + } + + var withStep = DSL.with() + nativeQueries.forEach { collectionName -> + withStep = withStep.with(DSL.name(collectionName)) + .`as`(DSL.resultQuery( + renderNativeQuerySQL( + config.nativeQueries[collectionName]!!, + request.arguments + ) + )) + } + + return withStep + } + fun mkNativeQueryCTE( request: QueryRequest ): org.jooq.WithStep { @@ -446,9 +548,7 @@ abstract class BaseQueryGenerator : BaseGenerator { e = where, request ) - } ?: DSL.noCondition())).also { - println("Where conditions: $it") - } + } ?: DSL.noCondition())) } protected fun getDefaultAggregateJsonEntries(aggregates: Map?): Field<*> { From 7c68e8823d81c8831106dfada9dac0d43740deb6 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 27 Nov 2024 09:52:48 -0700 Subject: [PATCH 2/2] Review feedback --- .../src/main/kotlin/io/hasura/mysql/JSONGenerator.kt | 1 + .../main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt b/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt index 08f96a3..ffa7577 100644 --- a/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt +++ b/ndc-connector-mysql/src/main/kotlin/io/hasura/mysql/JSONGenerator.kt @@ -30,6 +30,7 @@ object JsonQueryGenerator : BaseQueryGenerator() { fun queryRequestToSQLInternal( request: QueryRequest, ): SelectSelectStep<*> { + // JOOQ is smart enough to not generate CTEs if there are no native queries return mkNativeQueryCTEs(request).select( jsonArrayAgg( buildJSONSelectionForQueryRequest(request) diff --git a/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt b/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt index d949617..3b5bac1 100644 --- a/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt +++ b/ndc-sqlgen/src/main/kotlin/io/hasura/ndc/sqlgen/BaseQueryGenerator.kt @@ -89,6 +89,7 @@ abstract class BaseQueryGenerator : BaseGenerator { var nativeQueries = findAllNativeQueries(request) if (nativeQueries.isEmpty()) { + // JOOQ is smart enough to not generate CTEs if there are no native queries return DSL.with() } @@ -113,9 +114,9 @@ abstract class BaseQueryGenerator : BaseGenerator { } } - var withStep = DSL.with() + val withStep = DSL.with() nativeQueries.forEach { collectionName -> - withStep = withStep.with(DSL.name(collectionName)) + withStep.with(DSL.name(collectionName)) .`as`(DSL.resultQuery( renderNativeQuerySQL( config.nativeQueries[collectionName]!!,