[Dbix-class] DBIx::Class::ResultSet / resolve_prefetch() / Use of uninitialized value in pattern match

[Matt S Trout]

On Wed, Jul 30, 2008 at 11:17:59AM +0100, Chisel Wright wrote:
> On Wed, Jul 30, 2008 at 10:52:49AM +0100, Matt S Trout wrote:
> > Right, but is it a bug that we don't make undef alias become '' or is
> > it a bug that we don't die on it?
> > 
> > Assuming there's nothing using it in undef mode I think for the moment we
> > should just die() when not given an alias.
> > 
> > It's maybe arguable that you should have '' mean "don't add an alias at all"
> > but DBIC will never need that feature so I'd rather we left it for the moment.
> 
> I'm not entirely sure what _should_ happen.
> 
> As far as I can tell, there's no obvious reason why
> 
>   my @columns = $source->resolve_prefetch( { cd => 'artist' } );
> 
> should spew warnings.
> 
> To me it looks like a harmless call to make. I'm not sure what "alias"
> is, or where is should be set, and what to.

alias is the prefetch nest, so me.cd, me.cd.artist, etc., I think.
 
> This line is the one that has problems when $alias is undef:
> 
>   my $as_prefix = ($alias =~ /^.*?\.(.+)$/ ? $1.'.' : '');
> 
> To me it seems that $alias is allowed to be nothing, but wasn't checked
> for defined-ness before using in a m//

Hmm. No, I don't think it is. It's either 'me' or 'me.artist' or whatever.

I suspect you could make it die() on undef -or- '' and the tests would
still pass.

> The overall behaviour with/without my patch is the same, you just don't
> get logs full of "unitialised value" warnings with it.
> 
> Obviously as author and maintainer, it's your call, but $alias is
> undefined in some cases.

I've never seen a case where it is - do you have "normal" code that causes
that warning?

If not I think I'm in favour of throwing an exception for the moment, since
I really don't think it makes sense otherwise.

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director                    http://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/            http://www.shadowcat.co.uk/servers/