Django

Code

Ticket #399 (assigned)

Opened 3 years ago

Last modified 1 week ago

Bigint field object needed

Reported by: jmadson@techie.com Assigned to: permon (accepted)
Milestone: Component: Database layer (models, ORM)
Version: SVN Keywords: sprintsept14, bigint
Cc: listuser@peternixon.net, richard.davies@elastichosts.com Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

Description

The existing IntegerField? is not adequate for generating schemas that require a "bigint" field. Either IntegerField? needs to take a size/length parameter or a new Field type is necessary.

Attachments

bigint_patch.txt (7.1 kB) - added by MattCroydon on 08/26/05 19:43:56.
bigint_patch_06aug06.txt (4.5 kB) - added by Gopal Narayanan <gopastro@gmail.com> on 08/26/06 21:20:44.
bigint_patch_03jan07.diff (4.4 kB) - added by Gaël Chardon <gael.dev@4now.net> on 01/03/07 07:09:11.
399.diff (5.1 kB) - added by Marc Fargas <telenieko@telenieko.com> on 05/20/07 16:12:30.
Last patch with docs added.
django-bigint-20070711.patch (7.0 kB) - added by Peter Nixon <listuser@peternixon.net> on 07/11/07 09:09:55.
Add BigIntegerField? support to django (SQLite, MySQL, Postgresql, Oracle, MSSQL)
django-bigint-20070712.patch (7.4 kB) - added by Peter Nixon <listuser@peternixon.net> on 07/12/07 04:48:07.
Add BigIntegerField?? support to django (SQLite, MySQL, Postgresql (and postgresql_psycopg2), Oracle, MSSQL)
patch_tests.diff (8.8 kB) - added by permon on 09/14/07 15:02:26.
Previous patch with simple tests
patch.diff (9.2 kB) - added by permon on 09/14/07 18:18:19.
Updated, added constraint checking
bigint-patch-2007-09-15.diff (9.2 kB) - added by permon on 09/15/07 11:15:18.
bigint-patch-2008-01-04.diff (9.2 kB) - added by permon on 01/04/08 15:45:37.
English typos
bigint-patch-2008-02-03.diff (7.8 kB) - added by permon on 02/03/08 15:20:32.
-1 problem fix
bigint-patch-2008-04-13.diff (6.8 kB) - added by cpinto on 04/12/08 20:35:43.
bigint-patch-2008-04-25.diff (8.7 kB) - added by mwdiers <martin@diers.us> on 04/25/08 11:21:39.
Restored tests to previously submitted patch
bigint-patch-2008-06-28.diff (7.4 kB) - added by permon on 06/28/08 18:08:05.
missing typecast

Change History

08/22/05 16:52:51 changed by jmadson@techie.com

Additionally, a signedness boolean option is needed on IntegerField?.

08/26/05 19:43:56 changed by MattCroydon

  • attachment bigint_patch.txt added.

08/26/05 19:49:41 changed by MattCroydon

See bigint_patch.txt for a BigIntegerField? and PositiveBigIntegerField?. I took it for a spin on mysql but it needs testing on postgres and sqlite (the backend-specific stuff is there but has not been tested).

jmadson: I think PositiveIntegerField?, SmallPositiveIntegerField?, or BigPostitiveIntegerField? might be what you're looking for unless I misunderstood.

08/28/05 15:59:29 changed by MattCroydon

BigIntegerField? seems OK on PostgreSQL, but PostitiveBigIntegerField? throws ProgrammingError: ERROR: integer out of range for 18446744073709551615. I'll see what I can do.

08/29/05 05:57:28 changed by MattCroydon

See my post for details, but it looks like a PostitiveBigInteger? on PostgreSQL is only going to reach 9 bajillion whereas MySQL does 18 bajillion. Still needs testing on sqlite.

08/26/06 21:20:44 changed by Gopal Narayanan <gopastro@gmail.com>

  • attachment bigint_patch_06aug06.txt added.

