electric-sql / electric

Sync little subsets of your Postgres data into local apps and services.
https://electric-sql.com
Apache License 2.0
6.2k stars 146 forks source link

Bug: Wrong type cast leads to crash when starting #1712

Closed dustin-H closed 2 weeks ago

dustin-H commented 2 weeks ago

At this line https://github.com/electric-sql/electric/blob/7765d50cbe57c9990245802366caee8bcd35d6ff/packages/sync-service/lib/electric/connection_manager.ex#L409 electric is reading the pg_setting server_version and casting it to a number.

However this leads to crashes as many postgres as a service providers are setting this value to e.g. 16.4 (Debian 16.4-1.pgdg1123+1).

Also it's quite wrong to expect a number as it is defined as string by postgres:

https://www.postgresql.org/docs/current/runtime-config-preset.html#GUC-SERVER-VERSION

KyleAMathews commented 2 weeks ago

ah good catch! Thanks for the detailed bug report.

kevin-dp commented 2 weeks ago

Hi @dustin-H,

Thanks for the report! I was expecting the version number string to always be in the format x.y but as you explain this may not always be the case. As per your link to the documentation, the server_version_num setting seems more appropriate as it is guaranteed to be an integer (although stored as text). Thus, it seems safe to read server_version_num and cast it to an integer.