Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#21821 closed Bug (fixed)

ORM lookup refactor broke Oracle tests

Reported by: Tim Graham Owned by: Anssi Kääriäinen
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Tests fail to run with the following traceback:

Traceback (most recent call last):
  File "./runtests.py", line 384, in <module>
    options.failfast, args)
  File "./runtests.py", line 226, in django_tests
    test_labels or get_installed(), extra_tests=extra_tests)
  File "/home/tim/code/django/django/test/runner.py", line 147, in run_tests
    old_config = self.setup_databases()
  File "/home/tim/code/django/django/test/runner.py", line 109, in setup_databases
    return setup_databases(self.verbosity, self.interactive, **kwargs)
  File "/home/tim/code/django/django/test/runner.py", line 299, in setup_databases
    verbosity, autoclobber=not interactive)
  File "/home/tim/code/django/django/db/backends/creation.py", line 367, in create_test_db
    test_database=True)
  File "/home/tim/code/django/django/core/management/__init__.py", line 167, in call_command
    return klass.execute(*args, **defaults)
  File "/home/tim/code/django/django/core/management/base.py", line 291, in execute
    output = self.handle(*args, **options)
  File "/home/tim/code/django/django/core/management/commands/migrate.py", line 149, in handle
    emit_post_migrate_signal(created_models, self.verbosity, self.interactive, connection.alias)
  File "/home/tim/code/django/django/core/management/sql.py", line 244, in emit_post_migrate_signal
    using=db)
  File "/home/tim/code/django/django/dispatch/dispatcher.py", line 198, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/home/tim/code/django/django/contrib/auth/management/__init__.py", line 85, in create_permissions
    ctype = ContentType.objects.db_manager(using).get_for_model(klass)
  File "/home/tim/code/django/django/contrib/contenttypes/models.py", line 49, in get_for_model
    defaults={'name': smart_text(opts.verbose_name_raw)},
  File "/home/tim/code/django/django/db/models/manager.py", line 77, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/tim/code/django/django/db/models/query.py", line 416, in get_or_create
    return self.get(**lookup), False
  File "/home/tim/code/django/django/db/models/query.py", line 345, in get
    num = len(clone)
  File "/home/tim/code/django/django/db/models/query.py", line 121, in __len__
    self._fetch_all()
  File "/home/tim/code/django/django/db/models/query.py", line 962, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/home/tim/code/django/django/db/models/query.py", line 264, in iterator
    for row in compiler.results_iter():
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 693, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 766, in execute_sql
    sql, params = self.as_sql()
  File "/home/tim/code/django/django/db/backends/oracle/compiler.py", line 43, in as_sql
    with_col_aliases=True)
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 107, in as_sql
    where, w_params = self.compile(self.query.where)
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 78, in compile
    return node.as_sql(self, self.connection)
  File "/home/tim/code/django/django/db/models/sql/where.py", line 105, in as_sql
    sql, params = qn.compile(child)
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 78, in compile
    return node.as_sql(self, self.connection)
  File "/home/tim/code/django/django/db/models/lookups.py", line 120, in as_sql
    lhs_sql = connection.ops.field_cast_sql(db_type, field_internal_type) % lhs_sql
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 277, in field_cast_sql
    if db_type and db_type.endswith('LOB'):
AttributeError: 'CharField' object has no attribute 'endswith'

Change History (14)

comment:1 by Anssi Kääriäinen <akaariai@…>, 10 years ago

Resolution: fixed
Status: newclosed

In f468662e2495292356e5fd75241621563893fd4f:

Fixed #21821 -- db_type argument for field_cast_sql

The db_type argument for connection.ops.field_cast_sql wasn't correctly
set after merge of custom lookups patch.

comment:2 by Tim Graham, 10 years ago

Resolution: fixed
Status: closednew

Thanks, tests are now running. There are some errors of this type:

======================================================================
ERROR: test_unique_for_date (validation.test_unique.PerformUniqueChecksTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/validation/test_unique.py", line 120, in test_unique_for_date
    p.full_clean()
  File "/home/tim/code/django/django/db/models/base.py", line 998, in full_clean
    self.validate_unique(exclude=exclude)
  File "/home/tim/code/django/django/db/models/base.py", line 808, in validate_unique
    date_errors = self._perform_date_checks(date_checks)
  File "/home/tim/code/django/django/db/models/base.py", line 928, in _perform_date_checks
    qs = model_class._default_manager.filter(**lookup_kwargs)
  File "/home/tim/code/django/django/db/models/manager.py", line 77, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/tim/code/django/django/db/models/query.py", line 685, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "/home/tim/code/django/django/db/models/query.py", line 703, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "/home/tim/code/django/django/db/models/sql/query.py", line 1287, in add_q
    clause, require_inner = self._add_q(where_part, self.used_aliases)
  File "/home/tim/code/django/django/db/models/sql/query.py", line 1316, in _add_q
    current_negated=current_negated, connector=connector)
  File "/home/tim/code/django/django/db/models/sql/query.py", line 1186, in build_filter
    condition = self.build_lookup(lookups, col, value)
  File "/home/tim/code/django/django/db/models/sql/query.py", line 1091, in build_lookup
    next = lhs.get_lookup(lookup)
  File "/home/tim/code/django/django/db/models/sql/datastructures.py", line 25, in get_lookup
    return self.output_type.get_lookup(name)
  File "/home/tim/code/django/django/db/models/lookups.py", line 12, in get_lookup
    return self.class_lookups[lookup_name]
