Bug #40910 for CGI-Uploader: Columns width and height forced to accept nulls

Thu Nov 13 18:52:59 2008 RSAVAGE [...] cpan.org - Ticket created

Subject:

Columns width and height forced to accept nulls

The hash returned by sub extract_meta() uses width => $width, height => $height, when it should use width => $width || 0, height => $height || 0, Since the first version returns 2 undefs for non-image files, this means the declaration of those columns must accept nulls. I want to be able to declare them as: o height integer not null o width integer not null

Thu Apr 02 11:25:19 2009 MARKSTOS [...] cpan.org - Taken

Thu Apr 02 11:26:16 2009 MARKSTOS [...] cpan.org - Correspondence added

On Thu Nov 13 18:52:59 2008, RSAVAGE wrote: Show quoted text

> The hash returned by sub extract_meta() uses > width => $width, > height => $height, > when it should use > width => $width || 0, > height => $height || 0, > > Since the first version returns 2 undefs for non-image files, > this means the declaration of those columns must accept nulls. > > I want to be able to declare them as: > o height integer not null > o width integer not null

I'm not sure understand. It sounds like you want to upload non-images, but have their width and height declared as "0". Isn't is just as accurate to have them declared as null?

Thu Apr 02 11:26:17 2009 The RT System itself - Status changed from 'new' to 'open'

Thu Apr 02 11:26:18 2009 MARKSTOS [...] cpan.org - Status changed from 'open' to 'stalled'

Thu Apr 02 17:10:27 2009 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Thu, 2 Apr 2009 05:55:55 +1000 (EST)
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	"Ron Savage" <ron [...] savage.net.au>

Hi Mark MARKSTOS via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=40910 > > > On Thu Nov 13 18:52:59 2008, RSAVAGE wrote:

>> The hash returned by sub extract_meta() uses >> width => $width, >> height => $height, >> when it should use >> width => $width || 0, >> height => $height || 0, >> >> Since the first version returns 2 undefs for non-image files, >> this means the declaration of those columns must accept nulls. >> >> I want to be able to declare them as: >> o height integer not null >> o width integer not null

> > I'm not sure understand. It sounds like you want to upload non-images, > but have their width and height declared as "0". Isn't is just as > accurate to have them declared as null?

It's not that I want to upload non-images. I just want the code to handle that case. -- Ron Savage ron@savage.net.au http://savage.net.au/

Thu Apr 02 17:10:28 2009 The RT System itself - Status changed from 'stalled' to 'open'

Fri Apr 03 09:49:56 2009 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Fri, 3 Apr 2009 09:49:40 -0400
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

Show quoted text

> It's not that I want to upload non-images. I just want the code to handle > that case.

If you are only uploading images, doesn't it work to just set the data model to be "not null"? The only reason I could see for that to fail would be if the sizes could not be detected, in which case a NULL seems like a reasonable way to express that. Mark

Sat Apr 04 01:54:49 2009 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Sat, 04 Apr 2009 16:55:00 +1100
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Mark On Fri, 2009-04-03 at 09:49 -0400, mark@summersault.com via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=40910 > > >

> > It's not that I want to upload non-images. I just want the code to handle > > that case.

> > If you are only uploading images, doesn't it work to just set the data model to > be "not null"? The only reason I could see for that to fail would be if the sizes > could not be detected, in which case a NULL seems like a reasonable way to express that.

If I recall the idea was to protect against the situation where the cols for size were declared not null but the user uploaded a non-image file. In the latter case, the size would be indeterminate and I did not want the code returning undef. So '|| 0' returns 0. -- Ron Savage ron@savage.net.au http://savage.net.au/index.html

Mon Apr 06 09:38:44 2009 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Mon, 6 Apr 2009 09:37:59 -0400
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

Show quoted text

> > If you are only uploading images, doesn't it work to just set the data model to > > be "not null"? The only reason I could see for that to fail would be if the sizes > > could not be detected, in which case a NULL seems like a reasonable way to express that.

> > If I recall the idea was to protect against the situation where the cols > for size were declared not null but the user uploaded a non-image file. > > In the latter case, the size would be indeterminate and I did not want > the code returning undef. So '|| 0' returns 0.

But at that point you are declaring the columns to be "not null", but then creating a workaround to put null values in it, represented as zero. I think it's cleaner just to advise removing the constraint in that case. If people want to declare "not null" and they actually commit to not trying to put NULL values in it, then no changes are needed to the code, and "NOT NULL" can be declared in the database. Mark

