Skip to content

Commit c200d94

Browse files
committed
Ensure the custom rel cond resolver does not trigger forgotten compat 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.
1 parent 21621fe commit c200d94

File tree

4 files changed

+69
-1
lines changed

4 files changed

+69
-1
lines changed

lib/DBIx/Class/ResultSource.pm

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2163,10 +2163,28 @@ sub _resolve_relationship_condition {
21632163
;
21642164
}
21652165
}
2166+
elsif (
2167+
$col_eqs->{$lhs} =~ /^ ( \Q$args->{self_alias}\E \. .+ ) /x
2168+
and
2169+
($colinfos->{$1}||{})->{-result_source} == $rel_rsrc
2170+
) {
2171+
my ($lcol, $rcol) = map
2172+
{ $colinfos->{$_}{-colname} }
2173+
( $lhs, $1 )
2174+
;
2175+
carp_unique(
2176+
"The $exception_rel_id specifies equality of column '$lcol' and the "
2177+
. "*VALUE* '$rcol' (you did not use the { -ident => ... } operator)"
2178+
);
2179+
}
21662180
}
21672181
}
21682182

2169-
$ret
2183+
# FIXME - temporary, to fool the idiotic check in SQLMaker::_join_condition
2184+
$ret->{condition} = { -and => [ $ret->{condition} ] }
2185+
unless $ret->{condition} eq UNRESOLVABLE_CONDITION;
2186+
2187+
$ret;
21702188
}
21712189

21722190
=head2 related_source

lib/DBIx/Class/SQLMaker/OracleJoins.pm

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,19 @@ sub _recurse_oracle_joins {
8080
&& $jt !~ /inner/i;
8181
}
8282

83+
# FIXME - the code below *UTTERLY* doesn't work with custom conds... sigh
84+
# for the time being do not do any processing with the likes of _collapse_cond
85+
# instead only unroll the -and hack if present
86+
$on = $on->{-and}[0] if (
87+
ref $on eq 'HASH'
88+
and
89+
keys %$on == 1
90+
and
91+
ref $on->{-and} eq 'ARRAY'
92+
and
93+
@{$on->{-and}} == 1
94+
);
95+
8396
# sadly SQLA treats where($scalar) as literal, so we need to jump some hoops
8497
push @where, map { \sprintf ('%s%s = %s%s',
8598
ref $_ ? $self->_recurse_where($_) : $self->_quote($_),

t/lib/DBICTest/Schema/Track.pm

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,20 @@ __PACKAGE__->has_many (
116116
}
117117
);
118118

119+
__PACKAGE__->has_many (
120+
deliberately_broken_all_cd_tracks => __PACKAGE__,
121+
sub {
122+
# This is for test purposes only. A regular user does not
123+
# need to sanity check the passed-in arguments, this is what
124+
# the tests are for :)
125+
my $args = &check_customcond_args;
126+
127+
return {
128+
"$args->{foreign_alias}.cd" => "$args->{self_alias}.cd"
129+
};
130+
}
131+
);
132+
119133
our $hook_cb;
120134

121135
sub sqlt_deploy_hook {

t/relationship/custom.t

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use warnings;
33

44
use Test::More;
55
use Test::Exception;
6+
use Test::Warn;
67
use lib qw(t/lib);
78
use DBICTest ':DiffSQL';
89

@@ -297,6 +298,28 @@ is_deeply
297298
'Proper find with related via coderef cond',
298299
;
299300

301+
warnings_exist {
302+
is_same_sql_bind(
303+
$single_track->deliberately_broken_all_cd_tracks->as_query,
304+
'(
305+
SELECT me.trackid, me.cd, me.position, me.title, me.last_updated_on, me.last_updated_at
306+
FROM track track__row
307+
JOIN track me
308+
ON me.cd = ?
309+
WHERE track__row.trackid = ?
310+
)',
311+
[
312+
[{ dbic_colname => "me.cd", sqlt_datatype => "integer" }
313+
=> "track__row.cd" ],
314+
[{ dbic_colname => "track__row.trackid", sqlt_datatype => "integer" }
315+
=> 19 ],
316+
],
317+
'Expected nonsensical JOIN cond',
318+
),
319+
} 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)/,
320+
'Warning on 99.9999% malformed custom cond'
321+
;
322+
300323
$single_track->set_from_related( cd_cref_cond => undef );
301324
ok $single_track->is_column_changed('cd');
302325
is $single_track->get_column('cd'), undef, 'UNset from related via coderef cond';

0 commit comments

Comments
 (0)