From: Peter Rabbitson Date: Fri, 8 Feb 2013 15:16:04 +0000 (+0100) Subject: Move the infmap verification/exception way earlier X-Git-Tag: v0.08240~7 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=95e410363af05aa8a39ec5febc185e18600f69f8;p=dbsrgits%2FDBIx-Class.git Move the infmap verification/exception way earlier The thing deliberately is not a method on the source, because I am not entirely sure where it belongs *YET*. Just knew it does not belong in the (hopefully) extractable parser-gens. --- diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 13cfa03..4fff77e 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -3269,6 +3269,40 @@ sub _chain_relationship { return {%$attrs, from => $from, seen_join => $seen}; } +# FIXME - this needs to go live in Schema with the tree walker... or +# something +my $inflatemap_checker; +$inflatemap_checker = sub { + my ($rsrc, $relpaths) = @_; + + my $rels; + + for (@$relpaths) { + $_ =~ /^ ( [^\.]+ ) \. (.+) $/x + or next; + + push @{$rels->{$1}}, $2; + } + + for my $rel (keys %$rels) { + my $rel_rsrc = try { + $rsrc->related_source ($rel) + } catch { + $rsrc->throw_exception(sprintf( + "Inflation into non-existent relationship '%s' of '%s' requested, " + . "check the inflation specification (columns/as) ending in '...%s.%s'", + $rel, + $rsrc->source_name, + $rel, + ( sort { length($a) <=> length ($b) } @{$rels->{$rel}} )[0], + ))}; + + $inflatemap_checker->($rel_rsrc, $rels->{$rel}); + } + + return; +}; + sub _resolved_attrs { my $self = shift; return $self->{_attrs} if $self->{_attrs}; @@ -3340,6 +3374,14 @@ sub _resolved_attrs { } } + # validate the user-supplied 'as' chain + # folks get too confused by the (logical) exception message, need to + # go to some lengths to clarify the text + # + # FIXME - this needs to go live in Schema with the tree walker... or + # something + $inflatemap_checker->($source, \@as); + $attrs->{select} = \@sel; $attrs->{as} = \@as; diff --git a/lib/DBIx/Class/ResultSource/RowParser.pm b/lib/DBIx/Class/ResultSource/RowParser.pm index 141037b..56db86f 100644 --- a/lib/DBIx/Class/ResultSource/RowParser.pm +++ b/lib/DBIx/Class/ResultSource/RowParser.pm @@ -94,13 +94,13 @@ sub _resolve_collapse { my $relinfo; # run through relationships, collect metadata for my $rel (keys %$rel_cols) { - my $rel_src = __get_related_source($self, $rel, $rel_cols->{$rel}); - my $inf = $self->relationship_info ($rel); - $relinfo->{$rel}{is_single} = $inf->{attrs}{accessor} && $inf->{attrs}{accessor} ne 'multi'; - $relinfo->{$rel}{is_inner} = ( $inf->{attrs}{join_type} || '' ) !~ /^left/i; - $relinfo->{$rel}{rsrc} = $rel_src; + $relinfo->{$rel} = { + is_single => ( $inf->{attrs}{accessor} && $inf->{attrs}{accessor} ne 'multi' ), + is_inner => ( ( $inf->{attrs}{join_type} || '' ) !~ /^left/i), + rsrc => $self->related_source($rel), + }; # FIME - need to use _resolve_cond here instead my $cond = $inf->{cond}; @@ -403,7 +403,6 @@ sub _mk_row_parser { if (!$args->{collapse}) { $parser_src = sprintf('$_ = %s for @{$_[0]}', __visit_infmap_simple( $inflate_index, - { rsrc => $self }, # need the $rsrc to sanity-check inflation map once )); # change the quoted placeholders to unquoted alias-references @@ -540,7 +539,6 @@ sub __visit_infmap_simple { #$optional ||= ($args->{rsrc}->relationship_info($rel)->{attrs}{join_type} || '') =~ /^left/i; push @relperl, join ' => ', perlstring($rel), __visit_infmap_simple($rel_cols->{$rel}, { - rsrc => __get_related_source($args->{rsrc}, $rel, $rel_cols->{$rel}), # DISABLEPRUNE #non_top => 1, #is_optional => $optional, @@ -664,23 +662,6 @@ sub __unique_numlist { sort { $a <=> $b } keys %{ {map { $_ => 1 } @_ }} } -# This error must be thrown from two distinct codepaths, joining them is -# rather hard. Go for this hack instead. -sub __get_related_source { - my ($rsrc, $rel, $relcols) = @_; - try { - $rsrc->related_source ($rel) - } catch { - $rsrc->throw_exception(sprintf( - "Can't inflate prefetch into non-existent relationship '%s' from '%s', " - . "check the inflation specification (columns/as) ending in '...%s.%s'.", - $rel, - $rsrc->source_name, - $rel, - (sort { length($a) <=> length ($b) } keys %$relcols)[0], - ))}; -} - # keep our own DD object around so we don't have to fitz with quoting my $dumper_obj; sub __visit_dump { diff --git a/t/prefetch/incomplete.t b/t/prefetch/incomplete.t index f8e89de..e753962 100644 --- a/t/prefetch/incomplete.t +++ b/t/prefetch/incomplete.t @@ -105,7 +105,7 @@ throws_ok( sub { $schema->resultset('Track')->search({}, { join => { cd => 'artist' }, '+columns' => 'artist.name' } )->next; }, - qr|\QCan't inflate prefetch into non-existent relationship 'artist' from 'Track', check the inflation specification (columns/as) ending in '...artist.name'|, + qr|\QInflation into non-existent relationship 'artist' of 'Track' requested, check the inflation specification (columns/as) ending in '...artist.name'|, 'Sensible error message on mis-specified "as"', ); diff --git a/t/sqlmaker/limit_dialects/custom.t b/t/sqlmaker/limit_dialects/custom.t index 1bf3e07..c5e61c6 100644 --- a/t/sqlmaker/limit_dialects/custom.t +++ b/t/sqlmaker/limit_dialects/custom.t @@ -32,7 +32,7 @@ my $rs = $s->resultset ('CD'); warnings_exist { is_same_sql_bind ( $rs->search ({}, { rows => 1, offset => 3,columns => [ { id => 'foo.id' }, - { 'bar.id' => 'bar.id' }, + { 'artist.id' => 'bar.id' }, { bleh => \ 'TO_CHAR (foo.womble, "blah")' }, ]})->as_query, '( diff --git a/t/sqlmaker/limit_dialects/rownum.t b/t/sqlmaker/limit_dialects/rownum.t index 9e896fe..b01790f 100644 --- a/t/sqlmaker/limit_dialects/rownum.t +++ b/t/sqlmaker/limit_dialects/rownum.t @@ -19,6 +19,12 @@ $s->storage->sql_maker->limit_dialect ('RowNum'); my $rs = $s->resultset ('CD')->search({ id => 1 }); +# important for a test below, never traversed +$rs->result_source->add_relationship( + ends_with_me => 'DBICTest::Schema::Artist', sub {} +); + + my $where_bind = [ { dbic_colname => 'id' }, 1 ]; for my $test_set ( @@ -29,16 +35,16 @@ for my $test_set ( offset => 3, columns => [ { id => 'foo.id' }, - { 'bar.id' => 'bar.id' }, + { 'artist.id' => 'bar.id' }, { bleh => \'TO_CHAR (foo.womble, "blah")' }, ] }), sql => '( - SELECT id, bar__id, bleh + SELECT id, artist__id, bleh FROM ( - SELECT id, bar__id, bleh, ROWNUM rownum__index + SELECT id, artist__id, bleh, ROWNUM rownum__index FROM ( - SELECT foo.id AS id, bar.id AS bar__id, TO_CHAR (foo.womble, "blah") AS bleh + SELECT foo.id AS id, bar.id AS artist__id, TO_CHAR (foo.womble, "blah") AS bleh FROM cd me WHERE id = ? ) me @@ -56,17 +62,17 @@ for my $test_set ( offset => 3, columns => [ { id => 'foo.id' }, - { 'bar.id' => 'bar.id' }, + { 'artist.id' => 'bar.id' }, { bleh => \'TO_CHAR (foo.womble, "blah")' }, ], order_by => [qw( artist title )], }), sql => '( - SELECT id, bar__id, bleh + SELECT id, artist__id, bleh FROM ( - SELECT id, bar__id, bleh, ROWNUM rownum__index + SELECT id, artist__id, bleh, ROWNUM rownum__index FROM ( - SELECT foo.id AS id, bar.id AS bar__id, TO_CHAR(foo.womble, "blah") AS bleh + SELECT foo.id AS id, bar.id AS artist__id, TO_CHAR(foo.womble, "blah") AS bleh FROM cd me WHERE id = ? ORDER BY artist, title @@ -88,17 +94,17 @@ for my $test_set ( offset => 3, columns => [ { id => 'foo.id' }, - { 'bar.id' => 'bar.id' }, + { 'artist.id' => 'bar.id' }, { bleh => \'TO_CHAR (foo.womble, "blah")' }, ], order_by => 'artist', }), sql => '( - SELECT id, bar__id, bleh + SELECT id, artist__id, bleh FROM ( - SELECT id, bar__id, bleh, ROWNUM rownum__index + SELECT id, artist__id, bleh, ROWNUM rownum__index FROM ( - SELECT foo.id AS id, bar.id AS bar__id, TO_CHAR(foo.womble, "blah") AS bleh + SELECT foo.id AS id, bar.id AS artist__id, TO_CHAR(foo.womble, "blah") AS bleh FROM cd me WHERE id = ? ORDER BY artist