Skip to content

Emit DDL event for ALTER TABLE ADD CONSTRAINT with non-unique constraint - #5603

Open
fudianchn wants to merge 1 commit into
alibaba:masterfrom
fudianchn:fix/ddl-parser-add-constraint-event
Open

Emit DDL event for ALTER TABLE ADD CONSTRAINT with non-unique constraint#5603
fudianchn wants to merge 1 commit into
alibaba:masterfrom
fudianchn:fix/ddl-parser-add-constraint-event

Conversation

@fudianchn

@fudianchn fudianchn commented Jul 26, 2026

Copy link
Copy Markdown

AI 披露:本改动由 AI 编码代理辅助完成,我已逐行审改。

问题(What)

ALTER TABLE ... ADD CONSTRAINT 添加非 UNIQUE 约束(外键 / CHECK 等)时,canal 不产生任何 DDL 事件,下游感知不到这次表结构变更。

根因(Root cause)

DruidDdlParserSQLAlterTableAddConstraint 分支只在约束为 SQLUnique 时才把 DdlResult 加入结果;其余情况(外键/CHECK 等)DdlResult 已构造、processName 已调用,却被丢弃——既没 add,也没设 EventType

修复(Fix)

非 UNIQUE 约束按 EventType.ALTER 处理并加入结果(与既有 catch-all 分支一致),UNIQUE 仍为 CINDEX。仅给该分支补 else 并把 add 移出 if

测试(Testing)

新增两个用例覆盖 SQLAlterTableAddConstraint 的两条路径:

  • testAlterAddConstraintForeignKey... add constraint fk1 foreign key (a) references p(id),断言产生 1 条 DDL 事件、表名 retl_markEventType.ALTER
  • testAlterAddConstraintCheck... add constraint ck1 check (a > 0),断言同上。CHECK 走 catch-all else 分支,防止将来有人把 else 收窄成 instanceof SQLForeignKeyImpl 后 CHECK 再次被静默丢弃。

修复前两个用例均失败(结果列表为空,assertFalse 报错);修复后通过。DruidDdlParserTest 全部 9 个用例通过。

复现(Reproduce)

List<DdlResult> r = DruidDdlParser.parse(
    "alter table retl_mark add constraint fk1 foreign key (a) references p(id)", "retl");
// 修复前:r 为空(无 DDL 事件)
// 修复后:r = [DdlResult [schemaName=retl , tableName=retl_mark , type=ALTER ;]]

@CLAassistant

CLAassistant commented Jul 26, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@wenshao wenshao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +16 to +18
public void testAlterAddConstraintForeignKey() {
String queryString = "alter table retl_mark add constraint fk1 foreign key (a) references p(id)";
List<DdlResult> results = DruidDdlParser.parse(queryString, "retl");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The PR description states it fixes non-UNIQUE constraints (foreign key AND CHECK), but the test only covers FOREIGN KEY. If a future refactor replaces the catch-all else with an explicit instanceof SQLForeignKeyImpl check, ALTER TABLE t ADD CONSTRAINT ck1 CHECK (col > 0) would be silently dropped again — the exact bug this PR fixes — and no test would fail. Consider adding a companion test:

@Test
public void testAlterAddConstraintCheck() {
    String queryString = "alter table retl_mark add constraint ck1 check (a > 0)";
    List<DdlResult> results = DruidDdlParser.parse(queryString, "retl");
    Assert.assertFalse("ADD CONSTRAINT CHECK should emit a DDL event", results.isEmpty());
    DdlResult result = results.get(0);
    Assert.assertEquals("retl", result.getSchemaName());
    Assert.assertEquals("retl_mark", result.getTableName());
    Assert.assertEquals(EventType.ALTER, result.getType());
}
中文说明

PR 描述中说明修复了非 UNIQUE 约束(外键和 CHECK),但测试仅覆盖了 FOREIGN KEY。如果将来有人将 catch-all else 重构为显式的 instanceof SQLForeignKeyImpl 判断,ALTER TABLE t ADD CONSTRAINT ck1 CHECK (col > 0) 将再次被静默丢弃——即本 PR 修复的 bug——且不会有测试失败。建议补充 CHECK 约束的测试用例。

— qwen3.8-max-preview via Qwen Code /review

DruidDdlParser only enqueued a DdlResult for SQLAlterTableAddConstraint
when the constraint was a SQLUnique; for other constraints (FOREIGN KEY,
CHECK, ...) the DdlResult was built and processName'd but never added,
so ALTER TABLE t ADD CONSTRAINT fk1 FOREIGN KEY (...) emitted no DDL
event. Set EventType.ALTER for the non-unique case and always add the
result, mirroring the existing catch-all branch.
@fudianchn
fudianchn force-pushed the fix/ddl-parser-add-constraint-event branch from d02dbaa to 28fef88 Compare August 1, 2026 16:42
@fudianchn

fudianchn commented Aug 1, 2026

Copy link
Copy Markdown
Author

Added a CHECK-constraint companion test testAlterAddConstraintCheck covering the catch-all else branch, so a future refactor replacing the else with an explicit instanceof SQLForeignKeyImpl would now be caught. Verified: Tests run: 9, Failures: 0, Errors: 0 on DruidDdlParserTest with BUILD SUCCESS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants