Skip to content

Commit

Permalink
Ensure the custom rel cond resolver does not trigger forgotten compat…
Browse files Browse the repository at this point in the history
… shim

During the rush to get custom rels out the door (this is why rushing
fucking sucks), a697fa3 introduced a shortsighted workaround into
::SQLMaker::_from_chunk_to_sql(). This code slipped consequent review
and made its way into the codebase... 4 FUCKING YEARS AGO!!! >:(

Since it is not known how much stuff relies on the insanity being there
(moreover we have tests that rely on it) leave things as is for the time
being. The only change is  making the cond resolver *completely* oblivious to
the "single-element hash" workaround (albeit via a silly hack).

In the process exposed that ora-joins module is entirely incapable
of understanding non-equality conds... fml

See next commits for added warnings, etc.
  • Loading branch information
ribasushi committed Sep 15, 2014
1 parent 21621fe commit c200d94
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 1 deletion.
20 changes: 19 additions & 1 deletion lib/DBIx/Class/ResultSource.pm
Original file line number Diff line number Diff line change
Expand Up @@ -2163,10 +2163,28 @@ sub _resolve_relationship_condition {
;
}
}
elsif (
$col_eqs->{$lhs} =~ /^ ( \Q$args->{self_alias}\E \. .+ ) /x
and
($colinfos->{$1}||{})->{-result_source} == $rel_rsrc
) {
my ($lcol, $rcol) = map
{ $colinfos->{$_}{-colname} }
( $lhs, $1 )
;
carp_unique(
"The $exception_rel_id specifies equality of column '$lcol' and the "
. "*VALUE* '$rcol' (you did not use the { -ident => ... } operator)"
);
}
}
}

$ret
# FIXME - temporary, to fool the idiotic check in SQLMaker::_join_condition
$ret->{condition} = { -and => [ $ret->{condition} ] }
unless $ret->{condition} eq UNRESOLVABLE_CONDITION;

$ret;
}

=head2 related_source
Expand Down
13 changes: 13 additions & 0 deletions lib/DBIx/Class/SQLMaker/OracleJoins.pm
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ sub _recurse_oracle_joins {
&& $jt !~ /inner/i;
}

# FIXME - the code below *UTTERLY* doesn't work with custom conds... sigh
# for the time being do not do any processing with the likes of _collapse_cond
# instead only unroll the -and hack if present
$on = $on->{-and}[0] if (
ref $on eq 'HASH'
and
keys %$on == 1
and
ref $on->{-and} eq 'ARRAY'
and
@{$on->{-and}} == 1
);

# sadly SQLA treats where($scalar) as literal, so we need to jump some hoops
push @where, map { \sprintf ('%s%s = %s%s',
ref $_ ? $self->_recurse_where($_) : $self->_quote($_),
Expand Down
14 changes: 14 additions & 0 deletions t/lib/DBICTest/Schema/Track.pm
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,20 @@ __PACKAGE__->has_many (
}
);

__PACKAGE__->has_many (
deliberately_broken_all_cd_tracks => __PACKAGE__,
sub {
# This is for test purposes only. A regular user does not
# need to sanity check the passed-in arguments, this is what
# the tests are for :)
my $args = &check_customcond_args;

return {
"$args->{foreign_alias}.cd" => "$args->{self_alias}.cd"
};
}
);

our $hook_cb;

sub sqlt_deploy_hook {
Expand Down
23 changes: 23 additions & 0 deletions t/relationship/custom.t
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use warnings;

use Test::More;
use Test::Exception;
use Test::Warn;
use lib qw(t/lib);
use DBICTest ':DiffSQL';

Expand Down Expand Up @@ -297,6 +298,28 @@ is_deeply
'Proper find with related via coderef cond',
;

warnings_exist {
is_same_sql_bind(
$single_track->deliberately_broken_all_cd_tracks->as_query,
'(
SELECT me.trackid, me.cd, me.position, me.title, me.last_updated_on, me.last_updated_at
FROM track track__row
JOIN track me
ON me.cd = ?
WHERE track__row.trackid = ?
)',
[
[{ dbic_colname => "me.cd", sqlt_datatype => "integer" }
=> "track__row.cd" ],
[{ dbic_colname => "track__row.trackid", sqlt_datatype => "integer" }
=> 19 ],
],
'Expected nonsensical JOIN cond',
),
} qr/\Qrelationship 'deliberately_broken_all_cd_tracks' on source 'Track' specifies equality of column 'cd' and the *VALUE* 'cd' (you did not use the { -ident => ... } operator)/,
'Warning on 99.9999% malformed custom cond'
;

$single_track->set_from_related( cd_cref_cond => undef );
ok $single_track->is_column_changed('cd');
is $single_track->get_column('cd'), undef, 'UNset from related via coderef cond';
Expand Down

0 comments on commit c200d94

Please sign in to comment.