#35077 closed Cleanup/optimization (invalid)

Quering with int that has bigint in database no longer working.

Reported by: Matej Spiller Muys Owned by: nobody
Component: Database layer (models, ORM) Version: 5.0
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Matej Spiller Muys)

If you have table with bigint column in DB but field is defined as int (implicit auto field) it no longer selects the value from the database.
It silently returns 0 records without any warning. In 4.2 it returned all the records regardless of defined type.
Database that is used is mysql 8.

class BalanceSessionStatus(models.Model):
    class Meta(object):
        db_table = 'market_balancesession_status'
        app_label = 'market'
        default_permissions = ('add',)

    status = models.PositiveSmallIntegerField(null=False)
    created_at = models.DateTimeField(null=False, auto_now=True)


insert_id = 5
# insert_id = 1477468537765888

market_models.BalanceSessionStatus(
    id=insert_id,
    status=0,
).save()

assert market_models.BalanceSessionStatus.objects.filter(id=insert_id).exists()

Table is created with:

CREATE TABLE `market_balancesession_status` (
  `id` bigint NOT NULL,
  `status` tinyint NOT NULL,
  `created_at` datetime(6) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci;

It works with id = 5 but fails with 1477468537765888. But in 4.2 it worked.

Same for 4.2 and 5.0

DEBUG    tests.readonly:readonly.py:98 (0.003) UPDATE `market_balancesession_status` SET `status` = 0, `created_at` = '2024-01-01 18:38:57.105947' WHERE `market_balancesession_status`.`id` = 5; args=(0, '2024-01-01 18:38:57.105947', 5)
DEBUG    tests.readonly:readonly.py:98 (0.002) INSERT INTO `market_balancesession_status` (`id`, `status`, `created_at`) VALUES (5, 0, '2024-01-01 18:38:57.122702'); args=[5, 0, '2024-01-01 18:38:57.122702']
DEBUG    tests.readonly:readonly.py:98 (0.002) SELECT 1 AS `a` FROM `market_balancesession_status` WHERE `market_balancesession_status`.`id` = 5 LIMIT 1; args=(1, 5)

5.x (for 1477468537765888)
It does not check if the id already exists in DB when saving and it is not able to select it after insert ... but insert does happen.

DEBUG    tests.readonly:readonly.py:98 (0.003) INSERT INTO `market_balancesession_status` (`id`, `status`, `created_at`) VALUES (1477468537765888, 0, '2024-01-01 18:42:22.524123'); args=[1477468537765888, 0, '2024-01-01 18:42:22.524123']

4.2 (for 1477468537765888)

DEBUG    tests.readonly:readonly.py:98 (0.002) UPDATE `market_balancesession_status` SET `status` = 0, `created_at` = '2024-01-01 18:45:33.122244' WHERE `market_balancesession_status`.`id` = 1477468537765888; args=(0, '2024-01-01 18:45:33.122244', 1477468537765888)
DEBUG    tests.readonly:readonly.py:98 (0.002) INSERT INTO `market_balancesession_status` (`id`, `status`, `created_at`) VALUES (1477468537765888, 0, '2024-01-01 18:45:33.129311'); args=[1477468537765888, 0, '2024-01-01 18:45:33.129311']
DEBUG    tests.readonly:readonly.py:98 (0.002) SELECT 1 AS `a` FROM `market_balancesession_status` WHERE `market_balancesession_status`.`id` = 1477468537765888 LIMIT 1; args=(1, 1477468537765888)

I understand that there is minor inconsistency between model and DB ...
However django shouldn't silently not select the value but should emit a warning or error if value is invalid.

Change History (5)

comment:1 by Matej Spiller Muys, 12 months ago

Description: modified (diff)

comment:2 by Simon Charette, 12 months ago

Keywords: regression removed
Type: BugCleanup/optimization

Maybe we could add a release note to this effect but I don't think we should be emitting a warning at run time when this happens. Django won't generate migration or introspect mismatching column and field types so I think it's safe to expect that things might break if you venture down this route.

This relates to #27397.

comment:3 by Matej Spiller Muys, 12 months ago

I agree that this is dangerous behaviour. But if you model django after existing database it can happen.

It is perfectly fine and appreciated to expose such problems. But hidding the problem into sometihing that seems like a valid result, it is not.

It seems that https://github.com/django/django/commit/dde2537fbb04ad78a673092a931b449245a2d6ae
overflow or underflow triggers empty recordset esentially hiding the problem.
I would expect under or overflow exception to be propagated to the caller.

comment:4 by Simon Charette, 12 months ago

I agree that this is dangerous behaviour. But if you model django after existing database it can happen.

I would argue that creating your models by hand from an existing database is dangerous by nature and the reported problem here is just a side effect. There is a documented way of generating models from a pre-existing table and it should generate the proper mapping.

I would expect under or overflow exception to be propagated to the caller.

It is propagated if you validate the data before hand through validation errors. If you pass invalid data to a queryset lookup I would say that you venture into undefined behaviour territory and changing it to raise an exception would be backward compatible. For example, think of all IntegerField (and it's derivative such as AutoField and PositiveIntegerField) which would result in an unhandled exception when doing SomeModel.objects.get(pk=pk) and pk is used controlled (e.g. passed through an url pattern) and thus an overflowing value can be provided.

The one premise that the ORM and its query generation machinery must be able to assume is that it has a proper Python level schema mapping to the tables it's generating SQL for. It you break this premise you should expect the unexpected.

Last edited 12 months ago by Simon Charette (previous) (diff)

comment:5 by Mariusz Felisiak, 12 months ago

Resolution: invalid
Status: newclosed

Thanks for the report, however I agree with Simon that it's not a valid issue per se. Using a different data type in the underlying database is not a "minor inconsistency".

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