search

launchbadge / sqlx

🧰 The Rust SQL Toolkit. An async, pure Rust SQL crate featuring compile-time checked queries without a DSL. Supports PostgreSQL, MySQL, and SQLite.
Apache License 2.0
13.14k stars 1.24k forks source link

Issues with redshift #801

Open framp opened 3 years ago

framp commented 3 years ago

I'm aware redshift is not supported but it's almost there!

Things I noticed:

extra_float_digits could very well be just another option in PgConnectOptions; the other two require some more investigation. I'll try to come back and fix them as soon as I get some time. Feel free to close if it's out of scope and cluttering your issue tracking.

brianbruggeman commented 2 years ago

Another data point:

I have an older postgres, and the extra_float_digits is also not supported. Given time and support staffing, I'd have already moved to postgres 14, but at the moment, I'm still stuck on 10. I like that there's a default which felt reasonable, but I need a way to remove it entire, and I'd prefer not to carry a patch to the repo.

The actual error: error returned from database: unsupported startup parameter: extra_float_digits

As an aside, are these options really required for connectivity? It seems more of a very opinionated (and non-controllable) set of parameters.

abonander commented 2 years ago

I have an older postgres, and the extra_float_digits is also not supported. Given time and support staffing, I'd have already moved to postgres 14, but at the moment, I'm still stuck on 10.

Postgres 10 supports extra_float_digits? https://www.postgresql.org/docs/10/runtime-config-client.html#RUNTIME-CONFIG-CLIENT-FORMAT

It may have been added in a point-release greater than the one you're currently running. Would it possibly be easier to upgrade to 10.20 rather than 14? Or do you simply not have the ability to touch the running instance at all?

As an aside, are these options really required for connectivity? It seems more of a very opinionated (and non-controllable) set of parameters.

@mehcode can weigh in here since he wrote that part of the driver, but to my understanding the primary reasoning for setting extra_float_digits is so that floating-point values in the text format can be parsed into the exact same value as they are in Postgres, whereas the default value of 0 may trigger rounding on some platforms.

Granted, floating points shouldn't be used if you care about exact, reproducible values anyway, but to avoid surprising anyone we probably want to make sending the parameter opt-out rather than opt-in.

brianbruggeman commented 2 years ago

I have found a workaround by setting up pgbouncer, which has a setting that can let me ignore the extra_float_digits.

I'm not able to touch Postgres at this time; not enough time (we're short handed) and it could cause a massive fire. We have an always on instance on bare-metal with limited extra bare-metal machines and 10s of terabytes of data. Migration will likely happen within a few months to 14, though, when we swap to a cloud based solution.

parameter opt-out rather than opt-in.

Is this true, though? Perusing the code, it seems like there's a way to add a parameter, but not actually remove or update the parameter. That makes me think that it's not really opt-out.

For what it's worth, I also found this Pull request. And I saw these comments from @mehcode: here, which is now pretty dated (20 Dec 2020). Essentially, I think that implies it should be removed (??).

abonander commented 1 year ago

Honestly, I'd rather just lower the default for extra_float_digits to 2. From Postgres 12 onward, any positive value selects the shortest-precise format so it doesn't really matter what we pass: https://www.postgresql.org/docs/12/runtime-config-client.html#GUC-EXTRA-FLOAT-DIGITS

The last Postgres release to actually care about the exact value will sunset in less than two months, and this behavior only ever affected the text-format output of floats anyway.