Fwd: [Dbix-class] Re: as_subselect_rs->delete bug?
fREW Schmidt
frioux at gmail.com
Wed Jul 11 17:23:24 GMT 2012
Hey all,
So after having a conversation with rob regarding as_subselect_rs, I think
we should probably make ->delete's and ->update's from RS's like this die,
at least by default. The reason being that ->as_subselect_rs makes the rs
be based on a non-introspectible sql string, a runtime view basically, and
writing to those could cause unpredictable results. We've had similar
situations in DBIx::Class when using SQL
Server<http://search.cpan.org/%7Efrew/DBIx-Class-0.08198/lib/DBIx/Class/Sto=
rage/DBI/MSSQL.pm#Ordered_Subselects>.
Here's a contrived footgun:
$schema->resultset('Artist')->search(undef, { +columns =3D> { id =3D> \'=
2 as
id' } })->as_subselect_rs->search({ id =3D> 2 })->delete;
For posterity, here is an off list conversation Rob and I had:
.:10:30:58:. <robkinyon> ok
> .:10:31:18:. frew yo
> .:10:31:31:. frew "I don't think ->as_subselect_rs() shouldn't allow
> deletes."
> .:10:31:34:. <robkinyon> i don't
> .:10:31:36:. frew was that double negative on purpose?
> .:10:31:39:. <robkinyon> no
> .:10:31:42:. <robkinyon> i don't think it should
> .:10:31:44:. frew heh, ok, didn't think so
> .:10:31:52:. frew why not?
> .:10:32:57:. <robkinyon> as_subselect_rs() basically transforms SELECT x,y
> FROM table1 JOIN table2 into SELECT x,y FROM (SELECT x,y FROM
> table1 JOIN table2) AS me
> .:10:32:58:. <robkinyon> right?
> .:10:33:15:. frew yep
> .:10:33:32:. <robkinyon> if we have $rs which is SELECT x,y FROM table1
> .:10:33:42:. <robkinyon> $rs->delete on that becomes DELETE FROM table1
> .:10:33:43:. <robkinyon> right?
> .:10:33:51:. frew right.
> .:10:34:08:. <robkinyon> if it's SELECT =85 FROM table1 WHERE a=3D?, then=
it
> becomes DELETE FROM table1 WHERE a=3D?
> .:10:34:15:. <robkinyon> (please just walk down the garden path with me)
> .:10:34:22:. frew I am walking down said garden path
> .:10:35:00:. <robkinyon> SELECT =85 FROM t1 AS me JOIN t2 WHERE t1.a=3Dt2=
.b
> AND t1.c =3D ? =97> DELETE me.* FROM t1 AS me JOIN t2 WHERE ...
> .:10:35:01:. <robkinyon> right?
> .:10:35:05:. frew well
> .:10:35:08:. frew that's hopeful actually
> .:10:35:19:. <robkinyon> the me.* part should be there in all cases,
> actually
> .:10:35:24:. frew SQL Server at the very least has to use a subquery to
> use JOIN's in deletes
> .:10:35:26:. <robkinyon> i was eliding because i didn't want to type so
> much
> .:10:35:57:. <robkinyon> in the where clause in a way that would use
> ->as_query() ?
> .:10:36:11:. frew yes, basically
> .:10:36:13:. <robkinyon> ok
> .:10:36:19:. frew not by hand of coures, DBIC does it for you
> .:10:36:23:. <robkinyon> of course
> .:10:36:32:. <robkinyon> now, let's talk about what ->as_subselect_rs()
> does
> .:10:36:35:. frew k
> .:10:36:50:. <robkinyon> we have SELECT =85 FROM (SELECT =85 FROM =85) AS=
me
> .:10:36:54:. frew right.
> .:10:37:02:. <robkinyon> that becomes DELETE FROM (SELECT =85 FROM =85) A=
S me
> .:10:37:05:. frew (ignoring your obvioius mysqlisms :)
> .:10:37:15:. <robkinyon> you're trying to delete into an on-demand view
> .:10:37:23:. frew I'd say no
> .:10:37:33:. frew I'd say it should do waht delete does for complex
> selects right now
> .:10:37:39:. <robkinyon> remember - views and from-clause subqueries are
> identical
> .:10:37:47:. <robkinyon> a view is just a pre-defined from-clause subquery
> .:10:37:56:. frew sure
> .:10:38:07:. <robkinyon> and, as such, is meant to be non-introspectable
> .:10:38:16:. frew sure.
> .:10:38:28:. <robkinyon> in the relational algebra, you can consider
> from-clause subqueries to be read-only
> .:10:38:38:. <robkinyon> they should not be unrolable
> .:10:38:50:. frew right, agreed
> .:10:39:05:. frew hm.
> .:10:39:09:. <robkinyon> so, as_subselect_rs means the $rs is now readonly
> .:10:39:16:. <robkinyon> so, no ->delete, ->update, or anything else like
> that
> .:10:39:38:. <robkinyon> so, while i had the wrong reason, the conclusion
> (i think) is still correct
> .:10:39:47:. frew well...
> .:10:39:50:. frew ok
> .:10:39:50:. frew so
> .:10:39:59:. frew I agree with you due to the implementation of
> as_subselect_rs
> .:10:40:12:. <robkinyon> it's not an implementation question - it's a hard
> fact of the relational algebra. :)
> .:10:40:21:. frew if as_subselect_rs could seal away joins *without* doing
> this subquery, I'd argue against you
> .:10:40:39:. <robkinyon> and if elephants could climb trees, so many of my
> jokes as a kid would actually be funny
> .:10:40:51:. frew right
> .:10:40:59:. frew you see what I'm saying though right?
> .:10:41:00:. frew like
> .:10:41:04:. <robkinyon> i do see what you're saying
> .:10:41:11:. frew if you want your resultset methods to be ultimately
> useful
> .:10:41:18:. frew and they have joins
> .:10:41:22:. frew you really should be using this thing
> .:10:41:30:. frew well
> .:10:41:32:. frew actually
> .:10:41:33:. <robkinyon> if you could rewrite the the from clause aliases
> so that you didn't require the hammer that is subqueries, then
> ->seal() wouldn't require a read-only RS
> .:10:41:34:. frew take it back
> .:10:41:34:. frew so
> .:10:41:46:. frew someone uses as_subselect_rs
> .:10:41:53:. frew boo-hoo, it's read-only
> .:10:41:53:. <robkinyon> let's rename that right now
> .:10:41:56:. <robkinyon> it's a crappy name
> .:10:42:05:. frew but they need to delete
> .:10:42:07:. <robkinyon> it should really be called
> ->scramble_from_clause_aliases
>
.:10:42:28:. frew $artist_rs->search({ id =3D>
> $subselected_rs->get_column('id')->as_query })->delete
> .:10:42:29:. frew done
> .:10:42:32:. <robkinyon> and if it were to rename the joined table aliases
> to a random string, then you could do it
> .:10:42:40:. <robkinyon> frew
> .:10:42:41:. frew man, I'm not going to be part of this bikeshed again
> .:10:42:48:. <robkinyon> i'm not bikeshedding
> .:10:42:50:. frew no no
> .:10:42:52:. frew I mean the name
> .:10:42:54:. <robkinyon> i'm telling you how to get what you want. :)
> .:10:43:09:. <robkinyon> "as_subselect_rs" encodes implementation into the
> API
> .:10:43:12:. <robkinyon> which is dumb
> .:10:43:17:. frew Oh I know
> .:10:43:21:. frew I didn't like it at the time either
> .:10:43:23:. <robkinyon> and if it's a subselect, then it's readonly,
> period
> .:10:43:26:. frew but I'm sick of arguing about it
> .:10:43:34:. <robkinyon> i wasn't there in the beginning
> .:10:43:42:. frew yeah, it was a whole deal.
> .:10:43:46:. frew anyway
> .:10:43:52:. frew I like your re-alais thing..
> .:10:43:57:. <robkinyon> i'm just the dude who randomly walks into town,
> solves a crime, then wanders off into the sunset
> .:10:44:23:. frew lemme ponder on this
> .:10:44:25:. <robkinyon> first thing first - you need to modify
> as_subselect_rs to return a read-only resultset
> .:10:44:45:. <robkinyon> and expose a read_only =3D> 1 attribute in the
> second hashref of ->search()
> .:10:44:57:. >>> frew smells yaks
> .:10:45:03:. <robkinyon> which converts the resultset_class to be
> DBIx::Class::ResultSet::ReadOnly
>
---------- Forwarded message ----------
From: Rob Kinyon <rob.kinyon at gmail.com>
Date: Wed, Jul 11, 2012 at 9:30 AM
Subject: Re: [Dbix-class] Re: as_subselect_rs->delete bug?
To: fREW Schmidt <frioux at gmail.com>
Ah. I think I already sounded dumb. :) Meh.
That said, I don't think ->as_subselect_rs() shouldn't allow deletes.
Let's talk on IRC.
On Wed, Jul 11, 2012 at 6:50 AM, fREW Schmidt <frioux at gmail.com> wrote:
> Replying to you directly so neither of us sound dumb without confirmation
of
> something dumb happening.
>
> On Wed, Jul 11, 2012 at 8:35 AM, Rob Kinyon <rob.kinyon at gmail.com> wrote:
>>
>> Before we go ahead confirming failures and making fixes, let's step
>> back and discuss exactly what
>> $rs->search(...)->as_subselect_rs->delete should actually do and if it
>> should be something that's allowable. The point behind as_subselect_rs
>> was to allow a resultset to be used as the RHS for a column in a
>> search clause. If $rs->search() could detect that a $rs is the RHS,
>> then ->as_subselect_rs() would never be called by a user. More to the
>> point, the resultset returned by as_subselect_rs, when I designed the
>> subquery thing, was never meant to be used as a general-purpose $rs,
>> but only in specific contexts.
>
>
> Are you sure you aren't thinking of ->as_query ? as_subselect_rs is a
port
> of the as_virtual_view method from the ::ResultSet::VirtualView helper,
> which ijw came up with actually.
>
> as_query is for what you're saying, definitely. as_subselect_rs lets you
do
> this:
>
> $rs->search([
> { 'artist.name' =3D> 'metallica' },
> { 'me.id =3D> 1 },
> ], {
> join =3D> 'artist',
> })->as_subselect_rs->search([
> { 'artist.name' =3D> 'primus' },
> { 'me.id =3D> 2 },
> ], {
> join =3D> 'artist',
> })->all;
>
> It effectively "seals" away the joins so that they don't leak into later
> searches. (hence the original name was &seal)
>
> Anyway, please let me know if one of us is confused and I can send this to
> the list additionally. If my understanding of as_subselect_rs is correct
it
> should be legal for deletes, as it is generally a Good Thing to use in
> resultset methods that contain joins.
>
>
>
> --
> fREW Schmidt
> http://blog.afoolishmanifesto.com
--
Thanks,
Rob Kinyon
-- =
fREW Schmidt
http://blog.afoolishmanifesto.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20120711/3ff=
a121a/attachment.htm
More information about the DBIx-Class
mailing list