Skip Menu |
 

This queue is for tickets about the DBIx-Class CPAN distribution.

Report information
The Basics
Id: 111159
Status: rejected
Priority: 0/
Queue: DBIx-Class

People
Owner: Nobody in particular
Requestors: rrwo [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: [WISHLIST] Use Type::Tiny types for specifying columns
Date: Wed, 13 Jan 2016 17:49:05 +0000
To: bug-DBIx-Class [...] rt.cpan.org
From: Robert Rothenberg <rrwo [...] cpan.org>
Download (untitled) / with headers
text/plain 544b
It would be nice to be able to do something like __PACKAGE__->add_column( foo => Maybe[ Varchar[16] ] ); or __PACKAGE__->add_column( foo => { isa => Maybe[ Varchar[16] ], extra => { ... } } ); This is actually not hard to do. I have some (prototype) code in the Types::SQL distribution that does this. It makes the code easier to read, especially for developers who are already using Moo/Moose for other parts of the code. (And object attributes that depend on database columns can be given the same types.)
Subject: Re: [rt.cpan.org #111159] [WISHLIST] Use Type::Tiny types for specifying columns
Date: Wed, 13 Jan 2016 19:02:56 +0100
To: bug-DBIx-Class [...] rt.cpan.org
From: Peter Rabbitson <ribasushi [...] cpan.org>
Download (untitled) / with headers
text/plain 1.7k
On 01/13/2016 06:49 PM, Robert Rothenberg via RT wrote: Show quoted text
> Wed Jan 13 12:49:17 2016: Request 111159 was acted upon. > Transaction: Ticket created by RRWO > Queue: DBIx-Class > Subject: [WISHLIST] Use Type::Tiny types for specifying columns > Broken in: (no value) > Severity: (no value) > Owner: Nobody > Requestors: rrwo@cpan.org > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=111159 > > > > It would be nice to be able to do something like > > __PACKAGE__->add_column( foo => Maybe[ Varchar[16] ] ); > > or > > __PACKAGE__->add_column( > foo => { > isa => Maybe[ Varchar[16] ], > extra => { ... } > } > ); > > This is actually not hard to do. I have some (prototype) code in the > Types::SQL distribution that does this. > > It makes the code easier to read, especially for developers who are already > using Moo/Moose for other parts of the code. (And object attributes that > depend on database columns can be given the same types.) >
I have not looked at Types::SQL, so what follows is just a general observation. To stop the tendency of DBIC itself is becoming overly-monolithic, the general direction of such enhancements these days is to push them out of the core distrubution, and have them as standalone extra-sugar, akin to DBIx::Class::Candy. Is there a reason this can not be done in the same manner for the above (or even as a subset of ::Candy-like syntax)? Note - I have not thought through the entire proposal as there is little code to go with it (and Types::SQL does things "in the other direction" so it's hard to reason about it from there). So please elaborate more / point out what am I missing. Show quoted text
> > > >
Subject: Re: [rt.cpan.org #111159] [WISHLIST] Use Type::Tiny types for specifying columns
Date: Thu, 14 Jan 2016 12:26:52 +0000
To: bug-DBIx-Class [...] rt.cpan.org
From: Robert Rothenberg <rrwo [...] cpan.org>
Download (untitled) / with headers
text/plain 2.6k
On 13/01/16 18:03, Peter Rabbitson via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=111159 > > > On 01/13/2016 06:49 PM, Robert Rothenberg via RT wrote:
>> Wed Jan 13 12:49:17 2016: Request 111159 was acted upon. >> Transaction: Ticket created by RRWO >> Queue: DBIx-Class >> Subject: [WISHLIST] Use Type::Tiny types for specifying columns >> Broken in: (no value) >> Severity: (no value) >> Owner: Nobody >> Requestors: rrwo@cpan.org >> Status: new >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=111159 > >> >> >> It would be nice to be able to do something like >> >> __PACKAGE__->add_column( foo => Maybe[ Varchar[16] ] ); >> >> or >> >> __PACKAGE__->add_column( >> foo => { >> isa => Maybe[ Varchar[16] ], >> extra => { ... } >> } >> ); >> >> This is actually not hard to do. I have some (prototype) code in the >> Types::SQL distribution that does this. >> >> It makes the code easier to read, especially for developers who are already >> using Moo/Moose for other parts of the code. (And object attributes that >> depend on database columns can be given the same types.) >>
> > I have not looked at Types::SQL, so what follows is just a general > observation. > > To stop the tendency of DBIC itself is becoming overly-monolithic, the > general direction of such enhancements these days is to push them out of > the core distrubution, and have them as standalone extra-sugar, akin to > DBIx::Class::Candy. Is there a reason this can not be done in the same > manner for the above (or even as a subset of ::Candy-like syntax)?
Fair point. I've also been in touch w/author of DBIC::Candy as well about the idea. Show quoted text
> Note - I have not thought through the entire proposal as there is little > code to go with it (and Types::SQL does things "in the other direction" > so it's hard to reason about it from there). So please elaborate more / > point out what am I missing.
Well, given a type like Maybe[Varchar[16]] we can extract the info for the column definition: data_type => 'varchar', is_nullable => 1, size => 16 The code in Types::SQL::Util https://github.com/robrwo/Types-SQL/blob/master/lib/Types/SQL/Util.pm has a function to generate the column information from Type::Tiny objects (not just Types::SQL but also Str, Enum, Int). Something like use Types::SQL::Util; ... # Where $info is the column_info if ((blessed $info) && $info->isa('Type::Tiny')) { $info = { isa => $info }; } if (my $type = $info->{isa}) { my %t_info = column_info_from_type( $type ); $info->{$_} //= $t_info{$_} for keys %t_info; }
Download (untitled) / with headers
text/plain 1.4k
On Thu Jan 14 13:27:04 2016, RRWO wrote: Show quoted text
> > Well, given a type like Maybe[Varchar[16]] we can extract the info for > the > column definition: > > data_type => 'varchar', > is_nullable => 1, > size => 16
I looked more into this, and think it is a pretty bad idea as far as usability is concerned. Users should not care about 'Varchar' at all (in fact it is looked down upon in the Pg world now[1]). What users care about is "I need to store that many ascii or unicode characters. Similarly Users only care about "at least being able to fit uint_24" without really caring if the underlying storage is unsigned, or is exactly 3 bytes long. The point I am trying to make is that Types::SQL is decidedly abstracting at the wrong level. Currently the mangling of "varchar" into something the target DDL will find palatable is done by SQL::Translator with a really terrible mapping between each and every producer. Something like Types::SQL is supposed to make this easier on an architectural level (with a proper hierarchy of types, proper dialects (e.g. Varchar => Text) etc), instead of just adding minimal sugar. Thus the module is not suitable for further DBIC integration even as an external optional component. Furthermore I'd recommend DBIx::Class::Candy avoiding integration as well. We can do much better. I am closing this ticket in the queue, and am bringing it to the attention of the ::Candy maint. Cheers! [1] http://blog.jonanin.com/2013/11/20/postgresql-char-varchar/


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.