Option to have diesel_cli infer postgres array types as Array<T> instead of Array<Nullable<T>>

[mikeyhew]

Would you accept a PR that adds this as a configuration option to diesel.toml? We're in the process of upgrading from diesel 1.4 to diesel 2 and have a lot of postgres arrays in our schema, and the change to Array<Nullable> adds extra complexity to avoid an issue that we haven't encountered so far (arrays having null values). Our current workaround is to generate a schema.patch file that replaces all occurences of Array<Nullable> with Array, but it would be more robust if we could configure diesel_cli to infer Array as it did in diesel 1.x.

[weiznich]

Such an option sounds quite specific to me, although I can understand the need for this. I personally would like to go with a more generic approach there, that would introduce a general mechanism for remapping field types. Something like having a set of lookups in your diesel.toml that let you map to arbitrary types:

[[print_schema.type_map]]
database_typename = "int[]" #optional, maybe that can/should be a regex
schema_typename = "Array<Integer>" # optional, drop the column if not set
table_name = "foo" # optional, restrict to a specific table_name
schema_name = "bar" # optional, restrict to a specific schema name
column_name = "baz" # optional, restrict to a specific column name

(Field names, etc are obviously up for discussion)

I believe such a feature would not only address this specific use-case but allow to replace a large number of usages of schema.patch files.

[mikeyhew]

That seems like an interesting idea. For our use case of reverting to Array<T> instead of Array<Nullable<T>> we'd have to add an entry to the map for every array type in our schema, which isn't ideal but could be an improvement over the generated schema patch we're using right now

[weiznich]

I would expect that you should be able to cover that with a single regex for the type name, assuming that we go for the regex supported variant there.

[mikeyhew]

I was thinking the same but noticed this issue. Let's say we have this in our diesel.toml

[[print_schema.type_map]]
database_typename = "([^[\\]]+)[]"
schema_typename = "Array<$1>"

For int[] this would output Array<int>, but what we actually want is Array<Int4>

[weiznich]

Right, that wouldn't work. :(

That said: I don't think it wouldn't that bad to list all array types on it's own.

I'm happy to mentor an implementation of this feature, but I personally do currently not have the capacity to work on this.

[willbuckner]

Did we ever get anywhere with this? What's the current recommended way to generate Array<Text> for a TEXT[] field? Do I have to use a patch?

[weiznich]

We are still looking for contributors implementing the feature outlined above. We are happy to review your PR there.

Otherwise it's discouraged to post to old issues and discussion threads asking support questions or asking for updates

[willbuckner]

I'd be willing to try implementing it, but not sure if there's consensus on the recommended course of action. Should we go with your type_map idea above? This seemed like the best place for my post being that it's on-topic and shows that this is still something people are interested in, and I and others ended up on this thread searching for a proper workaround and what the roadmap plans are, so I disagree with shaming me for posting here.

[weiznich]

Well the first thing that you would need to do is to write an actual proposal for the new feature (as new discussion thread) and seek consensus there. I'm personally much in favor of the type_map approach as it would help solving other issues as well. This still needs a real design in place of the sketch given above. Due to other priorities I do not have the capacity to even work on the design, so you would be mostly on your own there. I would suggest nevertheless to first discuss the design before you start implementing this feature.

I and others ended up on this thread searching for a proper workaround

To be very clear about that: It's not a "workaround", you are actively seeking for features breaking the safety guarantees provided by diesel. For rather obvious reasons that's something I do not want to support in a straight forward way.

---

This seemed like the best place for my post being that it's on-topic and shows that this is still something people are interested in, and I and others ended up on this thread searching for a proper workaround and what the roadmap plans are, so I disagree with shaming me for posting here.

If you post to an old issue/discussion like this you likely send notifications/emails to at least the people involved in the original discussion + anyone that just subscribed to that thread. The older the discussion, the more likely that these people are just not interested in that discussion anymore. So it's just sending messages to people who might not want to receive them. There is no problem with just making a new discussion and linking the old one from there.

As for the roadmap: I already indicated in the thread before that this is not a feature I personally will work on. So it's not on my roadmap. I naturally cannot comment on what other people will contribute in the future, so it's not clear what you expect as "roadmap" here. After all that's an open source project and not a product, which mostly means the roadmap is whatever contributors like to work on and not what some user requests.

[mikeyhew]

Wow I had no idea GitHub had a threads feature. Fortunately this issue isn't that old and I'm probably the only person besides @weiznich getting pinged here.

The recommended approach is probably to just suck it up and use Vec<Option<T>> in your codebase. That's not very nice in my opinion though, and we're still using a script that runs through schema.rs with a regex, replaces all instances of Array<Nullable<T>> with Array<T>, and generates a patch file from the diff.

If the type map was implemented as described, I'd probably switch to it at some point even though you'd have to specify every array type individually, because that could still be generated and it wouldn't need to be regenerated as often as the schema patch.

[mikeyhew]

Also FWIW I appreciated getting pinged here, so I'd say just use your judgement about whether or not to reply to old threads, depending on how many people are in them and how old they are etc. I get notifications on old GitHub issues every so often and just unsubscribe if i don't care about them anymore