pgcentralfoundation / pgrx

Build Postgres Extensions with Rust!
Other
3.56k stars 239 forks source link

hash and btree operators generated from `PostgresHash` and/or `PostgresOrd` derives don't respect `#[pg_schema]` #1456

Open hoodiecollin opened 8 months ago

hoodiecollin commented 8 months ago

The SQL generated for the PostgresHash and PostgresOrd derive macros doesn't qualify the custom type with the correct schema.

Minimal example taken from my project:

#[pg_schema]
mod rs {
    /// standard Rust equality/comparison derives
    #[derive(Eq, PartialEq, Ord, Hash, PartialOrd)]
    /// Support using this struct as a Postgres type, which the easy way requires Serde
    #[derive(Clone, Copy, Debug, PostgresType, Serialize, Deserialize)]
    /// Support basic Postgres equality/comparison functions + Hash and BTree ops
    #[derive(PostgresEq, PostgresHash, PostgresOrd)]
    #[repr(transparent)]
    #[pgvarlena_inoutfuncs]
    pub struct CompactUuid([u8; 32]);

    // ...
}

Generates the following SQL (irrelevant parts omitted):

CREATE TYPE rs.CompactUuid;

CREATE  FUNCTION rs."compactuuid_hash"(
    "value" rs.CompactUuid /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
) RETURNS INT /* i32 */
IMMUTABLE STRICT PARALLEL SAFE
LANGUAGE c /* Rust */
AS 'MODULE_PATHNAME', 'compactuuid_hash_wrapper';

CREATE OPERATOR FAMILY CompactUuid_hash_ops USING hash;
CREATE OPERATOR CLASS CompactUuid_hash_ops DEFAULT FOR TYPE CompactUuid USING hash FAMILY CompactUuid_hash_ops AS
    OPERATOR    1   =  (CompactUuid, CompactUuid),
    FUNCTION    1   compactuuid_hash(CompactUuid);

CREATE  FUNCTION rs."compactuuid_cmp"(
    "left" rs.CompactUuid, /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
    "right" rs.CompactUuid /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
) RETURNS INT /* i32 */
IMMUTABLE STRICT PARALLEL SAFE
LANGUAGE c /* Rust */
AS 'MODULE_PATHNAME', 'compactuuid_cmp_wrapper';

CREATE OPERATOR FAMILY CompactUuid_btree_ops USING btree;
CREATE OPERATOR CLASS CompactUuid_btree_ops DEFAULT FOR TYPE CompactUuid USING btree FAMILY CompactUuid_btree_ops AS
    OPERATOR 1 <,
    OPERATOR 2 <=,
    OPERATOR 3 =,
    OPERATOR 4 >=,
    OPERATOR 5 >,
    FUNCTION 1 compactuuid_cmp(CompactUuid, CompactUuid);

The PostgresEq macro generates this which works as expected:

CREATE  FUNCTION rs."compactuuid_eq"(
    "left" rs.CompactUuid, /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
    "right" rs.CompactUuid /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
) RETURNS bool /* bool */
IMMUTABLE STRICT PARALLEL SAFE
LANGUAGE c /* Rust */
AS 'MODULE_PATHNAME', 'compactuuid_eq_wrapper';

CREATE OPERATOR rs.= (
    PROCEDURE=rs."compactuuid_eq",
    LEFTARG=rs.CompactUuid, /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
    RIGHTARG=rs.CompactUuid, /* pgrx::datum::varlena::PgVarlena<compact_uuid::rs::CompactUuid> */
    COMMUTATOR = =,
    NEGATOR = <>,
    RESTRICT = eqsel,
    JOIN = eqjoinsel,
    HASHES,
    MERGES
);
workingjubilee commented 8 months ago

The impl ToSql for PostgresOrdEntity needs to be fixed so that it actually uses the context: &PgrxSql argument to determine what schema to use for schema qualification:

https://github.com/pgcentralfoundation/pgrx/blob/4fa87bd57d5e673e280c987a75cd8129ff349f43/pgrx-sql-entity-graph/src/postgres_ord/entity.rs#L84-L103

Same for PostgresHashEntity:

https://github.com/pgcentralfoundation/pgrx/blob/4fa87bd57d5e673e280c987a75cd8129ff349f43/pgrx-sql-entity-graph/src/postgres_hash/entity.rs#L64-L82

This will somewhat resemble, if you squint really hard, how PostgresTypeEntity walks the graph:

https://github.com/pgcentralfoundation/pgrx/blob/4fa87bd57d5e673e280c987a75cd8129ff349f43/pgrx-sql-entity-graph/src/postgres_type/entity.rs#L71-L147

workingjubilee commented 8 months ago

Alternatively, all such code could be handled in a more uniform way, but I don't have any good ideas for that off the top of my head.