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

Add trusted option in the control file #1397

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion cargo-pgrx/src/command/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ pub(crate) fn install_extension(
true,
&package_manifest_path,
&mut output_tracking,
&pg_config,
)?;
}

Expand Down Expand Up @@ -217,6 +218,7 @@ pub(crate) fn install_extension(
false,
&package_manifest_path,
&mut output_tracking,
&pg_config,
)?;
}

Expand Down Expand Up @@ -245,6 +247,7 @@ fn copy_file(
do_filter: bool,
package_manifest_path: impl AsRef<Path>,
output_tracking: &mut Vec<PathBuf>,
pg_config: &PgConfig,
) -> eyre::Result<()> {
let Some(dest_dir) = dest.parent() else {
// what fresh hell could ever cause such an error?
Expand All @@ -266,7 +269,12 @@ fn copy_file(
// we want to filter the contents of the file we're to copy
let input = std::fs::read_to_string(src)
.wrap_err_with(|| format!("failed to read `{}`", src.display()))?;
let input = filter_contents(package_manifest_path, input)?;
let mut input = filter_contents(package_manifest_path, input)?;

if src.display().to_string().ends_with(".control") {
input = filter_out_fields_in_control(pg_config, input)?;
}

std::fs::write(dest, input).wrap_err_with(|| {
format!("failed writing `{}` to `{}`", src.display(), dest.display())
})?;
Expand Down Expand Up @@ -401,6 +409,7 @@ fn copy_sql_files(
true,
&package_manifest_path,
output_tracking,
&pg_config,
)?;
}
}
Expand Down Expand Up @@ -532,3 +541,17 @@ fn filter_contents(manifest_path: impl AsRef<Path>, mut input: String) -> eyre::

Ok(input)
}

// remove fields in control for versions not supported
// `trusted`` in only supported in version 13 and above
fn filter_out_fields_in_control(pg_config: &PgConfig, mut input: String) -> eyre::Result<String> {
if pg_config.major_version().unwrap() < 13 {
input = input
.lines()
.filter(|line| !line.starts_with("trusted"))
.collect::<Vec<_>>()
.join("\n");
}

Ok(input)
}
1 change: 1 addition & 0 deletions cargo-pgrx/src/templates/control
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ default_version = '@CARGO_VERSION@'
module_pathname = '$libdir/{name}'
relocatable = false
superuser = true
trusted = false
1 change: 1 addition & 0 deletions pgrx-examples/custom_types/custom_types.control
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ default_version = '@CARGO_VERSION@'
module_pathname = '$libdir/custom_types'
relocatable = false
superuser = false
trusted = false
Comment on lines 5 to +6
Copy link
Member

Choose a reason for hiding this comment

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

In a case where this was, e.g.

superuser = false
trusted = true

this would do nothing. While Postgres notes this is simply "irrelevant", I think pgrx should error, or at least warn on that case, rather than permitting it to pass by unremarked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I added another type of error for this case.

17 changes: 15 additions & 2 deletions pgrx-sql-entity-graph/src/control_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub struct ControlFile {
pub relocatable: bool,
pub superuser: bool,
pub schema: Option<String>,
pub trusted: bool,
}

impl ControlFile {
Expand Down Expand Up @@ -67,7 +68,7 @@ impl ControlFile {

temp.insert(k, v);
}
Ok(ControlFile {
let control_file = ControlFile {
comment: temp
.get("comment")
.ok_or(ControlFileError::MissingField { field: "comment" })?
Expand All @@ -86,7 +87,15 @@ impl ControlFile {
.ok_or(ControlFileError::MissingField { field: "superuser" })?
== &"true",
schema: temp.get("schema").map(|v| v.to_string()),
})
trusted: if let Some(v) = temp.get("trusted") { v == &"true" } else { false },
};

if !control_file.superuser && control_file.trusted {
// `trusted` is irrelevant if `superuser` is false.
return Err(ControlFileError::RedundantField { field: "trusted" });
}

Ok(control_file)
}
}

Expand All @@ -100,6 +109,7 @@ impl From<ControlFile> for SqlGraphEntity {
#[derive(Debug, Clone)]
pub enum ControlFileError {
MissingField { field: &'static str },
RedundantField { field: &'static str },
}

impl std::fmt::Display for ControlFileError {
Expand All @@ -108,6 +118,9 @@ impl std::fmt::Display for ControlFileError {
ControlFileError::MissingField { field } => {
write!(f, "Missing field in control file! Please add `{field}`.")?;
}
ControlFileError::RedundantField { field } => {
write!(f, "Redundant field in control file! Please remove `{field}`.")?;
}
};
Ok(())
}
Expand Down