From: Peter Rabbitson Date: Sun, 24 Oct 2010 02:29:30 +0000 (+0200) Subject: Switch code/documentation/examples/tests to the new single-arg syntax X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=6c4f4d69a8ec24b5e76bef0eb9c0d837d8e694ab;p=dbsrgits%2FDBIx-Class-Historic.git Switch code/documentation/examples/tests to the new single-arg syntax --- diff --git a/BRANCH b/BRANCH index a462b18..78cb8d5 100644 --- a/BRANCH +++ b/BRANCH @@ -36,7 +36,8 @@ The coderef is expected to return one or two values make $some_row_obj->set_from_related ($another_obj) work, and also to optimise $row_obj->relationship calls, by avoiding a full-join and instead constructing a WHERE clause with the contents of said - hashref + hashref. Naturally the values of this hashref *must be* plain + scalars. (Note - such rel-to-ident resolution is currently possible for *all* DBIC relationships, but can not be guaranteed for all custom rels possible via this syntax. Thus the escape hatch of a *mandatory* @@ -48,8 +49,8 @@ Why the complexity of two RVs - custom rels are generally simplifiable right? This is generally true, except when it isn't. The main issue is that a -coderef is blackbox, and we want to keep it this way for simplicity. -This means that no state is communicate from DBIC to the coderef (except +coderef is a blackbox, and we want to keep it this way for simplicity. +This means that no state is communicated from DBIC to the coderef (except for the optional row-obj), and no state is communicated out when the coderef returns (i.e. you can use this hashref for real joins but not for set_from_related). @@ -62,12 +63,13 @@ a return value for a specific scenario we *have* to do a full join when doing $artist->cds, as this is the only way to evaluate the artist.name condition. For this we need a defined $on_as_where, but a missing $vals_from_related, which will - signal the need to wrap a full query + signal the need to wrap a full query. Also set_from_related will + throw an exception here, as it largely makes no sense. -- Given the same relationship as above, we want - $new_cd->set_from_related($artist) to do the right thing depending - on the name of $artist - the coderef would be tasked to return - { artistid => xxx } or {} depending on the value of $artist->name +- Given a relationship + { 'cd.year' => '2000', 'artist.id' => \'cds.artistid } + we could use the knowledge of the year to pre-fill the correct value + via $artist->create_related('year_2k_cd', {}); What needs to be adjusted (non-exhaustive summary): diff --git a/lib/DBIx/Class/Relationship.pm b/lib/DBIx/Class/Relationship.pm index ff26676..29a7ffc 100644 --- a/lib/DBIx/Class/Relationship.pm +++ b/lib/DBIx/Class/Relationship.pm @@ -105,13 +105,13 @@ L. All helper methods are called similar to the following template: - __PACKAGE__->$method_name('relname', 'Foreign::Class', - \%cond | \@cond | \&conf, \%attrs); + __PACKAGE__->$method_name('relname', 'Foreign::Class', \%cond|\@cond|\&cond?, \%attrs?); Both C and C are optional. Pass C for C if you want to use the default value for it, but still want to set C. -See L for full documentation on definition of the C argument. +See L for full documentation on +definition of the C argument. See L for documentation on the attributes that are allowed in the C argument. @@ -159,7 +159,7 @@ OR =item cond A hashref, arrayref or coderef specifying a custom join expression. For -documentation see L. +more info see L. =back @@ -231,7 +231,7 @@ which can be assigned to relationships as well. =over 4 -=item Arguments: $accessor_name, $related_class, $their_fk_column|\%cond|\@cond\&cond?, \%attrs? +=item Arguments: $accessor_name, $related_class, $their_fk_column|\%cond|\@cond|\&cond?, \%attrs? =back @@ -271,7 +271,7 @@ OR =item cond A hashref, arrayref or coderef specifying a custom join expression. For -documentation see L. +more info see L. =back @@ -392,7 +392,7 @@ OR =item cond A hashref, arrayref or coderef specifying a custom join expression. For -documentation see L. +more info see L. =back @@ -440,7 +440,7 @@ current table allows nulls (i.e., has the C attribute set to a true value), than C will warn about this because it's naughty and you shouldn't do that. The warning will look something like: - "might_have/has_one" must not be on columns with is_nullable set to true (MySchema::SomeClass/key) + "might_have/has_one" must not be on columns with is_nullable set to true (MySchema::SomeClass/key) If you must be naughty, you can suppress the warning by setting C environment variable to a true value. Otherwise, @@ -487,7 +487,7 @@ OR =item cond A hashref, arrayref or coderef specifying a custom join expression. For -documentation see L. +more info see L. =back diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm index 44496c4..1a46d4e 100644 --- a/lib/DBIx/Class/Relationship/Base.pm +++ b/lib/DBIx/Class/Relationship/Base.pm @@ -15,16 +15,16 @@ DBIx::Class::Relationship::Base - Inter-table relationships =head1 SYNOPSIS - __PACKAGE__->add_relationship('spiders', - 'My::DB::Result::Creatures', - sub { - my ( $me_alias, $rel_alias) = @_; - return - { "${rel_alias}.id" => { '=' => \"${me_alias}.id"}, - "${rel_alias}.type" => { '=', "arachnid" }, - }; - - }); + __PACKAGE__->add_relationship( + spiders => 'My::DB::Result::Creatures', + sub { + my $args = shift; + return { + "$args->{foreign_alias}.id" => { -ident => "$args->{self_alias}.id" }, + "$args->{foreign_alias}.type" => 'arachnid' + }; + }, + ); =head1 DESCRIPTION @@ -42,8 +42,8 @@ methods, for predefined ones, look in L. =back - __PACKAGE__->add_relationship('relname', - 'Foreign::Class', + __PACKAGE__->add_relationship('relname', + 'Foreign::Class', $condition, $attrs); Create a custom relationship between one result source and another @@ -51,109 +51,141 @@ source, indicated by its class name. =head3 condition -The condition argument describes the JOIN expression used to connect -the two sources when creating SQL queries. +The condition argument describes the C clause of the C +expression used to connect the two sources when creating SQL queries. To create simple equality joins, supply a hashref containing the remote table column name as the key(s), and the local table column -name as the value(s), for example: +name as the value(s), for example given: - { 'foreign.author_id' => 'self.id' } + My::Schema::Author->has_many( + books => 'My::Schema::Book', + { 'foreign.author_id' => 'self.id' } + ); -will result in the C clause: +A query like: + + $author_rs->search_related('books')->next - author me JOIN book book ON book.author_id = me.id +will result in the following C clause: + + ... FROM author me LEFT JOIN book books ON books.author_id = me.id ... This describes a relationship between the C table and the C table where the C table has a column C containing the ID value of the C. -C and C are psuedo aliases and must be entered +C and C are pseudo aliases and must be entered literally. They will be replaced with the actual correct table alias when the SQL is produced. Similarly: - { - 'foreign.publisher_id' => 'self.publisher_id', - 'foreign.type_id' => 'self.type_id', - } + My::Schema::Book->has_many( + editions => 'My::Schema::Edition', + { + 'foreign.publisher_id' => 'self.publisher_id', + 'foreign.type_id' => 'self.type_id', + } + ); + + ... + + $book_rs->search_related('editions')->next will result in the C clause: - book me JOIN edition edition ON edition.publisher_id = me.publisher_id - AND edition.type_id = me.type_id + ... FROM book me + LEFT JOIN edition editions ON + editions.publisher_id = me.publisher_id + AND editions.type_id = me.type_id ... This describes the relationship from C to C, where the C table refers to a publisher and a type (e.g. "paperback"): As is the default in L, the key-value pairs will be Ced in the result. C can be achieved with an arrayref, for -example: +example a condition like: - [ - { 'foreign.left_itemid' => 'self.id' }, - { 'foreign.right_itemid' => 'self.id' }, - ] + My::Schema::Item->has_many( + related_item_links => My::Schema::Item::Links, + [ + { 'foreign.left_itemid' => 'self.id' }, + { 'foreign.right_itemid' => 'self.id' }, + ], + ); -which results in the C clause: +will translate to the following C clause: - items me JOIN related_items rel_link ON rel_link.left_itemid = me.id - OR rel_link.right_itemid = me.id + ... FROM item me JOIN item_relations related_item_links ON + related_item_links.left_itemid = me.id + OR related_item_links.right_itemid = me.id ... -This describes the relationship from C to C, -where C is a many-to-many linking table, linking Items -back to themselves. +This describes the relationship from C to C, where +C is a many-to-many linking table, linking items back to +themselves in a peer fashion (without a "parent-child" designation) -To create joins which describe more than a simple equality of column -values, the custom join condition coderef syntax can be used: +To specify joins which describe more than a simple equality of column +values, the custom join condition coderef syntax can be used. For +example: + My::Schema::Artist->has_many( + cds_80s => 'My::Schema::CD', sub { - my ( $me_alias, $rel_alias ) = @_; - return - ({ "${rel_alias}.artist" => { '=' => \"${me_alias}.artistid"}, - "${rel_alias}.year" => { '>', "1979", - '<', "1990" } - }); - } + my $args = shift; -this will result in the C clause: + return { + "$args->{foreign_alias}.artist" => { -ident => "$args->{self_alias}.artistid" }, + "$args->{foreign_alias}.year" => { '>', "1979", '<', "1990" }, + }; + } + ); - artist me LEFT JOIN cd cds_80s_noopt ON - ( cds_80s_noopt.artist = me.artistid - AND ( cds_80s_noopt.year < ? AND cds_80s_noopt.year > ? ) - ) + ... -with the bind values: + $artist_rs->search_related('cds_80s')->next; - '1990', '1979' +will result in the C clause: -C<$rel_alias> is the equivalent to C in the simple syntax, -and will be replaced by the actual remote table alias in the produced -SQL. Similarly, C<$me_alias> is the equivalent to C and will be -replaced with the local table alias in the SQL. + ... FROM artist me LEFT JOIN cd cds_80s ON + cds_80s.artist = me.artistid + AND cds_80s.year < ? + AND cds_80s.year > ? -The actual syntax returned by the coderef should be valid -L syntax, similar to normal -L conditions. +with the bind values: -To help optimise the SQL produced, a second optional hashref can be -returned to be used when the relationship accessor is called directly -on a Row object: + '1990', '1979' - sub { - my ( $me_alias, $rel_alias, $me_result_source, - $rel_name, $optional_me_object ) = @_; - return - ({ "${rel_alias}.artist" => { '=' => \"${me_alias}.artistid"}, - "${rel_alias}.year" => { '>', "1979", - '<', "1990" } - }, - $optional_me_object && - { "${rel_alias}.artist" => $optional_me_object->artistid, - "${rel_alias}.year" => { '>', "1979", - '<', "1990" } - }); +C<< $args->{foreign_alias} >> and C<< $args->{self_alias} >> are supplied the +same values that would be otherwise substituted for C and C +in the simple hashref syntax case. + +The coderef is expected to return a valid L query-structure, just +like what one would supply as the first argument to +L. The return value will be passed directly to +L and the resulting SQL will be used verbatim as the C +clause of the C statement associated with this relationship. + +While every coderef-based condition must return a valid C clause, it may +elect to additionally return a simplified join-free condition hashref when +invoked as C<< $row_object->relationship >>, as opposed to +C<< $rs->related_resultset('relationship') >>. In this case C<$row_object> is +passed to the coderef as C<< $args->{self_rowobj} >>, so a user can do the +following: + + sub { + my $args = shift; + + return ( + { + "$args->{foreign_alias}.artist" => { -ident => "$args->{self_alias}.artistid" }, + "$args->{foreign_alias}.year" => { '>', "1979", '<', "1990" }, + }, + $args->{self_rowobj} && { + "$args->{foreign_alias}.artist" => $args->{self_rowobj}->artistid, + "$args->{foreign_alias}.year" => { '>', "1979", '<', "1990" }, + }, + ); } Now this code: @@ -161,23 +193,38 @@ Now this code: my $artist = $schema->resultset("Artist")->find({ id => 4 }); $artist->cds_80s->all; -Produces: +Can skip a C altogether and instead produce: - SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track - FROM cd me - WHERE ( ( me.artist = ? AND ( me.year < ? AND me.year > ? ) ) ) + SELECT cds_80s.cdid, cds_80s.artist, cds_80s.title, cds_80s.year, cds_80s.genreid, cds_80s.single_track + FROM cd cds_80s + WHERE cds_80s.artist = ? + AND cds_80s.year < ? + AND cds_80s.year > ? With the bind values: '4', '1990', '1979' -The C<$optional_me_object> used to create the second hashref contains -a row object, the object that the relation accessor was called on. - -C<$me_result_source> the L of the table -being searched on, and C<$rel_name>, the name of the relation -containing this condition, are also provided as arguments. These may -be useful to more complicated condition calculation. +Note that in order to be able to use +L<< $row->create_related|DBIx::Class::Relationship::Base/create_related >>, +the coderef must not only return as its second such a "simple" condition +hashref which does not depend on joins being available, but the hashref must +contain only plain values/deflatable objects, such that the result can be +passed directly to L. For +instance the C constraint in the above example prevents the relationship +from being used to to create related objects (an exception will be thrown). + +In order to allow the user to go truly crazy when generating a custom C +clause, the C<$args> hashref passed to the subroutine contains some extra +metadata. Currently the supplied coderef is executed as: + + $relationship_info->{cond}->({ + self_alias => The alias of the invoking resultset ('me' in case of a row object), + foreign_alias => The alias of the to-be-joined resultset (often matches relname), + self_resultsource => The invocant's resultsource, + foreign_relname => The relationship name (does *not* always match foreign_alias), + self_rowobj => The invocant itself in case of $row_obj->relationship + }); =head3 attributes diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm index 3e74860..e5fb395 100644 --- a/lib/DBIx/Class/ResultSource.pm +++ b/lib/DBIx/Class/ResultSource.pm @@ -1568,11 +1568,13 @@ sub _resolve_condition { $self->throw_exception ('Unable to determine relationship name for condition resolution'); } - return $cond->(ref $for ? $as : $for, - ref $for ? 'me' : $as, - $self, - $rel, - ref $for ? $for : undef); + return $cond->({ + self_alias => ref $for ? $as : $for, + foreign_alias => ref $for ? $self->related_source($rel)->resultset->current_source_alias : $as, + self_resultsource => $self, + foreign_relname => $rel, + self_rowobj => ref $for ? $for : undef + }); } elsif (ref $cond eq 'HASH') { my %ret; diff --git a/t/lib/DBICTest/Schema/Artist.pm b/t/lib/DBICTest/Schema/Artist.pm index 0a20128..b8b7911 100644 --- a/t/lib/DBICTest/Schema/Artist.pm +++ b/t/lib/DBICTest/Schema/Artist.pm @@ -47,43 +47,43 @@ __PACKAGE__->has_many( __PACKAGE__->has_many( - cds_80s => 'DBICTest::Schema::CD', - sub { - my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_; - return - ({ "${rel_alias}.artist" => { '=' => \"${me_alias}.artistid"}, - "${rel_alias}.year" => { '>', "1979", - '<', "1990" } - }, - $optional_me_object && - { "${rel_alias}.artist" => $optional_me_object->artistid, - "${rel_alias}.year" => { '>', "1979", - '<', "1990" } - }); - } + cds_80s => 'DBICTest::Schema::CD', + sub { + my $args = shift; + + return ( + { "$args->{foreign_alias}.artist" => { '=' => { -ident => "$args->{self_alias}.artistid"} }, + "$args->{foreign_alias}.year" => { '>' => 1979, '<' => 1990 }, + }, + $args->{self_rowobj} && { + "$args->{foreign_alias}.artist" => $args->{self_rowobj}->artistid, + "$args->{foreign_alias}.year" => { '>' => 1979, '<' => 1990 }, + } + ); + }, ); __PACKAGE__->has_many( - cds_80s_noopt => 'DBICTest::Schema::CD', - sub { - my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_; - return - ({ "${rel_alias}.artist" => { '=' => \"${me_alias}.artistid"}, - "${rel_alias}.year" => { '>', "1979", - '<', "1990" } - }); - } + cds_80s_noopt => 'DBICTest::Schema::CD', + sub { + my $args = shift; + return ( + { "$args->{foreign_alias}.artist" => { -ident => "$args->{self_alias}.artistid" }, + "$args->{foreign_alias}.year" => { '>' => 1979, '<' => 1990 }, + } + ); + }, ); __PACKAGE__->has_many( - cds_90s => 'DBICTest::Schema::CD', - sub { - my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_; - return - ({ "${rel_alias}.artist" => { '=' => \"${me_alias}.artistid"}, - "${rel_alias}.year" => { '>', "1989", - '<', "2000" } - }); + cds_90s => 'DBICTest::Schema::CD', + sub { + my $args = shift; + return ( + { "$args->{foreign_alias}.artist" => { -ident => "$args->{self_alias}.artistid" }, + "$args->{foreign_alias}.year" => { '>' => 1989, '<' => 2000 }, + } + ); } ); diff --git a/t/lib/DBICTest/Schema/Track.pm b/t/lib/DBICTest/Schema/Track.pm index c27ba98..fb9e5c5 100644 --- a/t/lib/DBICTest/Schema/Track.pm +++ b/t/lib/DBICTest/Schema/Track.pm @@ -64,19 +64,19 @@ __PACKAGE__->belongs_to( ); __PACKAGE__->might_have ( - 'next_track', - __PACKAGE__, - sub { - my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_; - return - ({ "${rel_alias}.cd" => { '=', \"${me_alias}.cd" }, - "${rel_alias}.position" => { '>', \"${me_alias}.position" }, - }, - $optional_me_object && - { "${rel_alias}.cd" => $optional_me_object->cd, - "${rel_alias}.position" => { '>', $optional_me_object->position }, - }); - }, + next_track => __PACKAGE__, + sub { + my $args = shift; + return ( + { "$args->{foreign_alias}.cd" => { -ident => "$args->{self_alias}.cd" }, + "$args->{foreign_alias}.position" => { '>' => { -ident => "$args->{self_alias}.position" } }, + }, + $args->{self_rowobj} && { + "$args->{foreign_alias}.cd" => $args->{self_rowobj}->cd, + "$args->{foreign_alias}.position" => { '>' => $args->{self_rowobj}->position }, + } + ) + } ); 1;