feat(orm): Add JOIN support to Mapper/BaseBuilder and FK auto-detection in code generator by SBALAVIGNESH123 · Pull Request #2491 · drogonframework/drogon

SBALAVIGNESH123 · 2026-04-11T07:51:35Z

This addresses #241.

After studying the codebase, I noticed that Drogon already has a solid relationship system — the Relationship class, PivotTable, CSP template code generation, and the lazy loading getRelated() methods all work well through the JSON config. What's missing are three things: JOIN clause support in the query builder, join methods on the Mapper, and automatic foreign key detection in the code generator so users don't have to manually write relationship JSON for every FK constraint.This PR adds all three. The BaseBuilder now has a JoinClause struct and emits JOIN clauses in gen_sql() between FROM and WHERE. FilterBuilder and Mapper both get fluent innerJoin(), leftJoin(), rightJoin() methods that chain naturally with the existing API. On the code generator side, I added FK detection queries for all three backends — PostgreSQL via information_schema, MySQL via KEY_COLUMN_USAGE, and SQLite3 via PRAGMA foreign_key_list. The detected FKs get converted into Relationship objects and merged with any user-specified config, where user config always takes priority. The detection is best-effort so it won't break anything if a database doesn't support FK introspection.
Everything is additive and fully backward compatible — no existing APIs were changed or removed.

…on in code generator

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds SQL JOIN support to Drogon ORM query building and enhances model generation by auto-detecting foreign-key relationships across PostgreSQL/MySQL/SQLite.

Changes:

Added JOIN clause generation in BaseBuilder and fluent join methods in FilterBuilder.
Added JOIN support to Mapper by accumulating join fragments into generated SELECT/COUNT SQL.
Extended drogon_ctl create model to auto-detect foreign keys and merge them with user-configured relationships.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
orm_lib/inc/drogon/orm/Mapper.h	Adds fluent join methods and appends JOIN fragments to Mapper-generated SQL.
orm_lib/inc/drogon/orm/FilterBuilder.h	Adds fluent join methods backed by a structured join representation.
orm_lib/inc/drogon/orm/BaseBuilder.h	Introduces `JoinClause`/`JoinType` and emits JOIN clauses in generated SQL.
drogon_ctl/create_model.cc	Adds FK introspection per DB backend and merges detected relationships with user config.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-11T09:05:46Z

orm_lib/inc/drogon/orm/Mapper.h

+    Mapper<T> &innerJoin(const std::string &table,
+                         const std::string &onLeft,
+                         const std::string &onRight)
+    {
+        joinString_ += " INNER JOIN ";
+        joinString_ += table;
+        joinString_ += " ON ";
+        joinString_ += onLeft;
+        joinString_ += " = ";
+        joinString_ += onRight;
+        return *this;
+    }


These JOIN helpers concatenate raw table / onLeft / onRight directly into SQL, which allows SQL injection if any part is derived from user input (and also makes identifier quoting inconsistent across DBs). Consider switching Mapper JOINs to a structured representation (similar to BaseBuilder::JoinClause) and either (1) validate identifiers against a strict allowed-character set (e.g., [A-Za-z0-9_\\.]+), or (2) route identifiers through the library's identifier-quoting/escaping facility if one exists. At minimum, documenting that these arguments must be trusted SQL identifiers would reduce foot-guns.

Copilot · 2026-04-11T09:05:47Z

orm_lib/inc/drogon/orm/BaseBuilder.h

+inline std::string to_join_string(JoinType type)
+{
+    switch (type)
+    {
+        case JoinType::InnerJoin:
+            return "INNER JOIN";
+        case JoinType::LeftJoin:
+            return "LEFT JOIN";
+        case JoinType::RightJoin:
+            return "RIGHT JOIN";
+        case JoinType::FullJoin:
+            return "FULL JOIN";
+        default:
+            return "INNER JOIN";
+    }
+}


The default branch silently maps unknown values to INNER JOIN, which can hide bugs (e.g., if JoinType is extended or memory is corrupted). Prefer eliminating the default and letting compilers warn on non-exhaustive switches, or use an explicit failure path (e.g., unimplemented() / assertion) consistent with to_string(CompareOperator).

Copilot · 2026-04-11T09:05:47Z