08/26/06 23:14:23 changed by Gopal Narayanan <gopastro@gmail.com>

Matt Croydon patch is now quite dated for the existing codebase. I have updated ticket #399 with the new patch. It is called bigint_patch_06aug06.txt. Since I am most familiar with MySQL, I modified the backend code for MySQL. The changes for Oracle, postgres, sqlite, etc. should be minimal, and should be done for django/db/backends. I have done some simple tests. Please take it out for a spin.

08/28/06 18:14:44 changed by anonymous

  • summary changed from Bigint field object needed to [patch] Bigint field object needed.

01/03/07 07:09:11 changed by Gaël Chardon <gael.dev@4now.net>

  • attachment bigint_patch_03jan07.diff added.

01/03/07 07:24:15 changed by Gaël Chardon <gael.dev@4now.net>

I have updated Gopal (previously Matt Croydon) patch against revision [4274]
See bigint_patch_03jan07.diff

01/17/07 22:41:38 changed by SmileyChris

  • component changed from Metasystem to Database wrapper.
  • stage changed from Unreviewed to Design decision needed.

03/20/07 19:00:17 changed by SmileyChris

  • needs_better_patch set to 1.
  • stage changed from Design decision needed to Accepted.
  • needs_docs set to 1.

Ok, on second thoughts this is a valid ticket.

Current patch needs some documentation and implementation for other database backends.

05/20/07 16:12:30 changed by Marc Fargas <telenieko@telenieko.com>

  • attachment 399.diff added.

Last patch with docs added.

05/20/07 16:13:29 changed by Marc Fargas <telenieko@telenieko.com>

  • needs_docs deleted.
  • summary changed from [patch] Bigint field object needed to Bigint field object needed.

needs_docs = 0 and stripping redundant prefix from summary.

07/11/07 09:09:55 changed by Peter Nixon <listuser@peternixon.net>

  • attachment django-bigint-20070711.patch added.

Add BigIntegerField? support to django (SQLite, MySQL, Postgresql, Oracle, MSSQL)

07/11/07 09:34:41 changed by jacob

  • needs_tests set to 1.

07/12/07 04:48:07 changed by Peter Nixon <listuser@peternixon.net>

  • attachment django-bigint-20070712.patch added.

Add BigIntegerField?? support to django (SQLite, MySQL, Postgresql (and postgresql_psycopg2), Oracle, MSSQL)

07/12/07 04:49:48 changed by Peter Nixon <listuser@peternixon.net>

Oops. I missed introspection support for postgresql_psycopg2. Updated patch attached.

07/12/07 05:35:15 changed by Peter Nixon <listuser@peternixon.net>

  • cc set to listuser@peternixon.net.

09/13/07 06:16:54 changed by Thomas Güttler <hv@tbz-pariv.de>

The patch django-bigint-20070712.patch is working for me (psycopg2)

09/14/07 15:02:26 changed by permon

  • attachment patch_tests.diff added.

Previous patch with simple tests

09/14/07 18:18:19 changed by permon

  • attachment patch.diff added.

Updated, added constraint checking

09/14/07 18:34:53 changed by permon

  • owner changed from nobody to permon.
  • needs_better_patch deleted.
  • status changed from new to assigned.
  • needs_tests deleted.
  • stage changed from Accepted to Ready for checkin.

09/14/07 18:41:14 changed by permon

  • keywords set to sprintsept14.

09/15/07 00:30:36 changed by mattmcc

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.

The latest patch's approach for constraint checking (sys.maxint) isn't going to work for 32bit systems.

09/15/07 11:15:18 changed by permon

  • attachment bigint-patch-2007-09-15.diff added.

09/15/07 11:17:01 changed by permon

  • needs_better_patch deleted.
  • stage changed from Accepted to Ready for checkin.

sys.maxint was changed to class constant.

(follow-up: ↓ 20 ) 09/19/07 19:51:37 changed by jacob

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.

