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

Panics on different postgres types and custom types #9

Open
yellowHatpro opened this issue Apr 1, 2024 · 13 comments · May be fixed by #14
Open

Panics on different postgres types and custom types #9

yellowHatpro opened this issue Apr 1, 2024 · 13 comments · May be fixed by #14

Comments

@yellowHatpro
Copy link

Problem

  • sql-gen panics on some postgres types, like:
    "_int8" => "i64",
    "_int4" => "i32",
    "_int2" => "i16",
    "_text" => "String",
    "jsonb" => "serde_json::Value",
    "time" => "chrono::NaiveTime",
    "bool" => "bool",
    "bpchar" => "String",
    "char" => "String",
    "character" => "String",
  • it also fails to work with custom types/enums
@yellowHatpro
Copy link
Author

@jayy-lmao postgres types can be added with no issues, how can we have custom enums generated ?
I have to make them manually for this purpose, otherwise on generation it just panics.

@jayy-lmao
Copy link
Owner

jayy-lmao commented Apr 1, 2024

I've mostly aimed to support the types listed in https://docs.rs/sqlx/latest/sqlx/postgres/types/index.html.
However, I can see that some of the types there haven't been included, or are failing. I'll correct those in #10.

Custom enums seem like something we may want to add, but it will be a new feature and may take some work.

Some of these types with the underscore prefix, are you referring to that type without the underscore or is the underscore prefix common in Postgres? I see it's used for internal types, and not sure if sqlx will support it.

[Update: It looks like this may be because you're trying to use it with arrays, which also may need to be added]

@jayy-lmao
Copy link
Owner

I will add a naive support for arrays, which hopefully addresses these _<type> issues

@yellowHatpro
Copy link
Author

I have been working on a project which needs sql-gen, maybe I can help with fixing the errors due to custom enums. Regarding, the internal types, the above mentioned ones are some I encountered while working on the project, I'll make a pr for their mappings with appropriate rust types. If I find any else type panicking, I will update.

@yellowHatpro
Copy link
Author

Any idea how we can get started with supporting custom types?
Do we need to take args from the user for this purpose?

@jayy-lmao
Copy link
Owner

jayy-lmao commented Apr 1, 2024

Regarding, the internal types, the above mentioned ones are some I encountered while working on the project

I have merged the PR that allows for array type, time, and character types. Let me know if it has resolved any of the above.
I am happy for you to have a go at adding enum support. I should be able to review any changes this time of day.
The outputted syntax should match https://docs.rs/sqlx/latest/sqlx/postgres/types/index.html#enumerations.

Any idea how we can get started with supporting custom types?

The only custom type I can see supported by sqlx is enum type. I would check if there's a way to query postgres for enums and their options, then map those to models with their own model files. Then you would maybe need a list of which ones you had done this for, so that when a table references them, you can map it to a type on the struct.

It might be a bit of work.

@jayy-lmao
Copy link
Owner

jayy-lmao commented Apr 1, 2024

@yellowHatpro here is a query you may wish to use

SELECT
    n.nspname AS schema_name,
    t.typname AS enum_name,
    e.enumlabel AS enum_option
FROM
    pg_type t
JOIN
    pg_enum e ON t.oid = e.enumtypid
JOIN
    pg_catalog.pg_namespace n ON n.oid = t.typnamespace
WHERE
    t.typtype = 'e'
ORDER BY
    n.nspname,
    t.typname,
    e.enumsortorder;

To reiterate I think:
Step 1: Generate enum rust code from output of this query
Step 2: Update code that generates models to include custom types (and only custom types outputted from step 1). The function that maps them may need an additional field for a list of custom types [update: or you could map custom type fields outside this function, and only call it for fields which aren't a known custom type]

Then later we may want to
Step 3: detect altered rust enums to generate migration sql code to add new enum variants (like we do with added struct fields)

@yellowHatpro
Copy link
Author

Thanks, that was really helpful!
I will soon try the changes locally, and then ping here with the findings 😄

@yellowHatpro
Copy link
Author

Hi @jayy-lmao , I tried with your query and it worked for custom enums ✅✅.
I wrapped the convert data type function in another function which checks for custom types, and replaced it wherever convert_data_type was being called:

pub fn convert_type(data_type: &str,
                    custom_types: &Vec<String>) -> String {
    if custom_types.contains(&data_type.to_string()) {
        return to_pascal_case(data_type);
    } else {
        convert_data_type(data_type)
    }
}

However, now the error is popping on cube, which I think is a postgres extension. Is there any way to include cube (or any available extension present) in the sql query you highlighted?

@jayy-lmao
Copy link
Owner

Looking good!

If that's the case maybe in addition to the enum support we might need two changes:

  1. Rather than panicking, just throwing a warning and leaving out Tables that have an "erroring field"
  2. Allow the user to add overrides for type mappings.

Either something like

--override-type=cube:Vec<int32>

Or maybe more based on the table,field

--override-field=mytable.somefield:Vec<int32>

Either or both could be environment variables, and accept multiple values.
However for this idea to be useful to your case, I'd need to know if/how sqlx can support cube.
Haven't found anything so far, so will have to have a try myself when I get the chance.

Let me know if you see / know how to support cube with sqlx 👍

@yellowHatpro
Copy link
Author

Right, I had a look couple months back regarding sqlx cube support, didn't find anything relevant.
Will have a look again soon. 😄

@jayy-lmao
Copy link
Owner

I had a bit of investigate - and no support.
I've gotten a working implementation of sqlx::Encode and I'm working on sqlx::Decode. I'll make a PR to sqlx to see if they want it, but otherwise I might just release it as a small crate so that you can use it for your project. Or give you the code yourself and add that override feature.

@yellowHatpro
Copy link
Author

That's awesome! Thanks for the help !! 😀😀

@jayy-lmao jayy-lmao linked a pull request Aug 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants