polymorphic "bug" - don't add column if present

1 view
Skip to first unread message

Tvrtko

unread,
Feb 3, 2009, 3:27:28 PM2/3/09
to SQLElixir
There is a "bug" in EntityDescriptor.setup_table in a section of code
which is adding column if polymorphic option is used. In my oppinion,
column shouldn't be added if it is already present. This way I can
manually create column and still use polymorphic.

For example:

if self.polymorphic and \
self.inheritance in ('single', 'multi') and \
self.children and not self.parent:
self.add_column(Column(self.polymorphic,
options.POLYMORPHIC_COL_TYPE))

Should be:

if self.polymorphic and \
self.inheritance in ('single', 'multi') and \
self.children and not self.parent and \
self.get_column(self.polymorphic, False) is None:
self.add_column(Column(self.polymorphic,
options.POLYMORPHIC_COL_TYPE))

Regards,
Tvrtko

Gaetan de Menten

unread,
Feb 4, 2009, 6:54:58 AM2/4/09
to sqle...@googlegroups.com
On Tue, Feb 3, 2009 at 21:27, Tvrtko <qvx...@gmail.com> wrote:
>
> There is a "bug" in EntityDescriptor.setup_table in a section of code
> which is adding column if polymorphic option is used. In my oppinion,
> column shouldn't be added if it is already present. This way I can
> manually create column and still use polymorphic.

It's a sane suggestion and I would like to honor it, but this could
lead to a potentially confusing behavior if people add a column named
"row_type", it's silently accepted, but then it's used behind their
back as the polymorphic column.

There ought to be a way for the user to tell "I know what I'm doing",
and an option only for that seems way overkill. Maybe the following
scheme would do:
split the polymorphic option into
* polymorphic: a simple boolean, True by default
* polymorphic_col: the name for the polymorphic col. The option would
default to None, and if it's not the default name ("rowtype") would be
used. Using a manually defined column would only work if the column
name was set manually.

I'm not really happy with this scheme either since it is a bit complicated...

--
Gaëtan de Menten
http://openhex.org

Tvrtko

unread,
Feb 5, 2009, 12:45:32 PM2/5/09
to SQLElixir
On Feb 4, 12:54 pm, Gaetan de Menten <gdemen...@gmail.com> wrote:
> On Tue, Feb 3, 2009 at 21:27, Tvrtko <qvx3...@gmail.com> wrote:
>
> > There is a "bug" in EntityDescriptor.setup_table in a section of code
> > which is adding column if polymorphic option is used. In my oppinion,
> > column shouldn't be added if it is already present. This way I can
> > manually create column and still use polymorphic.
>
> It's a sane suggestion and I would like to honor it, but this could
> lead to a potentially confusing behavior if people add a column named
> "row_type", it's silently accepted, but then it's used behind their
> back as the polymorphic column.
>
> There ought to be a way for the user to tell "I know what I'm doing",
> and an option only for that seems way overkill.

Well, this covers it nicely:

1. If users says polymorphic=True, then it behaves like now: raises an
exception when there is a "row_type" column present. This prevents
unexpected behaviour.

2. But, if user says polymorphic="anything", than the user knows what
column he wants to use for polymorphic column, and if he created such
column in advance he knows what it will be used for. If there is no
such
column, we can add it - just like now.

Tvrtko
Reply all
Reply to author
Forward
0 new messages