Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling PgConnectOptions.options with >1 elements results in invalid arguments. #1730

Closed
kevincox opened this issue Mar 1, 2022 · 4 comments · Fixed by #1731
Closed

Calling PgConnectOptions.options with >1 elements results in invalid arguments. #1730

kevincox opened this issue Mar 1, 2022 · 4 comments · Fixed by #1731
Labels
db:postgres Related to PostgreSQL E-easy good first issue Good for newcomers

Comments

@kevincox
Copy link

kevincox commented Mar 1, 2022

	let mut connect_options = url.parse::<sqlx::postgres::PgConnectOptions>().unwrap()
		.options([
			("default_transaction_isolation", "serializable"),
			("idle_in_transaction_session_timeout", "5min"),
		]);
thread 'main' panicked at 'Couldn't connect to database.: Database(PgDatabaseError { severity: Fatal, code: "22023", message: "invalid value for parameter \"default_transaction_isolation\": \"serializable-c\"", detail: None, hint: Some("Available values: serializable, repeatable read, read committed, read uncommitted."), position: None, where: None, schema: None, table: None, column: None, data_type: None, constraint: None, file: Some("guc.c"), line: Some(6873), routine: Some("parse_and_validate_value") })', src/db.rs:23:14

This bug appears to have existed forever: #1539 (comment)

@kevincox
Copy link
Author

kevincox commented Mar 1, 2022

The workaround is to make multiple calls:

	let mut connect_options = url.parse::<sqlx::postgres::PgConnectOptions>().unwrap()
		.options([("default_transaction_isolation", "serializable")])
		.options([("idle_in_transaction_session_timeout", "5min")]);

@abonander
Copy link
Collaborator

abonander commented Mar 1, 2022

This bug appears to have existed forever:

The options() method has only existed since November. However, the fix should be pretty easy, and would simplify the method quite a bit.

I'd replace the following lines: https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/options/mod.rs#L354-L363

With this:

use std::fmt::Write;

for (k, v) in options {
    // Do this in here so `options_str` is only set if we have an option to insert
    let mut options_str = self.options.get_or_insert_with(String::new);

    if !options_str.is_empty() {
        options_str.push(' ');
    }

    write!(options_str, "-c {}={}", k, v).expect("failed to write an option to the string");
}

@abonander abonander added db:postgres Related to PostgreSQL E-easy good first issue Good for newcomers labels Mar 1, 2022
@kevincox
Copy link
Author

kevincox commented Mar 1, 2022

I meant since the introduction of the feature, so relatively low priority as it wasn't something newly broken.

I agree, that would clean up the code a lot.

@liushuyu
Copy link
Contributor

liushuyu commented Mar 2, 2022

A fix is proposed in #1731.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db:postgres Related to PostgreSQL E-easy good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants