Opened 3 years ago

Closed 3 years ago

#18218 closed Bug (fixed)

get_table_list in postgresql introspection is unordered

Reported by: claudep Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: introspection
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

After looking for the cause of test failures after r17942, I realized that get_table_list result ordering is not consistent between backends.

The list is explicitely ordered by table name in sqlite3, implicitely ordered (SHOW TABLES) in MySQL, and unordered in PostgreSQL and Oracle.

I suggest to fix this by ordering by table name in PostgreSQL and Oracle and adding a test case.

Attachments (1)

18218-1.diff (2.1 KB) - added by claudep 3 years ago.
Sort table names in get_table_list

Download all attachments as: .zip

Change History (6)

comment:1 Changed 3 years ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

Seems like a good idea to me. This would result in the models created in alphabetical order, too?

Changed 3 years ago by claudep

Sort table names in get_table_list

comment:2 Changed 3 years ago by claudep

  • Has patch set

This would result in sorted models by table name in inspectdb, at least. Models creation order is probably another story, but indirectly, the former might influence the latter. Of course, the user is free to reorder models in inspectdb result before feeding it to the database.

comment:3 Changed 3 years ago by akaariai

The patch does not work correctly on my Oracle setup. I get this error:

Traceback (most recent call last):
  File "/home/akaariai/Programming/django/tests/regressiontests/introspection/tests.py", line 25, in _inner
    return func(*args, **kwargs)
  File "/home/akaariai/Programming/django/tests/regressiontests/introspection/tests.py", line 43, in test_table_names
    self.assertEqual(tl, sorted(tl))
AssertionError: Lists differ: [u'auth_group', u'auth_group_p... != [u'auth_group', u'auth_group_p...

First differing element 7:
django_comments
django_comment_flags

  [u'auth_group',
   u'auth_group_permissions',
   u'auth_permission',
   u'auth_user',
   u'auth_user_groups',
   u'auth_user_user_permissions',
   u'django_admin_log',
+  u'django_comment_flags',
   u'django_comments',
-  u'django_comment_flags',
   u'django_content_type',
   u'django_flatpage',
   u'django_flatpage_sites',
   u'django_ixn_test_table',
   u'django_redirect',
   u'django_session',
   u'django_site',
   u'inspectdb_digitsincolumnname',
   u'inspectdb_message',
   u'inspectdb_people',
   u'inspectdb_peopledata',
   u'inspectdb_peoplemoredata',
   u'introspection_article',
   u'introspection_reporter']

The reason for the error seems to be that Oracle sorts '_' and 's' characters differently than Python. In general the sort order of the database might be different than the sort order of Python. A good enough fix for this case is to check just one pair of tables, hopefully a pair which is incorrectly sorted without the patch, but correctly sorted with the patch.

There are a couple of other Oracle tests failing already, but adding more isn't a good way forward.

BTW In comment:1 I meant to say that the model order created by inspectdb is altered (a positive side-effect), not that model creation order is altered. I managed to write the wrong thing there...

comment:4 Changed 3 years ago by akaariai

I went the easy route and just made introspection.table_names() return sorted() table names. This avoids the use of the Database's ORDER BY. I also reordered the introspection methods & usage a little, that is get_table_list() is never used directly outside of backends, always use table_names() instead.

See https://github.com/django/django/pull/16

comment:5 Changed 3 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

Merged.

Note: See TracTickets for help on using tickets.
Back to Top