_fields and _writable_fields are OrderedDicts

src/infi/clickhouse_orm/database.py

@@ -148,7 +148,7 @@ class Database(object):
             raise DatabaseException("You can't insert into read only and system tables")
 
         fields_list = ','.join(
-            ['`%s`' % name for name, _ in first_instance._writable_fields])
+            ['`%s`' % name for name in first_instance.fields(writable=True)])
 
         def gen():
             buf = BytesIO()

src/infi/clickhouse_orm/migrations.py

@@ -6,6 +6,7 @@ from .engines import MergeTree
 from .utils import escape
 
 from six.moves import zip
+from six import iteritems
 
 import logging
 logger = logging.getLogger('migrations')
@@ -65,7 +66,7 @@ class AlterTable(Operation):
         table_fields = dict(self._get_table_fields(database))
 
         # Identify fields that were deleted from the model
-        deleted_fields = set(table_fields.keys()) - set(name for name, field in self.model_class._fields)
+        deleted_fields = set(table_fields.keys()) - set(self.model_class.fields())
         for name in deleted_fields:
             logger.info('        Drop column %s', name)
             self._alter_table(database, 'DROP COLUMN %s' % name)
@@ -73,7 +74,7 @@ class AlterTable(Operation):
 
         # Identify fields that were added to the model
         prev_name = None
-        for name, field in self.model_class._fields:
+        for name, field in iteritems(self.model_class.fields()):
             if name not in table_fields:
                 logger.info('        Add column %s', name)
                 assert prev_name, 'Cannot add a column to the beginning of the table'
@@ -89,7 +90,8 @@ class AlterTable(Operation):
         # The order of class attributes can be changed any time, so we can't count on it
         # Secondly, MATERIALIZED and ALIAS fields are always at the end of the DESC, so we can't expect them to save
         # attribute position. Watch https://github.com/Infinidat/infi.clickhouse_orm/issues/47
-        model_fields = {name: field.get_sql(with_default_expression=False) for name, field in self.model_class._fields}
+        model_fields = {name: field.get_sql(with_default_expression=False)
+                        for name, field in iteritems(self.model_class.fields())}
         for field_name, field_sql in self._get_table_fields(database):
             # All fields must have been created and dropped by this moment
             assert field_name in model_fields, 'Model fields and table columns in disagreement'

src/infi/clickhouse_orm/models.py

@@ -1,8 +1,9 @@
 from __future__ import unicode_literals
 import sys
+from collections import OrderedDict
 from logging import getLogger
 
-from six import with_metaclass, reraise
+from six import with_metaclass, reraise, iteritems
 import pytz
 
 from .fields import Field, StringField
@@ -23,15 +24,19 @@ class ModelBase(type):
     def __new__(cls, name, bases, attrs):
         new_cls = super(ModelBase, cls).__new__(cls, str(name), bases, attrs)
         # Collect fields from parent classes
-        base_fields = []
+        base_fields = dict()
         for base in bases:
             if isinstance(base, ModelBase):
-                base_fields += base._fields
+                base_fields.update(base._fields)
+
+        fields = base_fields
+
         # Build a list of fields, in the order they were listed in the class
-        fields = base_fields + [item for item in attrs.items() if isinstance(item[1], Field)]
-        fields.sort(key=lambda item: item[1].creation_counter)
-        setattr(new_cls, '_fields', fields)
-        setattr(new_cls, '_writable_fields', [f for f in fields if not f[1].readonly])
+        fields.update({n: f for n, f in iteritems(attrs) if isinstance(f, Field)})
+        fields = sorted(iteritems(fields), key=lambda item: item[1].creation_counter)
+
+        setattr(new_cls, '_fields', OrderedDict(fields))
+        setattr(new_cls, '_writable_fields', OrderedDict([f for f in fields if not f[1].readonly]))
         return new_cls
 
     @classmethod
