Redo SQL-gen macros to handle things uniformly

[workingjubilee]

Should we name DDL-gen macros for their DDL? e.g. derive(CreateType) and #[create_function(or_replace)]?

This is just a thought, because I feel that stuff like PostgresType is a little vague, as is pg_extern. I have seen signs that sometimes people get confused and think you should be using that for "all functions used 'for Postgres'", when that isn't the case as many should be, e.g. in callbacks, regular extern "C". They have somewhat more specific purposes, e.g. generating DDL, and should be clearly marked. Thus this is effectively a sub-issue of #1454.

The only problem is that it wouldn't strictly replace e.g. pg_extern as a notion, because pg_extern does more than just emit DDL but also handles the Postgres V1 ABI, and you can also turn off schema emission, and etc. In other words, the names are somewhat vague because the concepts they define are also somewhat nebulous.

On the other hand, maybe truly separating "generate DDL for this" and "handle the ABI" as notions, with a sensible default of "both", is exactly what is called for.

[workingjubilee]

There are errors we could catch if we were clearer about linking things together, at least. For instance, this snippet:

extension_sql!(
    r#"
    CREATE OR REPLACE PROCEDURE my_proc(name VARCHAR) LANGUAGE C AS 'MODULE_PATHNAME', 'my_proc';
    "#,
    name = "my_proc"
);
#[pg_guard]
pub extern "C" fn my_proc(name: *const c_char) {
    todo!()
}

This code exists because at the time, pgrx doesn't have support for CREATE PROCEDURE. Little bodges like this have to be allowed to exist and to work. But that doesn't mean they have to have bad UX. The extension_sql!, despite being written right beside, doesn't directly attach itself to fn my_proc for macro expansion, so it is hard to then examine my_proc and see that the arg is *const c_char, which is simply not correct, as it should be FunctionCallInfo or *mut FunctionCallInfoBaseData. And any change which altered pg_guard to participate in this kind of argument-checking would be a pointless and invasive regression of build times.

Apparently pg_extern supports an sql = .. arg, but there's no way to reason from that to what the user meant, as far as I can see, given it uses this API:

• feat: extend sql generation via to_sql attribute #410

pgrx/pgrx-macros/src/lib.rs

Lines 1157 to 1166 in 70cab89

	## Usage for configuring SQL generation
	This attribute can be used to control the behavior of the SQL generator on a decorated item,
	e.g. `#[pgrx(sql = false)]`
	Currently `sql` can be provided one of the following:
	* Disable SQL generation with `#[pgrx(sql = false)]`
	* Call custom SQL generator function with `#[pgrx(sql = path::to_function)]`
	* Render a specific fragment of SQL with a string `#[pgrx(sql = "CREATE FUNCTION ...")]`

[workingjubilee]

Note that this doesn't "really" work for #[pg_trigger], which is instead just a CREATE FUNCTION that follows a certain format.

...question to answer: why is it not, in fact, #[create_function] with a specific arg/return value, especially given that #1475 is a problem?

[workingjubilee]

This initially was about a rename.

But since then, I have noticed there are so many small, subtly related issues, like:

• Refactoring ExternArgs -> SQL gen for type safety #690
• pg_trigger does not support security_definer #1475
• Allow CREATE OR REPLACE AGGREGATE #1552

In each case, it's a sign we're not thinking quite precisely about how the SQL itself works and thus the arguments we must allow an extension writer to impute to it, but are instead playing whack-a-mole with small problems. Emitting SQL needs first-class handling instead of being a bunch of ad-hoc format_args! invocations in pgrx-sql-entity-graph, but so do the macro inputs.

I think the real problem here isn't necessarily about the names, but about handling things much more consistently, and with careful thought about the actual SQL. Which might mean we should rename things, since we probably don't actually want creating a function usable in a trigger to be distinct from creating a function.

[workingjubilee]

One possible structure I am thinking of is implementing a clap-like derive for an SQL-formattable struct on our end that also determines the base set of arguments that the macro will accept. Then when we need to adjust the SQL, contributors just have to add a field. This may actually be doable entirely within a decl macro tho'.

It may also not be worth macros at all, this is just a thought. Having a highly regular handling, with each struct implementing a trait that defines it as having a "format to SQL" call, may be sufficient.