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

added enum support #392

Open
wants to merge 1 commit into
base: orm
Choose a base branch
from
Open

added enum support #392

wants to merge 1 commit into from

Conversation

cdraf
Copy link

@cdraf cdraf commented Oct 14, 2018

Added an enum converter that creates a text column in the db.

Added Enum type support so query syntax (select/filter/where) can be used and a unittest (test_enum.py) that demonstrates each.

To support Enum in python 2.7 and 3.3 I've changed setup.py to conditionally add a dependency on pypi enum34.

@Martmists-GH
Copy link

Enums should also support ints and floats since those can be used too, so it would be recommended to add dynamic support for those.

@luckydonald
Copy link

luckydonald commented Oct 26, 2018

His enums in unit tests are using integers, so they get stored as string and converted back afterwards?

class Result(Enum):
     SUCCESS = 0
     FAILURE = 1
     UNKNOWN = 2

Would it store SUCCESS or 0 in that text field?

@luckydonald
Copy link

luckydonald commented Oct 26, 2018

I actually was hoping something like

class Language(Enum):
  EN = 0
  DE = 1
  ES = 4

would result in something like:

DROP TABLE IF EXISTS Language;
CRATE TABLE Language
  id INTEGER PRIMARY KEY,
  value TEXT NOT NULL
;
INSERT INTO Language (
  id, value
) VALUES (
  (0, 'EN'),
  (1, 'DE'),
  (4, 'ES')
);

@cdraf
Copy link
Author

cdraf commented Nov 3, 2018

I apologize for the delayed response.

@Martmists all data types should be supported since the Enum name is what gets stored in the db.

@luckydonald it will store the Enum name SUCCESS. Storing the name is not as efficient as storing the value in cases where the value (ex: 1) takes up less space than the name (ex: SUCCESS) but this way will support pretty much any Enum value. I don't know if this is the direction pony wants to go.

Another thing to consider is that some dbs support enumerations which may be a better way to represent the data. This is not something that these changes support.

@luckydonald your Language example is how it will look in the db.

@luckydonald
Copy link

luckydonald commented Nov 12, 2018

@cdraf I added a ES key to my example above to make my intensions about mapping the values more clear.

But, now that I think about it, enums are all about storing some arbitrary value behind an textual key.
This makes sense, not to include the value into the database as that also could change later.
From how I understand it now however you'll using an incremental integer key instead?

@peteretep
Copy link

is there any chance this will be merged? I'm looking for an orm with simple enum support.

@luckydonald
Copy link

@cdraf
But will it create the table Language, so that other tables would reference it?#

class Language(Enum):
  EN = 0
  DE = 1
  ES = 4

becomming

DROP TABLE IF EXISTS Language;
CRATE TABLE Language
  id INTEGER PRIMARY KEY,
  value TEXT NOT NULL
;
INSERT INTO Language (
  id, value
) VALUES (
  (0, 'EN'),
  (1, 'DE'),
  (4, 'ES')
);

and

class SomeTable(db.Entity):
  id = Autoinclement()
  lang = Required(Language)

becomes

CRATE TABLE SomeTable
  id INTEGER PRIMARY KEY,
  lang INTEGER NOT NULL REFERENCES Language.id
;

So will it have a table for the enum, and reference that one?

@andgein
Copy link

andgein commented Feb 16, 2020

Hi, @kozlovsky! Is there any chance this will be merged or supported by Pony ORM directly soon?

@half2me
Copy link

half2me commented Apr 2, 2020

also looking forward to this

@luckydonald
Copy link

I made a simpler approach (#502) which will make the database actually store the value, and not as proposed here, the key of the enum.

This should allow the same usage from a python side, but makes the handling of the values in the database trivial, as an IntEnum they can be handled as int everywhere, and it doesn't changes the type of queries.

Also the table column definition would directly correspond to the definition in the table, making upgrading from a normal field to a Enum based field easy as well.
For the above example of a language key that definition could now be even improved:

lang = Required(Language, length=2)

@jason-johnson
Copy link

Can this be modified to use native database enum support where available (e.g. postgres)?

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 this pull request may close these issues.

None yet

7 participants