orm_lib/inc/drogon/orm/BaseBuilder.h

        std::string sql = "select " + columns_ + " from " + from_;
+        for (const auto &join : joins_)
+        {
+            sql += " " + to_join_string(join.type) + " " + join.table +
+                   " ON " + join.onLeft + " = " + join.onRight;
+        }


JOIN SQL is assembled via raw string concatenation of join.table, join.onLeft, and join.onRight. If any of these can originate from user-controlled inputs (directly or indirectly), this becomes an injection vector. Consider enforcing identifier validation/quoting at the point joins are added (e.g., in FilterBuilder::*Join()), so BaseBuilder can safely assume the join parts are sanitized.

Copilot · 2026-04-11T09:05:47Z

drogon_ctl/create_model.cc

+               "ON kcu.constraint_name = rc.constraint_name "
+               "AND kcu.constraint_schema = rc.constraint_schema "
+               "JOIN information_schema.constraint_column_usage ccu "
+               "ON rc.unique_constraint_name = ccu.constraint_name "


PostgreSQL FK introspection join is incomplete: rc.unique_constraint_name = ccu.constraint_name should also be joined on the constraint schema (e.g., rc.unique_constraint_schema = ccu.constraint_schema). Without it, the query can mis-resolve referenced tables/columns when constraint names collide across schemas or return incorrect/multiple matches.

Suggested change

"ON rc.unique_constraint_name = ccu.constraint_name "

"ON rc.unique_constraint_name = ccu.constraint_name "

"AND rc.unique_constraint_schema = ccu.constraint_schema "

Copilot · 2026-04-11T09:05:47Z

drogon_ctl/create_model.cc

+                    relJson["type"] = "has one";
+                    relJson["original_table_name"] = tableName;
+                    relJson["original_key"] = fkColumn;
+                    relJson["target_table_name"] = referencedTable;
+                    relJson["target_key"] = referencedColumn;


Relationship JSON normalization is inconsistent across backends: MySQL/SQLite lower-case original_table_name and target_table_name, but PostgreSQL uses raw tableName/referencedTable. This can produce backend-dependent relationship configs and mismatches when the rest of the generator assumes normalized naming. Consider applying the same normalization strategy across all three branches (either always toLower() or never), ideally matching existing generator conventions.

Copilot · 2026-04-11T09:05:48Z

drogon_ctl/create_model.cc