This isn't yet ready for checkin. The MAX_BIGINT variable in django.db.models.fields is fixed, but in reality is dependant on both architecture and database backend. I'm not sure what the fix is, but hard-coding 64 bits is going to look as silly in ten years as hardocoding 32 bits does today.

(in reply to: ↑ 19 ) 09/19/07 20:11:03 changed by permon

Replying to jacob:

This isn't yet ready for checkin. The MAX_BIGINT variable in django.db.models.fields is fixed, but in reality is dependant on both architecture and database backend. I'm not sure what the fix is, but hard-coding 64 bits is going to look as silly in ten years as hardcoding 32 bits does today.

But we _define_ BIGINT as 64 bits long (as written in doc). So why not to hardcode it that it will be always 64bit? Also in all db backends it is defined as 64bit type. So, there is no same situation like with integer data type which has always denpended on size of CPU register. This will always be 64bits. If there is a variant that db backend cannot save 64bit value, then we cannot us bigint type at all. But all backends supports 64bit values, so there is no problem with this. Yes, I also don't like `magic' value, but I think that it is adequate here because python doesn't define anything similar to sys.maxint.

11/25/07 14:24:08 changed by Vitaliy <vitaliyf@gmail.com>

Typo in latest patch:

raise ValueError("Value is so small/large to fit to field")

should be

raise ValueError("Value is too small/large to fit this field")

(or something like that. Also, same message is in tests/modeltests/field_types/models.py)

(follow-up: ↓ 25 ) 12/09/07 22:25:42 changed by corporeal

The SQL specification defines bigint as a 64-bit signed integer, and as such the definition of bigint in the context of SQL will never change, just as the definition of int has not changed despite the need for 64-bit signed integers today. Thus I believe jacob's reason for delaying check-in of this patch to be invalid. Please change check this in as soon as possible.

12/29/07 14:09:45 changed by Aaron Krill <corporeal@infinitephilosophy.com>

  • stage changed from Accepted to Ready for checkin.

12/31/07 04:56:10 changed by ubernostrum

  • stage changed from Ready for checkin to Accepted.

"Ready for checkin" is a determination made by triagers and committers. Please don't set it unless you're one of those people.

01/04/08 15:45:37 changed by permon

  • attachment bigint-patch-2008-01-04.diff added.

English typos

(in reply to: ↑ 22 ) 01/04/08 15:52:48 changed by permon

  • stage changed from Accepted to Ready for checkin.

Replying to corporeal: Agreed, changing to 'ready for checkin'

(follow-up: ↓ 27 ) 01/04/08 15:57:12 changed by ramiro

  • stage changed from Ready for checkin to Accepted.

(in reply to: ↑ 26 ) 01/04/08 16:05:05 changed by permon

Replying to ramiro:

Please, please read http://code.djangoproject.com/ticket/399#comment:24 and http://www.djangoproject.com/documentation/contributing/#ticket-triage

Ticket triagers: community members who keep track of tickets, making sure the tickets are always categorized correctly.

Probably I don't understand this correctly. Who are triagers if not us? Is there any list of them? How to make some progress with tickets, whose are ready for months and no one moves them to next stage?

01/05/08 09:35:44 changed by telenieko

  • keywords changed from sprintsept14 to sprintsept14, bigint.
  • needs_better_patch deleted.
  • version set to SVN.

Recalling comments, to avoid triagers loosing time reading the full log to take conclusions:

  • In comment:20 jacob says that why would we hardcode that Bigint field to 64bits. Thus marking as "needs improvement".
  • But corporeal says: "The SQL specification defines bigint as a 64-bit signed integer"

So, hardcoding it is not that bad. maybe we could ask the backend about the bigint size, but writting this for every backend knowing that BIGINT **must be** 64bits long seems a bit cumbersome.

So then, I'm setting "needs improvement" to 0, any triager please move to "Ready for checkin" or "Please ask every backend if they feel right beeing 64bit long" :)

(follow-up: ↓ 30 ) 02/01/08 15:23:59 changed by honeyman

