Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
branch: master
Commits on Jan 28, 2014
  1. @ribasushi
  2. @ribasushi
  3. @shadowcat-mst @ribasushi
Commits on Jan 27, 2014
  1. @ribasushi

    Skip annoyingly failing test on broken base.pm

    ribasushi authored
    Was broken in 5.19.7, fixed in 5.19.8
  2. @ribasushi

    Yet another attempt to solve the mystery-win32-failure cpantesters re…

    ribasushi authored
    …port
    
    While fa19e5d was a nice idea, its test was flawed. As seen in the fresh fail
    http://www.cpantesters.org/cpan/report/137dfaf3-7bbc-1014-aec2-3d202b825c07
    the writability test passed with flying colors, but the lock didn't open right
    after that. The only explanation at this point is that the FS underlying this
    particular dir can not create filenames starting with a dot <facepalm />
    Alternatively it may have to do with the umask(0) call, but this seems even
    more insane.
    
    Thus (as unlikely as it seems) the culprit seems to be the name. Hopefully
    changing it will be the end of this and we will smoke happily ever after...
  3. @ribasushi
  4. @ribasushi
Commits on Jan 24, 2014
  1. @ribasushi
  2. @ribasushi

    Augment the logic from aca094b, add deprecation to settle the distinc…

    ribasushi authored
    …t drama
    
    Details in attached IRC log
    
    magnet#dbic-cabal_20131108.log
    ====
    [19:01:38] <mst> using aggregates and distinct seems like it should be an error
    [19:02:12] <mst> since it makes no sense to ask for "GROUP BY everything we select" and then select aggregates AFAICS
    [19:03:20] <mst> so, yes, what we did before was ... wtf
    [19:04:44] <ribasushi> mst: the point is *sometimes* a select in a group-by may make sense is my gut feeling, I basically wanted to "send the nonsense" along, and have the rdbms be the judge
    [19:05:52] <mst> well ... what we did before was definitely completely wrong
    [19:06:09] <mst> what we're doing now ... I don't want to release it without thinking it through some more, but it's certainly less wrong
    [19:07:40] <mst> but I mean ... I dunno, I think group_over_selection makes sense for subquery wrapping
    [19:08:04] <mst> but in the case of 'distinct => 1' I think "can't do that with an aggregate in select, pass a real group by you fucking pansy" would be pretty good too
    [19:08:14] <mst> since I'm really not convinced it ever can make sense
    [19:08:27] <ribasushi> mst: and I am not convinced it never makes sense
    [19:08:36] <ribasushi> mst: so bubbling it up doesn't seem like a loss
    [19:09:09] <mst> hmmm
    [19:09:44] <ribasushi> mst: also I do not want to *entirely* discourage mysql/sqlite-centric grouppage - if a person wants to do that, they should be able to
    [19:09:56] <ribasushi> hence why the explicit \'' rule
    [19:10:03] <ribasushi> (that is back from 2009-ish actually)
    [19:10:05] <mst> actually, looking at this
    [19:10:23] <mst> the sql looks like what the user asked for
    [19:10:38] <ribasushi> how so?
    [19:10:59] <ribasushi> distinct is "final selection grouper" not "first selection"
    [19:11:00] <mst> well, they created a resultset of
    [19:11:04] <ribasushi> i.e. it is inherently lazy
    [19:11:29] <ribasushi> right, it does not take effect on the current selection, didn't from before I took over
    [19:11:53] <mst> SELECT year, COUNT(me.cdid) AS cnt ....
    [19:11:57] <mst> and then grabbed the 'cnt' column
    [19:13:06] <mst> ribasushi: ResultSet originally (0.05000) did '$attrs->{group_by} ||= $attrs->{select} if delete $attrs->{distinct};' in new()
    [19:13:15] <mst> so, yes, it takes effect on the current selection
    [19:13:36] <mst> so sql is correct according to the original behaviour as implemented
    [19:13:36] <ribasushi> mst: but it did this in *_resolve_attrs*
    [19:13:55] <ribasushi> which is after the selection is modified by RSETColumn
    [19:14:15] <ribasushi> or to put it diff. - RSetColumn does not do a subquery, just like a ->search doesn't
    [19:14:30] <mst> it didn't originally, it did it immediately you supplied the distinct attribute to a search
    [19:14:40] <mst> anything else is a bug; fix that and we don't need this crap
    [19:14:52] <ribasushi> well - when I came around it was lazified via the late resolver
    [19:15:08] <ribasushi> and I had bugs reported when that behavior changed later on when I was doin prefetches
    [19:15:17] <ribasushi> so this is crap in production for years
    [19:15:41] <ribasushi> mst: https://github.com/dbsrgits/dbix-class/blob/master/t/search/distinct.t behaviors on non-aggregates
    [19:15:55] <ribasushi> it accreted stuff obviously, the ages of tests are different
    [19:17:01] <ribasushi> mst: basically this boils to "is distinct => 1 lazy or eager" and I am afraid this ship sailed around 0.07
    [19:17:17] <ribasushi> "fixing" it will serve nothing but bring breakage
    [19:17:48] <mst> yeah, it looks like it got broken when _resolved_attrs was factored out of new()
    [19:17:53] <ribasushi> nod
    [19:18:28] <ribasushi> so - if we assume "distinct is lazy" - you see why I would go the way I did with the latest fix
    [19:19:22] <mst> ribasushi: distinct is lazy works fine, you just have to disallow aggregates in the select list in that case
    [19:19:30] <mst> and make them do an explicit group_by for that
    [19:20:31] <ribasushi> mst: aggregates via the {} syntax have been supported correctly forever - that is distinct knows to ignore them
    [19:20:45] <ribasushi> mst: this test is more or less "aggregate in literal" (due to being an alias)
    [19:20:47] <mst> even as far as 08000 it was straight up
    [19:20:51] <ribasushi> so I can't disallow it
    [19:20:56] <ribasushi> because it is... a literal
    [19:20:59] <mst> $attrs->{group_by} ||= $attrs->{select} if delete $attrs->{distinct};
    [19:21:11] <mst> so if we started allowing them via the {} syntax that wasn't me
    [19:21:17] <ribasushi> mst: I came around 0.08010 ;)
    [19:22:01] <mst> ribasushi: $attrs->{group_by} ||= $attrs->{select} if delete $attrs->{distinct};
    [19:22:07] <mst> ribasushi: in 08011 which is the first release you did
    [19:22:34] <ribasushi> mst: it is very possibkle that I broke it when I wasn't knowing wtf is on
    [19:22:46] <mst> I think you made lazy distinct work "better"
    [19:22:51] <mst> when it shouldn't've been lazy
    [19:22:59] <ribasushi> mst: in any case - it's old shit, so even if I am responsible it still boils to "people depend on that"
    [19:23:13] <ribasushi> mst: nod, may very well be my rookie mistake ;)
    [19:23:15] <mst> sure. at this point it's "I fucked it up, then you made it worse, we'll have to deprecate it"
    [19:23:19] <mst> most likely
    [19:23:34] <mst> and the only question is damage control so we don't break current usages while isolating the code
    [19:24:48] <ribasushi> deprecating it - I wouldn't go that far, I use distinct a lot myself because it does the group construction for me without me thinking
    [19:25:01] <ribasushi> juggling selection/ordering is hard enough ;)
  3. @ribasushi
  4. @ribasushi

    Shuffle author-side M::I stuff out of the way

    ribasushi authored
    Zero functional changes
  5. @ribasushi
  6. @ribasushi
  7. @ribasushi
  8. @ribasushi