+    std::string fkSql = "PRAGMA foreign_key_list(" + tableName + ");";
+    *client << fkSql << Mode::Blocking >> [&](const Result &fkResult) {


SQLite FK detection builds PRAGMA foreign_key_list(<table>) by concatenating tableName directly. Since tableName comes from CLI input, this is susceptible to SQL injection via crafted table names. Prefer quoting/escaping the identifier (e.g., wrapping in double-quotes and escaping embedded quotes) or validating tableName against a strict identifier pattern before using it in SQL.

Copilot · 2026-04-11T09:05:48Z

drogon_ctl/create_model.cc

+                bool exists = false;
+                for (const auto &r : allRelationships)
+                {
+                    if (r.originalKey() == fkColumn &&
+                        r.targetTableName() == referencedTable)
+                    {
+                        exists = true;
+                        break;
+                    }
+                }
+                if (!exists)
+                {
+                    Json::Value relJson;
+                    relJson["type"] = "has one";
+                    relJson["original_table_name"] = toLower(tableName);
+                    relJson["original_key"] = fkColumn;
+                    relJson["target_table_name"] = toLower(referencedTable);
+                    relJson["target_key"] = referencedColumn;
+                    relJson["enable_reverse"] = true;


The FK-to-Relationship conversion logic (dedupe scan, relJson construction, try/catch + push + logging) is duplicated across PG/MySQL/SQLite with small variations. Consider extracting a small helper (e.g., tryAddAutoRelationship(allRelationships, originalTable, fkColumn, referencedTable, referencedColumn, normalizeNames)) to reduce repetition and keep future behavior changes consistent across backends.

…schema join fix, extract helper, quote PRAGMA

SBALAVIGNESH123 · 2026-04-11T09:39:12Z

Addressed all Copilot review feedback in the latest commit (2d88a91):

Added isValidSqlIdentifier() to validate JOIN parameters against SQL injection
Removed silent default branch in o_join_string() switch
Fixed PostgreSQL FK query: added missing AND rc.unique_constraint_schema = ccu.constraint_schema join condition
Normalized table names consistently across all three backends
Quoted tableName in SQLite3 PRAGMA foreign_key_list() to prevent injection
Extracted ryAddAutoRelationship() helper to eliminate code duplication across PG/MySQL/SQLite3

SBALAVIGNESH123 · 2026-04-11T12:14:03Z

Ran the format check locally using Docker with the exact CI setup (clang-format-17 + dos2unix on Ubuntu 24.04) and fixed all formatting differences. The latest commit (b2c6157) should pass the format check cleanly now.

Changes across commits:

b7ef924: Initial implementation (JOIN support + FK auto-detection)
2d88a91: Addressed all Copilot review feedback (SQL injection prevention, PG schema fix, extracted helper, quoted PRAGMA)
d005037 + b2c6157: Applied exact clang-format-17 formatting via Docker

Could you approve the workflow run on the latest commit? (First-time contributor, so CI needs maintainer approval to trigger.) Thanks!

an-tao · 2026-04-12T03:40:50Z

@SBALAVIGNESH123 Thank you very much for the patch. Please take a look at the CoroMapper template to ensure consistency with this PR.

SBALAVIGNESH123 · 2026-04-12T04:46:59Z

Thanks for the review @an-tao! Good catch on CoroMapper — I should have included that from the start. The latest commit adds innerJoin(), leftJoin(), and rightJoin() overrides to CoroMapper that delegate to the Mapper base class and return CoroMapper& for proper chaining. I also injected joinString_ into the three CoroMapper query methods (findOne, findBy, and count) so JOINs work the same way whether you're using the sync, async, or coroutine API. The insert, update, and delete paths are untouched since JOINs don't apply there. Everything was formatted with clang-format-17 locally via Docker to match your CI setup. Let me know if anything else needs adjusting.

Mapper.h uses isValidSqlIdentifier() from BaseBuilder.h but didn't include it. This builds fine on older compilers through transitive includes, but GCC 15 with -Wtemplate-body correctly rejects the undeclared name during two-phase template lookup.

SBALAVIGNESH123 · 2026-04-12T10:45:03Z

Added one more fix in 4fcd7ec — Mapper.h was using isValidSqlIdentifier() from BaseBuilder.h but never included it. This compiles fine on most compilers through transitive includes, but GCC 15 with its new -Wtemplate-body diagnostic correctly rejects the undeclared name during two-phase template lookup. Added the missing #include <drogon/orm/BaseBuilder.h> in Mapper.h to fix it. Build-tested locally with GCC 15.2.0 (MinGW-w64) — everything compiles clean now including libdrogon.a and drogon_ctl.exe.

feat(orm): add JOIN support in BaseBuilder/Mapper and FK auto-detecti…

b7ef924

…on in code generator

an-tao requested a review from Copilot April 11, 2026 08:58

Copilot started reviewing on behalf of an-tao April 11, 2026 09:04 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

fix: address all Copilot review comments - SQL injection prevention, …

2d88a91

…schema join fix, extract helper, quote PRAGMA

Bala Vignesh S added 2 commits April 11, 2026 17:27

style: fix clang-format issues

d005037

style: apply exact clang-format-17 formatting

b2c6157

feat(orm): add JOIN support to CoroMapper for consistency with Mapper

e95e5f3

SBALAVIGNESH123 mentioned this pull request Apr 12, 2026

http/2 and http/3 support #349

Open

	"ON rc.unique_constraint_name = ccu.constraint_name "
	"ON rc.unique_constraint_name = ccu.constraint_name "
	"AND rc.unique_constraint_schema = ccu.constraint_schema "

		std::string fkSql = "PRAGMA foreign_key_list(" + tableName + ");";
		*client << fkSql << Mode::Blocking >> [&](const Result &fkResult) {

Uh oh!

Conversation

SBALAVIGNESH123 commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

SBALAVIGNESH123 commented Apr 11, 2026

Uh oh!

SBALAVIGNESH123 commented Apr 11, 2026

Uh oh!

an-tao commented Apr 12, 2026

Uh oh!

SBALAVIGNESH123 commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SBALAVIGNESH123 commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SBALAVIGNESH123 commented Apr 12, 2026 •

edited

Loading