Duckdb add branch_alpha by weidqi · Pull Request #2369 · drogonframework/drogon

weidqi · 2025-08-10T07:19:39Z

Add support for the DuckDB

Removed CLAUDE.md from .gitignore

Copilot

Pull request overview

This pull request adds comprehensive DuckDB database support to the Drogon web framework. DuckDB is an embedded analytical database similar to SQLite but optimized for analytical workloads. The implementation follows the existing patterns used for PostgreSQL, MySQL, and SQLite3 support, providing connection management, result handling, parameter binding, batch operations, and flexible configuration options.

Key Changes:

Implementation of DuckDB connection and result classes using the DuckDB C API
Flexible configuration system supporting DuckDB-specific options (threads, memory limits, access mode)
Batch SQL execution support using duckdb_extract_statements API
Integration with existing Drogon ORM infrastructure and code generation tools

Reviewed changes

Copilot reviewed 34 out of 36 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`orm_lib/src/duckdb_impl/DuckdbConnection.cc`	Core connection implementation with SQL execution, parameter binding, and batch operations
`orm_lib/src/duckdb_impl/DuckdbResultImpl.cc`	Result set implementation with type conversion and value caching
`orm_lib/inc/drogon/orm/DbConfig.h`	Added DuckdbConfig structure with configOptions support
`orm_lib/src/DbClient.cc`	Added factory methods for creating DuckDB clients
`orm_lib/src/DbClientImpl.cc`	Client implementation with configuration option handling
`lib/src/ConfigLoader.cc`	JSON config parsing for DuckDB config_options
`CMakeLists.txt`	Build system updates for DuckDB detection and compilation
`orm_lib/src/duckdb_impl/test/duck_test.cc`	Comprehensive test suite with 20+ test categories
`third_party/utf8proc/*`	UTF8 processing library files (appears to be third-party code)

Comments suppressed due to low confidence (1)

orm_lib/src/DbClientImpl.cc:1

Code and its comment were removed without clear justification. The comment indicates this handles cleanup of invalid connection pointers. If this is intentional cleanup, ensure the disconnect logic is handled elsewhere or the removal is documented.

/**

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

orm_lib/tests/db_test.cc

orm_lib/src/DbClientImpl.cc

orm_lib/src/duckdb_impl/DuckdbConnection.cc

orm_lib/src/duckdb_impl/changelog.md

orm_lib/src/duckdb_impl/readme.md

CMakeLists.txt

Copilot · 2025-11-25T07:18:13Z

third_party/utf8proc/utf8proc_wrapper.cpp

+#include "duckdb/common/exception.hpp"
+#include "duckdb/common/helper.hpp"
+
+using namespace std;


Using 'using namespace std;' in a header or source file that may be included elsewhere is generally discouraged as it pollutes the global namespace. Consider using explicit std:: prefixes instead.

Copilot · 2025-11-25T07:18:14Z

orm_lib/src/duckdb_impl/DuckdbConnection.cc

+            thisPtr->batchSqlCommands_ = std::move(sqlCommands);
+            thisPtr->executeBatchSql();
+        });
+}
+
+/**
+ * @brief 执行批量 SQL 的主逻辑
+ */
+void DuckdbConnection::executeBatchSql()
+{
+    loop_->assertInLoopThread();
+


[nitpick] The sqlCommands are moved into the lambda, then moved again into batchSqlCommands_. This is correct but creates an extra move operation. Consider moving directly: thisPtr->executeBatchSql(std::move(sqlCommands)); and updating executeBatchSql to accept parameters.

Suggested change

thisPtr->batchSqlCommands_ = std::move(sqlCommands);

thisPtr->executeBatchSql();

});

}

/**

* @brief 执行批量 SQL 的主逻辑

*/

void DuckdbConnection::executeBatchSql()

{

loop_->assertInLoopThread();

thisPtr->executeBatchSql(std::move(sqlCommands));

});

}

/**

* @brief 执行批量 SQL 的主逻辑

*/

void DuckdbConnection::executeBatchSql(std::deque<std::shared_ptr<SqlCmd>> &&sqlCommands)

