Repository navigation
Fix MySQL keyword fields and insert defaults - #283
Conversation
qicosmos
left a comment
There was a problem hiding this comment.
审查意见:
-
正确性(utility.hpp:513):
REGISTER_SKIP_INSERT_FIELD会导致 insert 永远跳过该字段,即使调用方显式指定插入成员也无法覆盖。若 API 支持“指定字段插入”,建议明确设计或仅在未显式指定成员时跳过。 -
正确性(utility.hpp:348):
get_fields<T>(DBType db_type)内部使用static fields + call_once,但结果依赖db_type。若同一类型先按 MySQL 调用、再按 SQLite/PostgreSQL 调用,会复用错误的字段引用格式。该问题已有基础风险,本 PR 引入更多 MySQL quote 后更明显,建议按db_type分缓存。 -
安全性/健壮性(utility.hpp:84):
quote_mysql_identifier对已被反引号包裹的字符串直接返回,不再转义内部反引号。建议始终去除外层后重新转义,或禁止传入已 quote 的标识符,避免异常输入生成非法 SQL。 -
性能/风格(utility.hpp:77):
std::set<std::string_view>用于字段查找略重,字段名集合无序,建议改为std::unordered_set<std::string_view>。 -
正确性(mysql.hpp:1129):跳过空
param_binds是合理修复,但需确认生成的空字段 insert SQL 在各数据库均合法,例如 MySQLinsert into t() values()、SQLite/PostgreSQL 是否一致;建议补充仅剩自增/默认字段的测试。 -
测试不足:新增 MySQL 关键字字段只测 create table,建议同时覆盖 insert/select/update,验证反引号在完整 SQL 路径中一致生效。
Resolve the MySQL bind-storage conflict using the current master implementation. Canonicalize quoted identifiers, quote selected update members, and support inserts containing only database defaults. Expand SQL and SQLite regression coverage and document skip-insert semantics.
qicosmos
left a comment
There was a problem hiding this comment.
审查意见:
-
PostgreSQL 参数数量错误(
postgresql.hpp:671-681, 693-704)
parameter_count仍使用members_count_v<T>,但insert会跳过 auto key 和 skip_insert 字段(573-580)。非default values场景下占位符数量会少于成员数,PQprepare可能失败。应按实际参与插入的字段数计算。 -
跳过字段需确认 SQL 生成同步(
mysql.hpp:1463-1467、mysql_async.hpp:1860-1864、sqlite.hpp:601-606、postgresql.hpp:573-580)
当前只跳过了参数绑定/values 收集。必须确认 insert SQL 的字段列表和占位符也同步跳过is_skip_insert_field,否则会出现字段数与参数数不匹配。diff 中未看到对应修改,建议补充测试覆盖。 -
sql.ends_with依赖 C++20(postgresql.hpp:675, 697)
如果项目仍支持 C++17,会编译失败。建议使用兼容实现,或明确提升标准要求。 -
PostgreSQL returning 字段未引用(
postgresql.hpp:679, 701)
returning+get_auto_key<T>()对关键字或大小写敏感字段不安全。建议使用 PostgreSQL 标识符引用函数。 -
全局注册表线程安全不足(
utility.hpp:74-93)
unordered_map/unordered_set在注册和查询时无锁。若存在动态库加载或运行期注册,可能数据竞争。至少文档说明仅静态初始化期使用,或加锁。 -
MySQL 标识符引用改动合理(
mysql.hpp:1378-1425、mysql_async.hpp:1015-1063、utility.hpp:372)
能修复关键字字段问题,quote_mysql_identifier也处理了反引号转义,整体方向正确。建议补充包含关键字字段、已带反引号字段、复合唯一键的测试。
Let PostgreSQL infer parameter types for prepared statements with omitted insert fields. Quote RETURNING identifiers, document registration timing and C++20 build requirements, and extend SQL generation tests.
qicosmos
left a comment
There was a problem hiding this comment.
审查意见:
-
潜在严重正确性问题:
is_skip_insert_field只在绑定参数处跳过(mysql.hpp:1466、postgresql.hpp:577、sqlite.hpp:604、mysql_async.hpp:1863)。需确认生成 INSERT SQL 的字段列表也同步跳过,否则会出现占位符数量与绑定参数不一致。建议检查/修改get_fields、insert SQL 生成逻辑。 -
线程安全问题:
utility.hpp:77-90的get_skip_insert_field_map()使用全局unordered_map,注册/查询无锁。README 说明需并发前注册,但代码层面仍可能被误用导致数据竞争。建议至少文档强调“非线程安全”,或加锁。 -
生命周期风险:
utility.hpp:77使用std::string_view作为 map/set key。当前宏传入结构名和#FIELD通常是静态生命周期,但若用户直接调用add_skip_insert_field传入临时字符串,会悬垂。建议改为std::string存储。 -
PostgreSQL 返回字段引用:postgresql.hpp:676、696 对
returning字段加了 quote,正确。但如果get_auto_key<T>()为空仍可能生成非法 SQL,建议调用前校验。 -
MySQL 标识符转义:mysql.hpp:1381 等对字段名加反引号,能修复关键字字段问题,方向正确。建议确认索引/唯一键生成处不会对已组合表达式字段误加引号。
-
代码风格:
quote_mysql_identifier/quote_postgresql_identifier放在utility.hpp可接受,但建议增加单元测试覆盖关键字、含反引号/双引号、已引用字段、skip insert 后无参数插入等场景。
qicosmos
left a comment
There was a problem hiding this comment.
审查意见:
-
线程安全问题(utility.hpp:76-91)
get_skip_insert_field_map()使用全局unordered_map/set,注册函数无锁。README 说明需并发前注册,但仍建议与现有 auto_key 一致,或明确静态初始化期限定;否则运行期注册会数据竞争。 -
string_view生命周期风险(utility.hpp:76-88)
map/set 存储std::string_view。当前宏传入结构名和字段字面量通常安全,但若用户直接调用add_skip_insert_field传临时字符串会悬空。建议存std::string。 -
MySQL 参数绑定修复合理(mysql.hpp:1506)
param_binds.empty()时避免访问¶m_binds[0],正确。 -
需确认 SQL 字段列表同步跳过(mysql.hpp:1463、postgresql.hpp:574、sqlite.hpp:601、mysql_async.hpp:1860)
bind 阶段跳过了is_skip_insert_field,但必须确保生成 insert SQL 的字段列表和占位符也同步跳过,否则会出现列数/参数数不匹配。diff 中未完整展示get_fields/insert SQL 生成,建议补充测试覆盖。 -
PostgreSQL prepare 修改合理但需注意(postgresql.hpp:553)
PQprepare参数个数改为 0 依赖类型推断,通常可行;但复杂表达式或类型歧义场景可能失败。建议增加插入默认字段、省略全部字段、返回自增 id 的测试。 -
标识符引用影响行为(mysql.hpp:1378 等)
MySQL 字段统一反引号引用可修复关键字问题,但若用户已传入复合表达式或带表名前缀的字段名,会被整体引用导致 SQL 变化。需确认框架只允许纯字段名。 -
PostgreSQL returning 修复正确(postgresql.hpp:673、693)
自增字段加双引号避免关键字/大小写问题,合理。
qicosmos
left a comment
There was a problem hiding this comment.
审查意见:
-
正确性风险:
mysql.hpp:1463、mysql_async.hpp:1860、postgresql.hpp:574、sqlite.hpp:601仅跳过了参数绑定。需确认生成insert字段列表和占位符的逻辑也同步跳过is_skip_insert_field,否则会出现占位符数量与绑定参数不一致。 -
线程安全:
utility.hpp:76-90使用全局unordered_map/set,注册和查询无锁。README 提到并发前注册,但代码层面无保护,若运行期注册会数据竞争。建议加锁或明确设计为静态初始化期只写。 -
生命周期风险:
utility.hpp:76使用std::string_view作为 map key/value,依赖传入字符串生命周期永久有效。宏参数一般安全,但add_skip_insert_field是公开函数,外部传临时字符串会悬垂。建议存std::string。 -
PostgreSQL 行为变化:
postgresql.hpp:553将PQprepare参数数改为 0 合理,但应确认所有 SQL 都能由 PostgreSQL 推断参数类型,尤其是NULL、重载函数或类型不明确表达式。 -
标识符引用一致性:
mysql.hpp:1378-1424、mysql_async.hpp:1015-1064对建表字段/主键加了反引号,方向正确。请确认其他 SQL 生成路径(insert/update/select/where/order by)也统一引用,否则 MySQL 关键字字段仍可能失败。 -
健壮性:
REGISTER_SKIP_INSERT_FIELD未校验字段是否存在,拼写错误会静默无效。可考虑在反射成员中校验或提供调试断言。
整体改动方向正确,重点需确认“跳过字段”同时影响 SQL 生成与参数绑定。
qicosmos
left a comment
There was a problem hiding this comment.
代码审查意见:
-
潜在正确性问题:
mysql.hpp:1466、mysql_async.hpp:1863、postgresql.hpp:577、sqlite.hpp:604绑定参数时跳过is_insert_skipped字段。请确认生成 insert SQL 的字段列表和占位符数量也同步跳过这些字段;否则会出现参数数量与 SQL 不匹配。diff 仅部分显示get_fields,需重点验证。 -
线程安全:
utility.hpp:78-91的get_skip_insert_field_map()使用全局unordered_map,add_skip_insert_field无锁。若用户运行期注册并发数据库操作,存在数据竞争。README 有说明,但建议代码层面加注释或限制 API 使用场景。 -
PostgreSQL prepare 修改需谨慎:
postgresql.hpp:556将PQprepare参数个数改为 0,依赖 PostgreSQL 推断参数类型。大多数场景可行,但某些表达式或重载函数中$1类型可能无法推断。建议增加测试覆盖 insert/update/select 参数绑定场景。 -
空参数执行行为:
postgresql.hpp:604删除param_values.empty()返回逻辑是合理的,可支持DEFAULT VALUES,但需确保make_param_value_buffers对空 vector 安全,且PQexecPrepared(..., 0, ...)传入空data()不出问题。 -
标识符转义:
utility.hpp:95-130MySQL/PostgreSQL 标识符转义整体合理,解决关键字字段问题。建议补充单测:关键字字段、已带反引号字段、字段名包含反引号/双引号。 -
风格建议:
REGISTER_SKIP_INSERT_FIELD与REGISTER_AUTO_KEY行为类似,建议在头文件注释中明确“insert 时也会跳过 auto key”。目前is_insert_skipped语义清晰。
总体方向正确,但必须补齐 SQL 生成与参数绑定一致性的测试。
Summary
Tests
Fixes #281
Fixes #282
October 2026 update
masterand resolved the MySQL binding conflict using the newer parameter-storage path.DEFAULT VALUESon SQLite/PostgreSQL and() VALUES ()on MySQL; documented that registered skip fields are always omitted byinsert.Validation: built
test_ormppwith MySQL, async MySQL, and PostgreSQL enabled; 5 focused cases and 48 assertions passed.git diff --checkand changed-line clang-format passed. MySQL server CRUD assertions could not run locally because the configuredrootaccount was denied. Review follow-up: PostgreSQL prepares now infer parameter types from SQL,RETURNINGquotes identifiers, and composite MySQL unique keys have SQL tests.