#212 InheritableSQLObject childName column in sqlmeta.columns

closed-wont-fix
Oleg Broytman
None
5
2007-08-21
2007-02-21
No

from sqlobject import *
from sqlobject.inheritance import InheritableSQLObject

__connection__ = 'sqlite:///:memory:'

class Base(InheritableSQLObject):
base = IntCol()
Base.createTable()

print j.Base.sqlmeta.columns
{'base': <SOIntCol base>, 'childName': <SOStringCol childName default=None>}

I think 'childName' should not be "listed" in sqlmeta.columns because it's a SQLObject "artifact" (just like 'id' column).

This complicate automatic conversion from SQLObject to other types (like TurboJson's jsonify_sqlobject() function).

Discussion

  • Oleg Broytman
    Oleg Broytman
    2007-02-22

    • assigned_to: nobody --> phd
     
  • Oleg Broytman
    Oleg Broytman
    2007-02-22

    Logged In: YES
    user_id=4799
    Originator: NO

    SQLObject emulates "id" columns. Unlike that fictional column childName is a real column. There is no way to remove it from .columns.

     
  • Oleg Broytman
    Oleg Broytman
    2007-02-22

    • status: open --> closed-wont-fix
     
  • Logged In: YES
    user_id=240225
    Originator: YES

    Ok, it should be hard to fix, but don't you agree it's a bug? Shouldn't be emulated like the "id" column? It's not user data.

     
  • Oleg Broytman
    Oleg Broytman
    2007-02-23

    Logged In: YES
    user_id=4799
    Originator: NO

    Why is it a bug? It works as advertised. It is a real column, and it presents in .columns.

    In any case it would be so hard to fix, and fixing it gives so little advantage - I am not going to work on it. But I can test a patch if one appears.

     
  • Logged In: YES
    user_id=240225
    Originator: YES

    Because it doesn't behave like a real column (1665328 is an example, a regular column is inherited to all it's descendant, and the "None" behavior is another example too) and it's not user data.

    But I understand you don't see a great advantage on fixing it, i'll try to work on it when I have some time if you agree something is wrong and are interested in patches for this... (and don't want to work on a patch that it will not be accepted).

     
  • Oleg Broytman
    Oleg Broytman
    2007-02-28

    • status: closed-wont-fix --> open-wont-fix
     
  • Oleg Broytman
    Oleg Broytman
    2007-02-28

    Logged In: YES
    user_id=4799
    Originator: NO

    Reopened; see bug 1665328 for details. Now waiting for a patch...

     
  • Logged In: YES
    user_id=240225
    Originator: YES

    This is no exactly a great patch, and doesn't fix the base problem, but it provides a consistent way to get all the meaningfull columns for a user.

    The patch adds a function getColumns() to the sqlmeta object (and the inheritable version), that allways get all the columns for a class (including parent classes), excluding for the 'childName' column but including the 'id' column.

    This patch makes asDict() function use getColumns(), so there is no need to override it in the inheritable sqlmeta class. This makes asDict() to work properly on inheritable sqlobjects, avoiding the "AttributeError: 'SomeSQLObject' object has no attribute '_SO_val_childName'" error.
    File Added: sqlobject.r2871.getColumns.patch

     
  • Logged In: YES
    user_id=240225
    Originator: YES

    Here are some untested testcases (I don't know how to use nosetest :)

    They test asDict(), not getColumn() directly.
    File Added: sqlobject.r2871.asDict.tests.patch

     
  • Oleg Broytman
    Oleg Broytman
    2007-08-21

    Logged In: YES
    user_id=4799
    Originator: NO

    The patch changes the public API - it adds a new method and changes the existing ones - hence it can be applied only to the trunk. I also changed the patch - mostly changed names. Applied and committed in the revisions 2885, 2886.

    PS. We use py.test, not nosetest. :)

     
  • Logged In: YES
    user_id=240225
    Originator: YES

    It adds a new method, but why do you say that changes the existing ones? asDict() only changes for inheritable objects (for normal objects asDict() works exactly the same) , but it was broken anyways, so I don't see how this change could break any existing code, if existing code can't use asDict() for inheritable objects.

     
  • Oleg Broytman
    Oleg Broytman
    2007-08-21

    Logged In: YES
    user_id=4799
    Originator: NO

    A new method is enough not to apply the patch to a stable branch to prevent 0.9.1 being backward-incompatible with 0.9.2.

     
  • Logged In: YES
    user_id=240225
    Originator: YES

    Fair enough.

    Is this bug ready to be closed?

     
  • Oleg Broytman
    Oleg Broytman
    2007-08-21

    Logged In: YES
    user_id=4799
    Originator: NO

    Initially the bug was opened with a different formulation, but if you think it's ok to close it - let's close it.

     
  • Oleg Broytman
    Oleg Broytman
    2007-08-21

    • status: open-wont-fix --> closed-fixed
     
  • Logged In: YES
    user_id=240225
    Originator: YES

    Well, I think some sort of consistency about SQLObject "artifact" columns being in sqlmeta.columns is desirable, but since the solution is too hard to achieve and the workarround seems reasonable, the bug could be closed. I think Resolution should be Wont Fix, though, as the original problem, as you said, is not really fixed.

     
  • Oleg Broytman
    Oleg Broytman
    2007-08-21

    • status: closed-fixed --> closed-wont-fix