Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to enable Spirit to handle MySQL GEOMETRY columns and SPATIAL indexes (per #686), primarily by updating parser behavior (via a go.mod replacement) and adding integration/unit tests that exercise DDL parsing and replication/copy pipelines with spatial data.
Changes:
- Add statement parsing test coverage for rewriting
CREATE SPATIAL INDEXintoALTER TABLE ... ADD SPATIAL INDEX. - Add buffered replication and buffered copier integration tests to validate GEOMETRY values round-trip correctly.
- Add a migration integration test that runs an
ALTER TABLEadding a generated GEOMETRY column and a SPATIAL index, plus update Go module dependencies (including areplaceto a forked TiDB parser).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/statement/statement_test.go | Adds coverage for CREATE SPATIAL INDEX rewrite behavior. |
| pkg/repl/subscription_buffered_test.go | Adds integration test ensuring buffered-map replication preserves GEOMETRY row images and applies correctly. |
| pkg/migration/migration_test.go | Adds migration test for combined generated GEOMETRY column + SPATIAL index DDL. |
| pkg/copier/buffered_test.go | Adds buffered copier test verifying GEOMETRY values copy correctly (incl. SPATIAL index presence). |
| go.mod | Adds indirect dependency and replace to a forked TiDB parser module. |
| go.sum | Updates sums for new/changed module dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| replace github.com/pingcap/tidb/pkg/parser => github.com/morgo/tidb/pkg/parser v0.0.0-20260409131709-d591801319ba |
There was a problem hiding this comment.
The committed replace pins github.com/pingcap/tidb/pkg/parser to a personal fork. This makes builds depend on a non-upstream module path/commit and can break reproducibility and supply-chain expectations. Consider waiting for the upstream TiDB parser change to be released and then updating the require version (removing the replace), or alternatively vendor/fork under an org-controlled module path with a clear upgrade plan and tracking issue.
| replace github.com/pingcap/tidb/pkg/parser => github.com/morgo/tidb/pkg/parser v0.0.0-20260409131709-d591801319ba |
A Pull Request should be associated with an Issue.
Fixes #686
.. but it uses a go.mod replace to do so. We need to wait for the TiDB PR to merge, and hope it adds SPATIAL index support too. We can followup with a PR if it does not.