Sorry for meddling in, but shouldn't django/db/models/fields/init.py have

if value > self.MAX_BIGINT or value < -self.MAX_BIGINT - 1

instead of

if value > self.MAX_BIGINT or value < -self.MAX_BIGINT

?

(in reply to: ↑ 29 ) 02/03/08 15:20:06 changed by permon

Replying to honeyman:

Sorry for meddling in, but shouldn't django/db/models/fields/init.py have if value > self.MAX_BIGINT or value < -self.MAX_BIGINT - 1 instead of if value > self.MAX_BIGINT or value < -self.MAX_BIGINT ?

You're right. Appending new patch.

02/03/08 15:20:32 changed by permon

  • attachment bigint-patch-2008-02-03.diff added.

-1 problem fix

04/12/08 20:35:43 changed by cpinto

  • attachment bigint-patch-2008-04-13.diff added.

04/12/08 20:36:56 changed by cpinto

The patch I just submitted fixes an "integer out of range" in pgsql by specifying the internal type of the BigIntegerField?

(in reply to: ↑ description ) 04/25/08 10:46:12 changed by mwdiers <martin@diers.us>

There was a lot of work on this, and then it all just stopped, waiting for Triage team to move to check-in. I would like to repeat permon's question above.

How do we give a "shout-out" to the triage members when a ticket has met check-in requirements, but yet has been sitting for months, particularly when we don't know who they are? (I understand it's a volunteer effort and all, but it's still a valid question).

I really need this patch, and it seems like many others do as well. As I am working on the newforms-admin branch, I would like to not have to apply this patch everytime I update SVN.

04/25/08 11:21:39 changed by mwdiers <martin@diers.us>

  • attachment bigint-patch-2008-04-25.diff added.

Restored tests to previously submitted patch

(follow-up: ↓ 34 ) 04/25/08 15:45:10 changed by SmileyChris

  • stage changed from Accepted to Ready for checkin.

Yep, this does look ready for checkin to me. The best way to get tickets rolling again is to ask about them in django-dev group. Would you mind doing that now?

I'll promote anyway.

(in reply to: ↑ 33 ) 04/25/08 15:54:18 changed by mwdiers <martin@diers.us>

Replying to SmileyChris:

Yep, this does look ready for checkin to me. The best way to get tickets rolling again is to ask about them in django-dev group. Would you mind doing that now?

Thank you. I will do so right away.

(follow-up: ↓ 36 ) 05/01/08 13:10:14 changed by wesley

Hi, I have an issue with the latest patch (bigint-patch-2008-04-25.diff) and fixtures. When I load a fixtures file that contains BigInteger? fields, I have this error : "Value is too small/large to fit this field"

In file django/db/models/fields/init.py in class BigIntegerField? in function get_db_prep_save(self, value). The type of value parameter is unicode instead of long. (The content of value match correctly with the biginteger field in my fixtures file).

If I cast value parameter to long in the condition statement all works fine, here is my update :

    def get_db_prep_save(self, value):
        value_long =  long(value)
        if value_long > self.MAX_BIGINT or value_long < -self.MAX_BIGINT - 1: 
            raise ValueError("Value is to small/large to fit this field") 
        return super(BigIntegerField, self).get_db_prep_save(value)

(in reply to: ↑ 35 ) 05/01/08 15:37:10 changed by mwdiers <martin@diers.us>

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Unreviewed.

The value should have already been converted to a Long or an Integer by that point in the code. So the problem lies elsewhere. JSON and YAML serialization work just fine. It's only XML that is producing this error. From what I can tell, a regular IntegerField? should also be producing this error, but for some reason it is not, and I cannot determine why not.

In any case, the proper solution for something like this is to override to_python():

    def to_python(self, value):
        return super(BigIntegerField, self).to_python(long(value))

I'm changing this bug to Unreviewed, as it's obviously not ready for checkin.

