Skip to content

Commit

Permalink
Remove unused source-mapping code (#1424)
Browse files Browse the repository at this point in the history 
The code for `SqlMapping::Source` and `source_only_to_sql_type` has been
dead code for some time. I am become garbage collector, destroyer of
code.
  • Loading branch information
@workingjubilee
workingjubilee committed Dec 5, 2023
1 parent b80d139 commit ca46cf9
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 144 deletions.
35 changes: 2 additions & 33 deletions pgrx-sql-entity-graph/src/aggregate/entity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,22 +269,10 @@ impl ToSql for PgAggregateEntity {
.ok_or_else(|| {
eyre!("Macro expansion time suggested a composite_type!() in return")
}),
Ok(SqlMapping::Source { array_brackets }) => {
let sql = context
.source_only_to_sql_type(used_ty.ty_source)
.map(|v| fmt::with_array_brackets(v, array_brackets))
.ok_or_else(|| {
eyre!("Macro expansion time suggested a source only mapping in return")
})?;
Ok(sql)
}
Ok(SqlMapping::Skip) => {
Err(eyre!("Cannot use skipped SQL translatable type as aggregate const type"))
}
Err(err) => match context.source_only_to_sql_type(used_ty.ty_source) {
Some(source_only_mapping) => Ok(source_only_mapping.to_string()),
None => Err(err).wrap_err("While mapping argument"),
},
Err(err) => Err(err).wrap_err("While mapping argument"),
}
};

Expand Down Expand Up @@ -364,27 +352,8 @@ impl ToSql for PgAggregateEntity {
)
})?
}
Ok(SqlMapping::Source {
array_brackets,
}) => context
.source_only_to_sql_type(arg.used_ty.ty_source)
.map(|v| {
fmt::with_array_brackets(v, array_brackets)
})
.ok_or_else(|| {
eyre!(
"Macro expansion time suggested a source only mapping in return"
)
})?,
Ok(SqlMapping::Skip) => return Err(eyre!("Got a skipped SQL translatable type in aggregate args, this is not permitted")),
Err(err) => {
match context.source_only_to_sql_type(arg.used_ty.ty_source) {
Some(source_only_mapping) => {
source_only_mapping.to_string()
}
None => return Err(err).wrap_err("While mapping argument"),
}
}
Err(err) => return Err(err).wrap_err("While mapping argument")
},
variadic = if arg.used_ty.variadic { "VARIADIC " } else { "" },
maybe_comma = if needs_comma { ", " } else { " " },
Expand Down
19 changes: 0 additions & 19 deletions pgrx-sql-entity-graph/src/metadata/sql_translatable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ pub enum SqlMapping {
Composite {
array_brackets: bool,
},
/// Some types are still directly from source
Source {
array_brackets: bool,
},
/// Placeholder for some types with no simple translation
Skip,
}
Expand Down Expand Up @@ -185,9 +181,6 @@ where
Ok(SqlMapping::Composite { array_brackets: _ }) => {
Ok(SqlMapping::Composite { array_brackets: true })
}
Ok(SqlMapping::Source { array_brackets: _ }) => {
Ok(SqlMapping::Source { array_brackets: true })
}
Ok(SqlMapping::Skip) => Ok(SqlMapping::Skip),
err @ Err(_) => err,
},
Expand All @@ -204,9 +197,6 @@ where
Ok(Returns::One(SqlMapping::Composite { array_brackets: _ })) => {
Ok(Returns::One(SqlMapping::Composite { array_brackets: true }))
}
Ok(Returns::One(SqlMapping::Source { array_brackets: _ })) => {
Ok(Returns::One(SqlMapping::Source { array_brackets: true }))
}
Ok(Returns::One(SqlMapping::Skip)) => Ok(Returns::One(SqlMapping::Skip)),
Ok(Returns::SetOf(_)) => Err(ReturnsError::SetOfInArray),
Ok(Returns::Table(_)) => Err(ReturnsError::TableInArray),
Expand Down Expand Up @@ -237,15 +227,6 @@ unsafe impl SqlTranslatable for i32 {
}
}

unsafe impl SqlTranslatable for u32 {
fn argument_sql() -> Result<SqlMapping, ArgumentError> {
Ok(SqlMapping::Source { array_brackets: false })
}
fn return_sql() -> Result<Returns, ReturnsError> {
Ok(Returns::One(SqlMapping::Source { array_brackets: false }))
}
}

