Hello @hongyunyan, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini or gemini-code-assist here, providing a summary of this pull request.

This pull request introduces a new integration test focused on verifying TiCDC's behavior when replicating data that violates foreign key constraints. The test simulates a scenario where tables with a foreign key relationship are created, foreign key checks are disabled on the upstream database, and then data that violates the foreign key constraint is inserted. The changefeed replicates this data downstream, and the test uses the sync-diff tool to confirm that the final data state downstream is consistent with the upstream, ensuring CDC can handle such scenarios correctly.

Highlights

• New Integration Test: Adds a dedicated integration test suite under tests/integration_tests/foreign_key_check to specifically test foreign key replication scenarios.
• Foreign Key Violation Scenario: The test creates tables with a foreign key constraint (departments and employees), disables foreign key checks on the upstream TiDB instance, and inserts data into the employees table with a dept_id that does not exist in the departments table, thus violating the FK.
• Data Consistency Verification: Utilizes the sync-diff tool with a specific configuration (diff_config.toml) to compare the data between the upstream and downstream databases after replication, ensuring consistency despite the FK violation upstream.
• CI Integration: Includes the newly added foreign_key_check test in the list of light integration tests (run_light_it_in_ci.sh) to be executed automatically in the CI pipeline.

Changelog

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Test added, checks FK,
Data flows the CDC way,
Consistency wins the day.

Code Review

This pull request introduces a new integration test for foreign-key-check functionality. The test aims to verify that TiCDC can successfully replicate data that violates foreign key constraints when upstream has FK checks disabled, to a downstream MySQL sink that (presumably) has FK detection enabled. The setup involves disabling FK checks on the upstream, inserting data that violates FKs, and then using sync_diff_inspector to ensure the data is replicated correctly.

The overall structure of the test, including the shell script, SQL setup, and diff configuration, appears sound for its purpose. The addition of the test to the CI run is also appropriate.

I have one suggestion for improving the maintainability of the test script.

Summary of Findings

• Unused variables in test script: The run.sh script defines CDC_COUNT and DB_COUNT which are not subsequently used. This was commented on with medium severity.
• Non-English comment in SQL script: The pre.sql file contains a comment in Chinese (-- 外键约束). While functional, using English for comments is generally preferred in projects with international contributors for better universal understanding. This was not commented on directly due to its low severity.
• Missing newline at end of SQL file: The pre.sql file is missing a newline character at the end. This is a minor stylistic issue. This was not commented on directly due to its low severity.
• Hardcoded temporary path in config: The diff_config.toml uses a hardcoded path in /tmp for output-dir. This is common and generally acceptable for test configurations. This was not commented on directly due to its low severity in a test context.
• Naming in diff_config.toml: In diff_config.toml, the source TiDB (port 4000) is named mysql1 and the target MySQL (port 3306) is named tidb0. This naming could be slightly counter-intuitive. This is a minor clarity point and was not commented on directly due to its low severity.

Merge Readiness

The pull request is generally in good shape and adds a valuable integration test. There is one medium-severity issue regarding unused variables in the run.sh script that should be addressed to improve code maintainability. Once that is resolved, the PR should be ready for merging. As an AI, I am not authorized to approve pull requests; please ensure further review and approval from authorized maintainers.