Opened 12 months ago
Closed 12 months ago
#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 )
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 , 12 months ago
Description: | modified (diff) |
---|
comment:2 by , 12 months ago
Keywords: | regression removed |
---|---|
Type: | Bug → Cleanup/optimization |
comment:3 by , 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 , 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.
comment:5 by , 12 months ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
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".
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.