weldsorm / welds

BSD 3-Clause "New" or "Revised" License
119 stars 6 forks source link

Postgres (v15) TEXT[] is view as _TEXT in wields check #14

Closed NexRX closed 9 months ago

NexRX commented 10 months ago

Hi, I'm just getting started with this lovely crate! Really like the ergonomics.

But for some reason my schema migration contains arrays of text that aren't being interrupted properly by the checking process in wields.

I get the following error:

public.users (High): The Column `permissions` is has changed, db_type: _TEXT welds_type: TEXT[]
public.users (High): The Column `roles` is has changed, db_type: _VARCHAR welds_type: TEXT[]

Yet in my schema, I do try using TEXT[]:

permissions text[] NOT NULL DEFAULT array[]::text[],
roles varchar[] NOT NULL DEFAULT array[]::varchar[]

All other fields work fine (including text and customs), just not the array ones. Any clues?

lex148 commented 10 months ago

Congratulations! I think you found a bug :)

Would you mind sending me an example of your tables schema to help me get this fixed?

Best I can tell, the ORM is doing CRUD correctly on these types, just detecting changes where they don't exist. Is that what you are seeing?

NexRX commented 10 months ago

Sure thing!

-- User Types
CREATE TYPE authority AS ENUM(
    'None',
    'Moderator',
    'Admin',
    'SuperAdmin',
    'Owner'
);

-- Users
CREATE TABLE
    users (
        id UUID DEFAULT gen_random_uuid () PRIMARY KEY,
        enabled BOOLEAN NOT NULL,
        disabled_reason VARCHAR(255),
        username VARCHAR(255) NOT NULL UNIQUE,
        email VARCHAR(255) NOT NULL UNIQUE,
        has_image BOOLEAN NOT NULL,
        name VARCHAR(255),
        surname VARCHAR(255),
        joined TIMESTAMPTZ NOT NULL DEFAULT now(),
        authority AUTHORITY NOT NULL DEFAULT 'None',
        permissions text[] NOT NULL DEFAULT array[]::text[],
        roles varchar[] NOT NULL DEFAULT array[]::varchar[]
    );

Quite a minor workable bug at least 😁 As a work arround in my test I'm just filtering out these mismatches like so:

let filtered_diff: Vec<welds::check::Issue> = diff
    .iter()
    .filter(|v| {
        v.kind.as_changed().map_or(true, |v| {
            !((v.db_type == "_TEXT".to_string()
                || v.db_type == "_VARCHAR".to_string())
                && v.welds_type == "TEXT[]")
        })
    })
    .map(|v| v.to_owned()) // iter .to_owned() differed
    .collect();

Querying, (de-)serialzing and everything else works as expected I should note.

lex148 commented 9 months ago

fix for this is up at https://github.com/weldsorm/welds/pull/17

NexRX commented 9 months ago

Awesome, I'll close this since you've already got a pull request to track this. Thanks for the awesome work you're doing and support with the crate!