unsafe impl SqlTranslatable for String {
fn argument_sql() -> Result<SqlMapping, ArgumentError> {
Ok(SqlMapping::literal("TEXT"))
Expand Down
80 changes: 2 additions & 78 deletions pgrx-sql-entity-graph/src/pg_extern/entity/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,48 +164,8 @@ impl ToSql for PgExternEntity {
);
args.push(buf);
}
Ok(SqlMapping::Source { array_brackets }) => {
let sql_type = context
.source_only_to_sql_type(arg.used_ty.ty_source)
.map(|v| fmt::with_array_brackets(v, array_brackets))
.ok_or_else(|| {
eyre!(
"Macro expansion time suggested a source only mapping in return"
)
})?;
let buf = format!("\
\t\"{pattern}\" {variadic}{schema_prefix}{sql_type}{default}{maybe_comma}/* {type_name} */\
",
pattern = arg.pattern,
schema_prefix = context.schema_prefix_for(&graph_index),
// First try to match on [`TypeId`] since it's most reliable.
default = if let Some(def) = arg.used_ty.default { format!(" DEFAULT {}", def) } else { String::from("") },
variadic = if metadata_argument.variadic { "VARIADIC " } else { "" },
maybe_comma = if needs_comma { ", " } else { " " },
type_name = metadata_argument.type_name,
);
args.push(buf);
}
Ok(SqlMapping::Skip) => (),
Err(err) => {
match context.source_only_to_sql_type(arg.used_ty.ty_source) {
Some(sql_type) => {
let buf = format!("\
\t\"{pattern}\" {variadic}{schema_prefix}{sql_type}{default}{maybe_comma}/* {type_name} */\
",
pattern = arg.pattern,
schema_prefix = context.schema_prefix_for(&graph_index),
// First try to match on [`TypeId`] since it's most reliable.
default = if let Some(def) = arg.used_ty.default { format!(" DEFAULT {}", def) } else { String::from("") },
variadic = if metadata_argument.variadic { "VARIADIC " } else { "" },
maybe_comma = if needs_comma { ", " } else { " " },
type_name = metadata_argument.type_name,
);
args.push(buf);
}
None => return Err(err).wrap_err("While mapping argument"),
}
}
Err(err) => return Err(err).wrap_err("While mapping argument"),
}
}
String::from("\n") + &args.join("\n") + "\n"
Expand All @@ -230,14 +190,8 @@ impl ToSql for PgExternEntity {
let sql_type = match metadata_retval.return_sql {
Ok(Returns::One(SqlMapping::As(ref sql))) => sql.clone(),
Ok(Returns::One(SqlMapping::Composite { array_brackets })) => fmt::with_array_brackets(ty.composite_type.unwrap().into(), array_brackets),
Ok(Returns::SetOf(SqlMapping::Source { array_brackets })) => fmt::with_array_brackets(context.source_only_to_sql_type(ty.ty_source).unwrap(), array_brackets),
Ok(other) => return Err(eyre!("Got non-plain mapped/composite return variant SQL in what macro-expansion thought was a type, got: {other:?}")),
Err(err) => {
match context.source_only_to_sql_type(ty.ty_source) {
Some(source_only_mapping) => source_only_mapping,
None => return Err(err).wrap_err("Error mapping return SQL")
}
},
Err(err) => return Err(err).wrap_err("Error mapping return SQL")
};
format!(
"RETURNS {schema_prefix}{sql_type} /* {full_path} */",
Expand All @@ -260,7 +214,6 @@ impl ToSql for PgExternEntity {
let sql_type = match metadata_retval.return_sql {
Ok(Returns::SetOf(SqlMapping::As(ref sql))) => sql.clone(),
Ok(Returns::SetOf(SqlMapping::Composite { array_brackets })) => fmt::with_array_brackets(ty.composite_type.unwrap().into(), array_brackets),
Ok(Returns::SetOf(SqlMapping::Source { array_brackets })) => fmt::with_array_brackets(context.source_only_to_sql_type(ty.ty_source).unwrap(), array_brackets),
Ok(_other) => return Err(eyre!("Got non-setof mapped/composite return variant SQL in what macro-expansion thought was a setof")),
Err(err) => return Err(err).wrap_err("Error mapping return SQL"),
};
Expand All @@ -282,9 +235,6 @@ impl ToSql for PgExternEntity {
let composite = table_items[idx].ty.composite_type.unwrap();
fmt::with_array_brackets(composite.into(), *array_brackets)
},
SqlMapping::Source { array_brackets } => {
fmt::with_array_brackets(context.source_only_to_sql_type(table_items[idx].ty.ty_source).unwrap(), *array_brackets)
}
SqlMapping::Skip => todo!(),
}
}).collect()
Expand Down Expand Up @@ -449,19 +399,6 @@ impl ToSql for PgExternEntity {
.ok_or(eyre!("Found a composite type but macro expansion time did not reveal a name, use `pgrx::composite_type!()`"))?.to_string()
}
}
Ok(SqlMapping::Source { array_brackets }) => {
if array_brackets {
let composite_type = context
.source_only_to_sql_type(self.fn_args[0].used_ty.ty_source)
.ok_or(eyre!(
"Found a source only mapping but no source mapping exists for this"
))?;
format!("{composite_type}[]")
} else {
context.source_only_to_sql_type(self.fn_args[0].used_ty.ty_source)
.ok_or(eyre!("Found a composite type but macro expansion time did not reveal a name, use `pgrx::composite_type!()`"))?.to_string()
}
}
Ok(SqlMapping::Skip) => {
return Err(eyre!(
"Found an skipped SQL type in an operator, this is not valid"
Expand Down Expand Up @@ -502,19 +439,6 @@ impl ToSql for PgExternEntity {
.ok_or(eyre!("Found a composite type but macro expansion time did not reveal a name, use `pgrx::composite_type!()`"))?.to_string()
}
}
Ok(SqlMapping::Source { array_brackets }) => {
if array_brackets {
let composite_type = context
.source_only_to_sql_type(self.fn_args[1].used_ty.ty_source)
.ok_or(eyre!(
"Found a source only mapping but no source mapping exists for this"
))?;
format!("{composite_type}[]")
} else {
context.source_only_to_sql_type(self.fn_args[1].used_ty.ty_source)
.ok_or(eyre!("Found a composite type but macro expansion time did not reveal a name, use `pgrx::composite_type!()`"))?.to_string()
}
}
Ok(SqlMapping::Skip) => {
return Err(eyre!(
"Found an skipped SQL type in an operator, this is not valid"
Expand Down
6 changes: 0 additions & 6 deletions pgrx-sql-entity-graph/src/pgrx_sql.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,6 @@ impl PgrxSql {
})
}

pub fn source_only_to_sql_type(&self, _ty_source: &str) -> Option<String> {
// HACK for `Result<T, E>`
// ...well, actually, nothing!
None
}

pub fn get_module_pathname(&self) -> String {
if self.versioned_so {
let extname = &self.extension_name;
Expand Down
8 changes: 0 additions & 8 deletions pgrx/src/datum/array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,6 @@ where
SqlMapping::As(sql) => Ok(SqlMapping::As(format!("{sql}[]"))),
SqlMapping::Skip => Err(ArgumentError::SkipInArray),
SqlMapping::Composite { .. } => Ok(SqlMapping::Composite { array_brackets: true }),
SqlMapping::Source { .. } => Ok(SqlMapping::Source { array_brackets: true }),
}
}

Expand All @@ -927,9 +926,6 @@ where
Returns::One(SqlMapping::Composite { array_brackets: _ }) => {
Ok(Returns::One(SqlMapping::Composite { array_brackets: true }))
}
Returns::One(SqlMapping::Source { array_brackets: _ }) => {
Ok(Returns::One(SqlMapping::Source { array_brackets: true }))
}
Returns::One(SqlMapping::Skip) => Err(ReturnsError::SkipInArray),
Returns::SetOf(_) => Err(ReturnsError::SetOfInArray),
Returns::Table(_) => Err(ReturnsError::TableInArray),
Expand All @@ -946,7 +942,6 @@ where
SqlMapping::As(sql) => Ok(SqlMapping::As(format!("{sql}[]"))),
SqlMapping::Skip => Err(ArgumentError::SkipInArray),
SqlMapping::Composite { .. } => Ok(SqlMapping::Composite { array_brackets: true }),
SqlMapping::Source { .. } => Ok(SqlMapping::Source { array_brackets: true }),
}
}

Expand All @@ -958,9 +953,6 @@ where
Returns::One(SqlMapping::Composite { array_brackets: _ }) => {
Ok(Returns::One(SqlMapping::Composite { array_brackets: true }))
}
Returns::One(SqlMapping::Source { array_brackets: _ }) => {
Ok(Returns::One(SqlMapping::Source { array_brackets: true }))
}
Returns::One(SqlMapping::Skip) => Err(ReturnsError::SkipInArray),
Returns::SetOf(_) => Err(ReturnsError::SetOfInArray),
Returns::Table(_) => Err(ReturnsError::TableInArray),
Expand Down

0 comments on commit ca46cf9

Please sign in to comment.