{

loop_->assertInLoopThread();

batchSqlCommands_ = std::move(sqlCommands);

hwc0919 · 2025-11-25T07:25:40Z

It's really bad to integrate various niche products into drogon source code.

I don't encourage it, nor do I like such attemps.

We should enhance our plugin api, and let user write 3rd-party plugins.

marty1885

Wow this is a crap ton of code..

Why is utf8proc added? I don't see it being used and complicating our license

marty1885 · 2025-11-25T07:24:59Z

.gitignore

 *.out
 *.app

+trantor/


This is a submodule. Why is it in the ignore list?

let me fix it

CMakeLists.txt

marty1885 · 2025-11-25T07:25:58Z

CMakeLists.txt

+        target_link_libraries(${PROJECT_NAME} PRIVATE ${DuckDB_LIBRARIES})
+        target_include_directories(${PROJECT_NAME} PRIVATE ${DuckDB_INCLUDE_DIRS})
+        set(DROGON_FOUND_DUCKDB TRUE)
+        # 使用 duckdb_impl 目录下的 DuckDB C API 直接实现


For global development. I prefer keeping comments English

marty1885 · 2025-11-25T07:26:31Z

CMakeLists.txt

+    if(NOT DuckDB_FOUND)
+        # Try to find DuckDB manually
+        find_library(DUCKDB_LIBRARY
+            NAMES duckdb
+            PATHS /usr/lib /usr/local/lib /opt/homebrew/lib
+        )
+        find_path(DUCKDB_INCLUDE_DIR
+            NAMES duckdb.h
+            PATHS /usr/include /usr/local/include /opt/homebrew/include
+        )
+
+        if(DUCKDB_LIBRARY AND DUCKDB_INCLUDE_DIR)
+            set(DuckDB_LIBRARIES ${DUCKDB_LIBRARY})
+            set(DuckDB_INCLUDE_DIRS ${DUCKDB_INCLUDE_DIR})
+            set(DuckDB_FOUND TRUE)
+        endif()
+    endif()


This should be handled in FindDuckDB.

marty1885 · 2025-11-25T07:27:37Z

drogon_ctl/create_model.cc

+    std::vector<ColumnInfo> cols;
+
+    // 使用 PRAGMA table_info 查询表结构（DuckDB 兼容 SQLite）
+    std::string sql = "PRAGMA table_info(" + tableName + ");";


It feels possible to SQLi here?

marty1885 · 2025-11-25T07:31:47Z

drogon_ctl/create_model.cc

+        [&](bool isNull, std::string &&tableName) mutable {
+            if (!isNull)
+            {
+                std::cout << "table name:" << tableName << std::endl;


Please use trantor's LOG_* instead of cout or cerr.

oh，thanks

weidqi · 2025-11-26T03:56:39Z

It's really bad to integrate various niche products into drogon source code.

I don't encourage it, nor do I like such attemps.

We should enhance our plugin api, and let user write 3rd-party plugins.

I would certainly like to use a plugin approach for extension, but I haven't found a good implementation path for extending the ORM using plugins. If you know of one, please let me know and help us improve Drogon's support for more databases.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…TARGET_FILE:_drogon_ctl>

Removed directories 'trantor/'

weidqi and others added 14 commits July 3, 2025 22:40

初始化duckdb的两种实现目录

c451a4e

添加sqlite_api_wrapper源代码到工程,2025-07-03晚于沙特吉达

35c3cfe

添加sqlite_wrapper源代码,编译通过

28f96d7

初步实现了duckdb wrapper,有待进一步测试

e03d948

修改SqlBinder,增加对ClientType::DuckDB的处理逻辑

cc5fae8

Added comprehensive test cases.

6c01a8a

fixed some bug.

5a85d8a

Lint

1dfad64

More lint

7abc38b

为 Drogon 框架添加 DuckDB 数据库支持，使用原生c api

2db43f4

功能增强：DuckDB 配置参数灵活化支持

cf42232

Merge branch 'drogonframework:master' into duckdb-add-branch

d171eda

同步主版本

b73a47e

Update .gitignore

20f9261

Removed CLAUDE.md from .gitignore

an-tao requested review from Copilot, hwc0919 and marty1885 November 25, 2025 07:14

Copilot started reviewing on behalf of an-tao November 25, 2025 07:14 View session

Copilot finished reviewing on behalf of an-tao November 25, 2025 07:15

Copilot AI reviewed Nov 25, 2025

View reviewed changes

marty1885 requested changes Nov 25, 2025

View reviewed changes

weidqi and others added 7 commits November 27, 2025 08:55

Update orm_lib/tests/db_test.cc

2505553

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update orm_lib/src/DbClientImpl.cc

11ce913

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update orm_lib/src/duckdb_impl/DuckdbConnection.cc

b1d0d89

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update orm_lib/src/duckdb_impl/readme.md

72640f7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update orm_lib/src/duckdb_impl/changelog.md

6dabe7b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update CMakeLists.txt

6599b0e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

The reviewer's comments and suggestions have been incorporated.

9076552

weidqi and others added 3 commits November 27, 2025 09:47

Adjusted some settings to resolve a local build issue for: COMMAND $<…

c3174f9

…TARGET_FILE:_drogon_ctl>

configure modify

ec22479

Remove trantor

fe1f72c

Removed directories 'trantor/'

weidqi requested a review from marty1885 November 28, 2025 08:33

Uh oh!

Conversation

weidqi commented Aug 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

hwc0919 commented Nov 25, 2025

Uh oh!

marty1885 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marty1885 Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

weidqi Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marty1885 Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

marty1885 Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

marty1885 Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

marty1885 Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

weidqi Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

weidqi commented Nov 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marty1885 left a comment •

edited

Loading