Skip to content

Support MySQL unclosed polygons, switch to go-geom#4135

Draft
ilidemi wants to merge 2 commits intomainfrom
mysql-unclosed-polygons
Draft

Support MySQL unclosed polygons, switch to go-geom#4135
ilidemi wants to merge 2 commits intomainfrom
mysql-unclosed-polygons

Conversation

@ilidemi
Copy link
Copy Markdown
Contributor

@ilidemi ilidemi commented Apr 1, 2026

MySQL <5.7.9 had a bug where unclosed polygons were accepted even though they're invalid according to the spec https://bugs.mysql.com/bug.php?id=77505

Switching from go-geos to go-geom solves this. The downside is that MultiPoint output is different, MULTIPOINT (5 6, 7 8) vs old MULTIPOINT ((5 6), (7 8)), but ClickHouse doesn't support multipoint either way (although regex parsing would be easier for the old one).

ClickHouse does support unclosed polygons.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026 •
edited
Loading

❌ 6 Tests Failed:

Tests completed Failed Passed Skipped
2196 6 2190 238
View the top 3 failed test(s) by shortest run time 
github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuiteMySQL_CH
Stack Traces | 0s run time 
=== RUN   TestPeerFlowE2ETestSuiteMySQL_CH
=== PAUSE TestPeerFlowE2ETestSuiteMySQL_CH
=== CONT  TestPeerFlowE2ETestSuiteMySQL_CH
--- FAIL: TestPeerFlowE2ETestSuiteMySQL_CH (0.00s)
2026/04/01 23:52:01 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/04/01 23:52:01 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}

github.com/PeerDB-io/peerdb/flow/e2e::TestGenericBQ
Stack Traces | 0.01s run time 
=== RUN   TestGenericBQ
=== PAUSE TestGenericBQ
=== CONT  TestGenericBQ
--- FAIL: TestGenericBQ (0.01s)
2026/04/02 00:01:12 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/04/02 00:01:12 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/04/02 00:01:12 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mychcl_qihryv6l.test_nullengine

github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
Stack Traces | 0.01s run time 
=== RUN   TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
=== PAUSE TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
=== CONT  TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
--- FAIL: TestPeerFlowE2ETestSuiteMySQL_CH_Cluster (0.01s)

github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuiteMySQL_CH/Test_MySQL_Geometric_Types
Stack Traces | 19.1s run time 
=== RUN   TestPeerFlowE2ETestSuiteMySQL_CH/Test_MySQL_Geometric_Types
=== PAUSE TestPeerFlowE2ETestSuiteMySQL_CH/Test_MySQL_Geometric_Types
=== CONT  TestPeerFlowE2ETestSuiteMySQL_CH/Test_MySQL_Geometric_Types
2026/04/01 23:51:14 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/04/01 23:51:14 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/04/01 23:51:14 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mych_39zmb0gc.test_nullengine
    clickhouse_mysql_test.go:416: WaitFor waiting for initial snapshot count 2026-04-01 23:51:18.109652266 +0000 UTC m=+86.918578713
    clickhouse_mysql_test.go:428: WaitFor waiting for CDC count 2026-04-01 23:51:18.113729233 +0000 UTC m=+86.922655690
2026/04/01 23:51:18 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mych_39zmb0gc.test_nullengine
    clickhouse_mysql_test.go:462: 
        	Error Trace:	.../flow/e2e/clickhouse_mysql_test.go:462
        	Error:      	Not equal: 
        	            	expected: "POINT EMPTY"
        	            	actual  : ""
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-POINT EMPTY
        	            	+
        	Test:       	TestPeerFlowE2ETestSuiteMySQL_CH/Test_MySQL_Geometric_Types
        	Messages:   	geometry_col value mismatch at row 9
--- FAIL: TestPeerFlowE2ETestSuiteMySQL_CH/Test_MySQL_Geometric_Types (19.14s)

github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_Geometric_Types
Stack Traces | 20.3s run time 
=== RUN   TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_Geometric_Types
=== PAUSE TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_Geometric_Types
=== CONT  TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_Geometric_Types
2026/04/02 00:00:18 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/04/02 00:00:18 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
    clickhouse_mysql_test.go:416: WaitFor waiting for initial snapshot count 2026-04-02 00:00:22.717550295 +0000 UTC m=+631.526476742
    clickhouse_mysql_test.go:428: WaitFor waiting for CDC count 2026-04-02 00:00:22.726144324 +0000 UTC m=+631.535070781
    clickhouse_mysql_test.go:462: 
        	Error Trace:	.../flow/e2e/clickhouse_mysql_test.go:462
        	Error:      	Not equal: 
        	            	expected: "POINT EMPTY"
        	            	actual  : ""
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-POINT EMPTY
        	            	+
        	Test:       	TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_Geometric_Types
        	Messages:   	geometry_col value mismatch at row 9
--- FAIL: TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_Geometric_Types (20.27s)

github.com/PeerDB-io/peerdb/flow/e2e::TestGenericBQ/Test_Inheritance_Table_With_Dynamic_Setting
Stack Traces | 33.6s run time 
=== RUN   TestGenericBQ/Test_Inheritance_Table_With_Dynamic_Setting
=== PAUSE TestGenericBQ/Test_Inheritance_Table_With_Dynamic_Setting
=== CONT  TestGenericBQ/Test_Inheritance_Table_With_Dynamic_Setting
    generic_test.go:984: UNEXPECTED STATUS TIMEOUT STATUS_SNAPSHOT
    bigquery.go:86: begin tearing down postgres schema bq_oanadl3n_20260402000010
2026/04/02 00:00:44 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mychcl_xcvruzer.test_blobs
2026/04/02 00:00:44 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mychcl_ievdfvzg.test_bit
--- FAIL: TestGenericBQ/Test_Inheritance_Table_With_Dynamic_Setting (33.59s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

❌ Test Failure

Analysis: Test_MySQL_Geometric_Types consistently fails across all matrix variants with MySQL error 3037 "Invalid GIS data provided to function st_geomfromtext", caused by the PR's unclosed polygon test data being rejected by MySQL.
Confidence: 0.97

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

serprex
serprex approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@serprex serprex left a comment

Choose a reason for hiding this comment

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

pretty sure we went with go-geos because go-geom didn't have enough for postgis compat. mysql compat should be much simpler tho

@ilidemi
Copy link
Copy Markdown
Contributor Author

ilidemi commented Apr 9, 2026

Current status is we're declaring invalid geometry as unsupported for now (as MySQL 8 or even fresh 5.7 would refuse the insert) but open to reconsidering if more cases come up

@pfcoperez pfcoperez mentioned this pull request Apr 13, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serprex serprex serprex approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ilidemi @serprex