add TryGetableArray to DeriveValueType macros

[sinder38]

PR Info

• Closes DeriveEntityModel types don't implement TryGetable trait when wrapped in a Vec #2967
  User wrapped types created by DeriveValueType macros don't implement TryGetableArray which results in them unusable in FromQueryResult structs when in Vec<T>

Bug Fixes

• Allow User wrapped types to be used in Vec inside FromQueryResult structs

Breaking Changes

if users implemented TryGetableArray manually it will require removal

Changes

Adds missing implementation to macros

[sinder38]

umm, why is this even allowed?
this is quite confusing.

sea-orm/tests/common/features/value_type.rs

Lines 72 to 73 in e36eea4

	#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)]
	pub struct StringVec(pub Vec<String>);

error[E0277]: the trait bound `Vec<Vec<std::string::String>>: TryGetable` is not satisfied
  --> tests/common/features/value_type.rs:72:39
   |
72 | #[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)]
   |                                       ^^^^^^^^^^^^^^^ the trait `TryGetable` is not implemented for `Vec<Vec<std::string::String>>`
   |
   = help: the following other types implement trait `TryGetable`:
             Vec<Braced>
             Vec<Hyphenated>
             Vec<JsonValue>
             Vec<NaiveDate>
             Vec<NaiveDateTime>
             Vec<NaiveTime>
             Vec<OffsetDateTime>
             Vec<PrimitiveDateTime>
           and 21 others
   = note: this error originates in the derive macro `DeriveValueType` (in Nightly builds, run with -Z macro-backtrace for more info)

There seems to be 2 ways to solve this:

1. To add more implementations that would cover all cases
   Also I should mention that PostgreSQL supports up to 6 dimensions of arrays, even though I dought anyone nests more than 3

2. It is possible to handle this by not implementing TryGetableArray for types that have Vec inside of them but I think it is bad and confusing. Probably the best way for this option would be to allow user to mark Vec types with something like #[sea_orm(as_pg_array)].

[sinder38]

why not ready

This is not ready to be merged, I handled only DeriveValueTypeStruct and didn't change DeriveValueTypeString.

My reasoning behind it is that I don't fully understand the design goal yet, it could be either:

• "interpret strings in the dB as this complex type"
• "use this convenient wrapper to handle underlying dB type as string".

most likely the first one.

[sinder38]

Marking this PR as non-draft even though it is not ready to get active feedback...

[tyt2y3]

It is possible to handle this by not implementing TryGetableArray for types that have Vec inside of them
seems like this is a sensible way

[sinder38]

I determite whether to omit TryGetableArray implementation automatically using this:
https://github.com/SeaQL/sea-orm/pull/2972/changes#diff-30cac0d1ada3283259a6839cee56f20544324fabfc5b276387ab14d66cfad16eR157

Increase this once you decide to support dimensionality bigger than 1-2

// Dimensionality 2, which is unsupported
#[derive(FromQueryResult)]
pub struct Row {
    pub matrix: Vec<Vec<i32>> 
}

On separate note, I noticed that test and documentation don't handle array_type.
This:

#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)]
#[sea_orm(array_type="String")]
pub struct StringVecNoImpl(pub Vec<String>);

[sinder38]

Any plans to merge?

[sinder38]

@Huliiiiii, this is a frustrating bug for me. Please tell what else does the core team want for this to get merged?

[Huliiiiii]

Should this be pub(crate)?

[sinder38]

Doesn't have to since it used in one place only right now(honestly, don't remember if it can be used in more places).

[Huliiiiii]

What do you think of the name skip_try_getable_array_impl?
@tyt2y3

[tyt2y3]

thank you for the contribution.
I think I'd like the behaviour to be opt-in instead of opt-out.
so may be #[sea_orm(try_getable_array)] and support it for both the tuple-struct path and the value_type = "String" path?