Mon Apr 06 17:45:31 2009 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Tue, 07 Apr 2009 07:45:56 +1000
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Mark On Mon, 2009-04-06 at 09:38 -0400, mark@summersault.com via RT wrote: Show quoted text

> <URL: http://rt.cpan.org/Ticket/Display.html?id=40910 > >

> > > If you are only uploading images, doesn't it work to just set the data model to > > > be "not null"? The only reason I could see for that to fail would be if the sizes > > > could not be detected, in which case a NULL seems like a reasonable way to express that.

> > > > If I recall the idea was to protect against the situation where the cols > > for size were declared not null but the user uploaded a non-image file. > > > > In the latter case, the size would be indeterminate and I did not want > > the code returning undef. So '|| 0' returns 0.

> > But at that point you are declaring the columns to be "not null", but > then creating a workaround to put null values in it, represented as > zero. > > I think it's cleaner just to advise removing the constraint in that > case. > > If people want to declare "not null" and they actually commit to not > trying to put NULL values in it, then no changes are needed to the code, > and "NOT NULL" can be declared in the database.

OK. Leave the code as it is. -- Ron Savage ron@savage.net.au http://savage.net.au/index.html

Tue Apr 07 05:53:28 2009 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Tue, 07 Apr 2009 19:53:53 +1000
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Mark Just for the record... On Mon, 2009-04-06 at 17:45 -0400, ron@savage.net.au via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=40910 > > > Hi Mark > > On Mon, 2009-04-06 at 09:38 -0400, mark@summersault.com via RT wrote:

> > <URL: http://rt.cpan.org/Ticket/Display.html?id=40910 > > >

> > > > If you are only uploading images, doesn't it work to just set the data model to > > > > be "not null"? The only reason I could see for that to fail would be if the sizes > > > > could not be detected, in which case a NULL seems like a reasonable way to express that.

> > > > > > If I recall the idea was to protect against the situation where the cols > > > for size were declared not null but the user uploaded a non-image file. > > > > > > In the latter case, the size would be indeterminate and I did not want > > > the code returning undef. So '|| 0' returns 0.

> > > > But at that point you are declaring the columns to be "not null", but > > then creating a workaround to put null values in it, represented as > > zero.

Of course I was. This advice comes from Joe Celko's SQL For Smarties, p 120, where he advocates using not null on /all/ columns wherever possible. That's why you're wrong on this one. But I give up. -- Ron Savage ron@savage.net.au http://savage.net.au/index.html

Tue Apr 14 20:40:11 2009 MARKSTOS [...] cpan.org - Correspondence added

Show quoted text

> This advice comes from Joe Celko's SQL For Smarties, p 120, where he > advocates using not null on /all/ columns wherever possible.

I read this book as well, also now try to make maximum use of "not null". However, the debate here has become about the appropriateness of using other values to represent null so that a column can be declared "not null". This seems like a philosophical difference which may not be resolved. But, hey, couldn't you define a column like this: my_col INT NOT NULL DEFAULT 0 This represents clearly that if the column would be null you are substituting another value instead. Either way, based on our conversation so far, I'm marking this as 'resolved'. Mark

Tue Apr 14 20:40:15 2009 MARKSTOS [...] cpan.org - Status changed from 'open' to 'resolved'

Tue Apr 14 20:40:46 2009 MARKSTOS [...] cpan.org - Reference by ticket #40909 added

Wed Apr 15 03:48:14 2009 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #40910] Columns width and height forced to accept nulls
Date:	Wed, 15 Apr 2009 17:49:14 +1000
To:	bug-CGI-Uploader [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Mark On Tue, 2009-04-14 at 20:40 -0400, MARKSTOS via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=40910 > > >

> > This advice comes from Joe Celko's SQL For Smarties, p 120, where he > > advocates using not null on /all/ columns wherever possible.

> > I read this book as well, also now try to make maximum use of "not > null". However, the debate here has become about the appropriateness of > using other values to represent null so that a column can be declared > "not null". This seems like a philosophical difference which may not be > resolved. > > But, hey, couldn't you define a column like this: > > my_col INT NOT NULL DEFAULT 0 > > This represents clearly that if the column would be null you are > substituting another value instead.

Yes. I was just worried you didn't get my point. But that solves it. Show quoted text

> Either way, based on our conversation so far, I'm marking this as > 'resolved'.

Understood. -- Ron Savage ron@savage.net.au http://savage.net.au/index.html

Wed Apr 15 03:48:15 2009 The RT System itself - Status changed from 'resolved' to 'open'