kaleidos / grails-postgresql-extensions

Grails plugin to use postgresql native elements such as arrays, hstores,...
Apache License 2.0
78 stars 62 forks source link

Can we serialize Enum[] to String[] instead of int[]? #71

Open ph4t opened 9 years ago

ph4t commented 9 years ago

Currently the implementation will serialize an array of enums to an array of ordinal values. Using the Juice enum example from the documentation: [Juice.ORANGE, Juice.GRAPE] will be stored as [0, 2]

I think there are a few difficulties with this approach:

  1. If the order of enums is changed, this will change the ordinal values. As result, aren't you going to get wrong enums when existing record are read? E.g.: Assume we have [0,2] stored. If we change the enum to
static enum Juice {
        ORANGE(0),
        GRAPE(2),
        APPLE(1)
}

Shouldn't we get [Juice.ORANGE, Juice.APPLE] instead of original [Juice.ORANGE, Juice.GRAPE] ?

Btw, if ordinal values are used, why do we have value field in Juice? Aren't these 2 unrelated?

  1. If the table is used by other teams/tools we need to share mapping of our ordinal values to enum names with them. It would be much more descriptive if we just had varchars...
  2. GORM will map an Enum field to a varchar type. So it would make more sense to map Enum[] to varchar[]
ilopmar commented 8 years ago

Hi.

First of all I'm sorry for taking too much time to answer this issue.

Currently I'm working on migrating all the Java code to Groovy + @CompileStatic and I'm studying the file in which these mappings are done BidiEnumMap.

I'm not the original author of this code and I don't remember why we did this but after analyzing the code I think you're right. It seems strange to store the ids instead of just storing the value itself.

I'll ask with @Alotor after my holidays because he did the code but I think we should change this to just store the value.

edwardotis commented 8 years ago

+1 for String[] Storing enums as positional ints is a dangerous database anti-pattern.

pytsekas commented 6 years ago

Is there solution for this? Same as 102