Commits on Jan 23, 2014
  1. @ribasushi
  2. @ribasushi
  3. @ribasushi
  4. @ribasushi

    Move hrefaddr to DBIC::_Util, give most functions a prototype

    ribasushi authored
    This way we can safely do e.g. ( hrefaddr $foo, $unrelated_bar )
  5. @ribasushi

    Improve error reporting when we encounter broken exception objects

    ribasushi authored
    Undo parts of 935ea66 (which inadevrtently broke 153a6b3), while
    keeping the entire shebang running after issuing a stern warning.
  6. @ribasushi
  7. @ribasushi

    GAH! DBD::SQLite's sqlite_db_filename() is relatively new, undo 9e75be9

    ribasushi authored
    On the bright side, re-trapping $db_file now *SEEMS TO WORK*. What the hell?
    Ripping out the dogE-crap of 9e75be9, and trying to forget all of this ever
    happened...
Commits on Jan 22, 2014
  1. @ribasushi
  2. @ribasushi
  3. @ribasushi
  4. @ribasushi
  5. @ribasushi

    Massive incompatible change of ::BlockRunner internals

    ribasushi authored
    It was never documented as usable externally (though folks do use it, sigh)
    This last set of changes settles the design for proper documentation and
    opening up
  6. @ribasushi

    Fix long standing issue with resultset growth on repeated execution (…

    ribasushi authored
    …GHPR#29)
    
    The "save the SQLA argstack" dirty solution introduced in 975b573 actually
    resulted in an infinitely growing nested data structure, containing the
    history of *every* reinvocation of the resultset. Scale back the problematic
    part, though this is still an interim workaround. A DQ-based stack should
    make this much cleaner.
Commits on Jan 21, 2014
  1. @ribasushi

    Stop using precomputed SQLite testdb name, fix test-end bug in replic…

    ribasushi authored
    …ated.t
    
    This was an... odd one. Originally the problem manifested as a classic
    var-sub-sub closure problem, leading to $db_file being captured and tripping
    a more involved testcase that is coming in a subsequent commit. However while
    getting rid of the reference itself was easy, this led to the outer coderef
    itself being "leaked out".
    
    The reproducing oneliner is:
    
    ~$ perl -MScalar::Util=weaken -Mstrict -Mwarnings -e '
     sub foo {
       my $rv = sub { "wtf" }; return $rv;
     }
    
     my $should_be_gone;
    
     {
       $should_be_gone = foo();
       weaken ($should_be_gone);
     }
    
     warn $should_be_gone;  # why is this still defined@!%#$!$#
    '
    
    Various tools indicated that it is somehow attached to the PAD of the static
    foo() sub, but I could not figure out *why* exactly this is happening, nor
    how to properly list it: PadWalker's closed_over() shows nothing for foo()
    
    Thus invoking doge on the whole shebang, and moving on. SUCH WTF, WOW!
    
    Incientally changing things to use the current SQLite filename revealed
    a bug in t/storage/replicated.t, which was never noticed as it only
    resulted in annoying warnings under Win32 global destroy (test
    teardown).
Commits on Jan 20, 2014
  1. @ribasushi

    Add extra operations to the heavy duty leaktrace scope

    ribasushi authored
    Fire all so-far-collected resultsets multiple times
  2. @ribasushi
  3. @ribasushi
  4. @ribasushi
Commits on Jan 19, 2014
  1. @ribasushi
  2. @ribasushi

    Switch to a global symtable "classdata" visitor

    ribasushi authored
    This not only allows us to track anything global, not only CAG stuff
    but also removes a bunch of workarounda from t/52leaks.t \o/
Something went wrong with that request. Please try again.