Skip to content

Commit a74ab93

Browse files
authored
fast_import: remove hardcoding of pg_version (#9878)
Before, we hardcoded the pg_version to 140000, while the code expected version numbers like 14. Now we use an enum, and code from `extension_server.rs` to auto-detect the correct version. The enum helps when we add support for a version: enums ensure that compilation fails if one forgets to put the version to one of the `match` locations. cc #9218
1 parent 7404887 commit a74ab93

File tree

3 files changed

+47
-20
lines changed

3 files changed

+47
-20
lines changed

compute_tools/src/bin/compute_ctl.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ use compute_tools::compute::{
5858
forward_termination_signal, ComputeNode, ComputeState, ParsedSpec, PG_PID,
5959
};
6060
use compute_tools::configurator::launch_configurator;
61-
use compute_tools::extension_server::get_pg_version;
61+
use compute_tools::extension_server::get_pg_version_string;
6262
use compute_tools::http::api::launch_http_server;
6363
use compute_tools::logger::*;
6464
use compute_tools::monitor::launch_monitor;
@@ -326,7 +326,7 @@ fn wait_spec(
326326
connstr: Url::parse(connstr).context("cannot parse connstr as a URL")?,
327327
pgdata: pgdata.to_string(),
328328
pgbin: pgbin.to_string(),
329-
pgversion: get_pg_version(pgbin),
329+
pgversion: get_pg_version_string(pgbin),
330330
live_config_allowed,
331331
state: Mutex::new(new_state),
332332
state_changed: Condvar::new(),

compute_tools/src/bin/fast_import.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use anyhow::Context;
2929
use aws_config::BehaviorVersion;
3030
use camino::{Utf8Path, Utf8PathBuf};
3131
use clap::Parser;
32+
use compute_tools::extension_server::{get_pg_version, PostgresMajorVersion};
3233
use nix::unistd::Pid;
3334
use tracing::{info, info_span, warn, Instrument};
3435
use utils::fs_ext::is_directory_empty;
@@ -131,11 +132,17 @@ pub(crate) async fn main() -> anyhow::Result<()> {
131132
//
132133
// Initialize pgdata
133134
//
135+
let pg_version = match get_pg_version(pg_bin_dir.as_str()) {
136+
PostgresMajorVersion::V14 => 14,
137+
PostgresMajorVersion::V15 => 15,
138+
PostgresMajorVersion::V16 => 16,
139+
PostgresMajorVersion::V17 => 17,
140+
};
134141
let superuser = "cloud_admin"; // XXX: this shouldn't be hard-coded
135142
postgres_initdb::do_run_initdb(postgres_initdb::RunInitdbArgs {
136143
superuser,
137144
locale: "en_US.UTF-8", // XXX: this shouldn't be hard-coded,
138-
pg_version: 140000, // XXX: this shouldn't be hard-coded but derived from which compute image we're running in
145+
pg_version,
139146
initdb_bin: pg_bin_dir.join("initdb").as_ref(),
140147
library_search_path: &pg_lib_dir, // TODO: is this right? Prob works in compute image, not sure about neon_local.
141148
pgdata: &pgdata_dir,

compute_tools/src/extension_server.rs

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,33 @@ fn get_pg_config(argument: &str, pgbin: &str) -> String {
103103
.to_string()
104104
}
105105

106-
pub fn get_pg_version(pgbin: &str) -> String {
106+
pub fn get_pg_version(pgbin: &str) -> PostgresMajorVersion {
107107
// pg_config --version returns a (platform specific) human readable string
108108
// such as "PostgreSQL 15.4". We parse this to v14/v15/v16 etc.
109109
let human_version = get_pg_config("--version", pgbin);
110-
parse_pg_version(&human_version).to_string()
110+
parse_pg_version(&human_version)
111111
}
112112

113-
fn parse_pg_version(human_version: &str) -> &str {
113+
pub fn get_pg_version_string(pgbin: &str) -> String {
114+
match get_pg_version(pgbin) {
115+
PostgresMajorVersion::V14 => "v14",
116+
PostgresMajorVersion::V15 => "v15",
117+
PostgresMajorVersion::V16 => "v16",
118+
PostgresMajorVersion::V17 => "v17",
119+
}
120+
.to_owned()
121+
}
122+
123+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
124+
pub enum PostgresMajorVersion {
125+
V14,
126+
V15,
127+
V16,
128+
V17,
129+
}
130+
131+
fn parse_pg_version(human_version: &str) -> PostgresMajorVersion {
132+
use PostgresMajorVersion::*;
114133
// Normal releases have version strings like "PostgreSQL 15.4". But there
115134
// are also pre-release versions like "PostgreSQL 17devel" or "PostgreSQL
116135
// 16beta2" or "PostgreSQL 17rc1". And with the --with-extra-version
@@ -121,10 +140,10 @@ fn parse_pg_version(human_version: &str) -> &str {
121140
.captures(human_version)
122141
{
123142
Some(captures) if captures.len() == 2 => match &captures["major"] {
124-
"14" => return "v14",
125-
"15" => return "v15",
126-
"16" => return "v16",
127-
"17" => return "v17",
143+
"14" => return V14,
144+
"15" => return V15,
145+
"16" => return V16,
146+
"17" => return V17,
128147
_ => {}
129148
},
130149
_ => {}
@@ -263,24 +282,25 @@ mod tests {
263282

264283
#[test]
265284
fn test_parse_pg_version() {
266-
assert_eq!(parse_pg_version("PostgreSQL 15.4"), "v15");
267-
assert_eq!(parse_pg_version("PostgreSQL 15.14"), "v15");
285+
use super::PostgresMajorVersion::*;
286+
assert_eq!(parse_pg_version("PostgreSQL 15.4"), V15);
287+
assert_eq!(parse_pg_version("PostgreSQL 15.14"), V15);
268288
assert_eq!(
269289
parse_pg_version("PostgreSQL 15.4 (Ubuntu 15.4-0ubuntu0.23.04.1)"),
270-
"v15"
290+
V15
271291
);
272292

273-
assert_eq!(parse_pg_version("PostgreSQL 14.15"), "v14");
274-
assert_eq!(parse_pg_version("PostgreSQL 14.0"), "v14");
293+
assert_eq!(parse_pg_version("PostgreSQL 14.15"), V14);
294+
assert_eq!(parse_pg_version("PostgreSQL 14.0"), V14);
275295
assert_eq!(
276296
parse_pg_version("PostgreSQL 14.9 (Debian 14.9-1.pgdg120+1"),
277-
"v14"
297+
V14
278298
);
279299

280-
assert_eq!(parse_pg_version("PostgreSQL 16devel"), "v16");
281-
assert_eq!(parse_pg_version("PostgreSQL 16beta1"), "v16");
282-
assert_eq!(parse_pg_version("PostgreSQL 16rc2"), "v16");
283-
assert_eq!(parse_pg_version("PostgreSQL 16extra"), "v16");
300+
assert_eq!(parse_pg_version("PostgreSQL 16devel"), V16);
301+
assert_eq!(parse_pg_version("PostgreSQL 16beta1"), V16);
302+
assert_eq!(parse_pg_version("PostgreSQL 16rc2"), V16);
303+
assert_eq!(parse_pg_version("PostgreSQL 16extra"), V16);
284304
}
285305

286306
#[test]

0 commit comments

Comments
 (0)