@@ -107,14 +112,14 @@ class Model(with_metaclass(ModelBase)):
         self._database = None
 
         # Assign field values from keyword arguments
-        for name, value in kwargs.items():
+        for name, value in iteritems(kwargs):
             field = self.get_field(name)
             if field:
                 setattr(self, name, value)
             else:
                 raise AttributeError('%s does not have a field called %s' % (self.__class__.__name__, name))
         # Assign default values for fields not included in the keyword arguments
-        for name, field in self._fields:
+        for name, field in iteritems(self.fields()):
             if name not in kwargs:
                 setattr(self, name, field.default)
 
@@ -174,7 +179,7 @@ class Model(with_metaclass(ModelBase)):
         '''
         parts = ['CREATE TABLE IF NOT EXISTS `%s`.`%s` (' % (db_name, cls.table_name())]
         cols = []
-        for name, field in cls._fields:
+        for name, field in iteritems(cls.fields()):
             cols.append('    %s %s' % (name, field.get_sql()))
         parts.append(',\n'.join(cols))
         parts.append(')')
@@ -201,7 +206,7 @@ class Model(with_metaclass(ModelBase)):
         - `database`: if given, sets the database that this instance belongs to.
         '''
         from six import next
-        field_names = field_names or [name for name, field in cls._fields]
+        field_names = field_names or list(cls.fields())
         values = iter(parse_tsv(line))
         kwargs = {}
         for name in field_names:
@@ -221,8 +226,8 @@ class Model(with_metaclass(ModelBase)):
         - `include_readonly`: if false, returns only fields that can be inserted into database.
         '''
         data = self.__dict__
-        fields = self._fields if include_readonly else self._writable_fields
-        return '\t'.join(field.to_db_string(data[name], quote=False) for name, field in fields)
+        fields = self.fields(writable=not include_readonly)
+        return '\t'.join(field.to_db_string(data[name], quote=False) for name, field in iteritems(fields))
 
     def to_dict(self, include_readonly=True, field_names=None):
         '''
@@ -231,13 +236,13 @@ class Model(with_metaclass(ModelBase)):
         - `include_readonly`: if false, returns only fields that can be inserted into database.
         - `field_names`: an iterable of field names to return (optional)
         '''
-        fields = self._fields if include_readonly else self._writable_fields
+        fields = self.fields(writable=not include_readonly)
 
         if field_names is not None:
-            fields = [f for f in fields if f[0] in field_names]
+            fields = [f for f in fields if f in field_names]
 
         data = self.__dict__
-        return {name: data[name] for name, field in fields}
+        return {name: data[name] for name in fields}
 
     @classmethod
     def objects_in(cls, database):
@@ -246,6 +251,11 @@ class Model(with_metaclass(ModelBase)):
         '''
         return QuerySet(cls, database)
 
+    @classmethod
+    def fields(cls, writable=False):
+        # noinspection PyProtectedMember,PyUnresolvedReferences
+        return cls._writable_fields if writable else cls._fields
+
 
 class BufferModel(Model):
 

src/infi/clickhouse_orm/system_models.py

@@ -126,7 +126,7 @@ class SystemPart(Model):
         assert isinstance(conditions, string_types), "conditions must be a string"
         if conditions:
             conditions += " AND"
-        field_names = ','.join([f[0] for f in cls._fields])
+        field_names = ','.join(cls.fields())
         return database.select("SELECT %s FROM %s WHERE %s database='%s'" %
                                (field_names,  cls.table_name(), conditions, database.db_name), model_class=cls)
 

tests/test_inheritance.py

@@ -11,7 +11,7 @@ from infi.clickhouse_orm.engines import *
 class InheritanceTestCase(unittest.TestCase):
 
     def assertFieldNames(self, model_class, names):
-        self.assertEquals(names, [name for name, field in model_class._fields])
+        self.assertEquals(names, list(model_class.fields()))
 
     def test_field_inheritance(self):
         self.assertFieldNames(ParentModel, ['date_field', 'int_field'])