(follow-up: ↓ 40 ) 05/01/08 16:21:40 changed by mwdiers <martin@diers.us>

Ok. I got it. The problem is introduced by trying to perform validation, and since IntegerField? does no validation at all - by design - it produces no error here. Model is smart enough to parse out the integer by the time it gets to the DB.

But there is a bigger question: Why are we validating this field at all? Why not rely upon the backend to provide its own validation, just like we do with IntegerField??

Seems to me the fix is to skip validation entirely. That would also avoid the whole mess with different bigint sizes on different architectures.

05/02/08 12:41:29 changed by mwdiers <martin@diers.us>

For anyone who would like BigInteger? support, there is no need to wait for or to apply this patch. If you are using newforms, the following is all you need:

from django.db.models.fields import IntegerField
from django.conf import settings

class BigIntegerField(IntegerField):
	empty_strings_allowed=False
	def get_internal_type(self):
		return "BigIntegerField"
	
	def db_type(self):
		return 'NUMBER(19)' if settings.DATABASE_ENGINE == 'oracle' else 'bigint'

05/31/08 17:09:44 changed by jacob

  • stage changed from Unreviewed to Accepted.

(in reply to: ↑ 37 ) 06/28/08 18:03:43 changed by permon

Replying to mwdiers <martin@diers.us>:

Ok. I got it. The problem is introduced by trying to perform validation, and since IntegerField? does no validation at all - by design - it produces no error here. Model is smart enough to parse out the integer by the time it gets to the DB. But there is a bigger question: Why are we validating this field at all? Why not rely upon the backend to provide its own validation, just like we do with IntegerField?? Seems to me the fix is to skip validation entirely. That would also avoid the whole mess with different bigint sizes on different architectures.

a) You're right, there is a possibility, that value arrives as string. So cast to long is necessary. b) Why there is validation -> main reason was that each backend behaves differently. If I remember well, SQLite saves clipped value, MySQL raises exception and so on. For me looked like best way to have same behaviour with all backends.

06/28/08 18:08:05 changed by permon

  • attachment bigint-patch-2008-06-28.diff added.

missing typecast

11/20/08 06:50:17 changed by Richard Davies <richard.davies@elastichosts.com>

  • cc changed from listuser@peternixon.net to listuser@peternixon.net, richard.davies@elastichosts.com.

11/24/08 22:10:41 changed by rcoup

the current patch doesn't handle None values - get_db_prep_save() should explicitly check for it:

def get_db_prep_save(self, value):
    if value is not None:
        value_long =  long(value)
        if value_long > self.MAX_BIGINT or value_long < -self.MAX_BIGINT - 1: 
            raise ValueError("Value is to small/large to fit this field") 
    return super(BigIntegerField, self).get_db_prep_save(value)

(follow-up: ↓ 44 ) 11/24/08 22:35:42 changed by mwdiers

I cannot speak for the developers, but when this patch has come up on the development group, time and again they have said that it is not needed, because it is now so easy to create your own special field types, on the fly, and in the case of a BigInt?, it is trivial to do so. See the above example which I gave, dated 5/2/08.

I completely agree with this, especially considering the direction that Python 3 seems to be heading, where there is no longer any distinction at all between an Int and a Long, and the fact that 64-bit architectures are quickly relegating the old Int/Long distinction to an historical observation.

I would not be surprised if they eventually close this ticket as a "wontfix".

(in reply to: ↑ 43 ) 11/25/08 17:04:20 changed by permon

  • stage changed from Accepted to Design decision needed.

Replying to mwdiers: There are two possibilities - we want to have BIGINT in all db backends, then this patch should be finished and incorporated into trunk. Second possibility is that bigint is not 'basic' field and we will discard this ticket. But it depends on somone from core developers. Again I think, that situation has changed after 1.0, so I will change status to 'design decision'.

Depending on result of this decision, I will try to finish the patch or discard it. From my point of view are both solutions valid.


Add/Change #399 (Bigint field object needed)




Change Properties
Action