TypeError: unhashable type: 'list'

Let me know if you need more details.

comment:3 by Shai Berger, 10 years ago

FWIW, I will look at this tomorrow if not solved by then.

comment:4 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In 994c842bc9266f3d1e36b10e9365d74844ccb28c:

Fixed Oracle query failures caused by lookup refactor

Refs #21821.

comment:5 by Anssi Kääriäinen, 10 years ago

I think I got the latest failure fixed. I don't have Oracle available just now so I can't verify that Oracle passes all tests. Leaving as unfixed until all tests pass.

comment:6 by Anssi Kääriäinen, 10 years ago

While working on these issues I recalled that I forgot to add in lookup support for Oracle when there are more than 1000 parameters for the lookup. This commit should fix it: https://github.com/akaariai/django/commit/ac8e62768f13ae2ece4288539eb753b753ae47cf. The commit also cleans up non-necessary rhs parameter for process_rhs().

I can't test the patch on Oracle before tomorrow. This is complex enough patch to require testing on Oracle, so I'll wait until I can test the patch.

comment:7 by Tim Graham, 10 years ago

New error with the above patch applied to master:

======================================================================
ERROR: test_regex (lookup.tests.LookupTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/lookup/tests.py", line 507, in test_regex
    ['<Article: f>', '<Article: fo>', '<Article: foo>', '<Article: fooo>'])
  File "/home/tim/code/django/django/test/testcases.py", line 829, in assertQuerysetEqual
    items = six.moves.map(transform, qs)
  File "/home/tim/code/django/django/db/models/query.py", line 140, in __iter__
    self._fetch_all()
  File "/home/tim/code/django/django/db/models/query.py", line 962, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/home/tim/code/django/django/db/models/query.py", line 264, in iterator
    for row in compiler.results_iter():
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 693, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 766, in execute_sql
    sql, params = self.as_sql()
  File "/home/tim/code/django/django/db/backends/oracle/compiler.py", line 40, in as_sql
    with_col_aliases=with_col_aliases)
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 107, in as_sql
    where, w_params = self.compile(self.query.where)
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 78, in compile
    return node.as_sql(self, self.connection)
  File "/home/tim/code/django/django/db/models/sql/where.py", line 105, in as_sql
    sql, params = qn.compile(child)
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 78, in compile
    return node.as_sql(self, self.connection)
  File "/home/tim/code/django/django/db/models/lookups.py", line 131, in as_sql
    operator_plus_rhs = self.get_rhs_op(connection, rhs_sql)
  File "/home/tim/code/django/django/db/models/lookups.py", line 135, in get_rhs_op
    return connection.operators[self.lookup_name] % rhs
KeyError: 'regex'

comment:8 by Anssi Kääriäinen, 10 years ago

I have three different fixes for custom lookups:

First two are oracle-only, last might affect other backends, too.

These were found by checking what sql.where make_atom() does differently compared to class based lookups.

comment:9 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In a9f171151814c03fcb0dcc87b153397e90562ea2:

Fixed Oracle in lookup when more than 1000 params

Refs #21821.

comment:10 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In b86321e9a42652909d9c3373f3b97fcc9ed2c290:

Fixed regex lookup on Oracle

Refs #21821

comment:11 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In 1c360dbbf56b76e1df7c945458ae2987306fcfcd:

Use date_extract_sql() for DateFields

Refs #21821

comment:12 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In 980eda01909c9fd5c5e16f276ca663e71371a0a2:

Fixed custom_lookups tests for Oracle

Refs #21821

comment:13 by Anssi Kääriäinen, 10 years ago

Resolution: fixed
Status: newclosed

This one should be now fixed.

Interesting tests seem to pass even without 1c360dbbf56b76e1df7c945458ae2987306fcfcd. I haven't ran full test suite on Oracle, but other databases pass without that, and timezones tests pass on Oracle also without that commit.

I noticed that the __search lookup isn't tested at all. It is only supported on MySQL. It likely doesn't work currently, but that is food for another ticket.

Version 0, edited 10 years ago by Anssi Kääriäinen (next)

in reply to:  13 comment:14 by Jakub Roztočil, 10 years ago

Replying to akaariai:

This one should be now fixed.

Interestingly tests seem to pass even without 1c360dbbf56b76e1df7c945458ae2987306fcfcd. I haven't ran full test suite on Oracle, but other databases pass without that, and timezones tests pass on Oracle also without that commit.

I noticed that the __search lookup isn't tested at all. It is only supported on MySQL. It likely doesn't work currently, but that is food for another ticket.

Edit note: One reason for the high amount of regressions is that I intentionally did not try to move existing make_atom() logic to class based lookups. The make_atom logic was somewhat weird in some places. The end result should be much cleaner now.

The __search lookup indeed doesn't work. I've opened a ticked & pull request: #22489

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