-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The slice-to-hash-with-rename feature is incomplete #143
Comments
Sat Feb 16 16:41:14 2013 TIMB [...] cpan.org - Correspondence added A small clear test case would be appreciated. Sat Feb 16 16:41:16 2013 The RT System itself - Status changed from 'new' to 'open' |
Sat Feb 16 17:04:34 2013 TIMB [...] cpan.org - Correspondence added On Sat Feb 16 13:40:13 2013, RIBASUSHI wrote:
It would be very expensive for fetchrow_* to consider such logic. I presume that you're calling it Perhaps what's needed here is for some kind of bind_columns-like call to setup binding to your |
Sat Feb 16 17:23:38 2013 TIMB [...] cpan.org - Correspondence added From the bind_columns docs: : For compatibility with old scripts, the first parameter will be Perhaps we can make use of that old attribute hash and simplify the code in that "fancy Suggestions would be welcome. Patches would be delightful :) |
Sun Feb 17 03:24:19 2013 ribasushi [...] leporine.io - Correspondence added Date: Sun, 17 Feb 2013 19:24:00 +1100 On Sat, Feb 16, 2013 at 05:04:35PM -0500, Tim_Bunce via RT wrote:
Um... no... many many many times on the same handle. That is: my $rs_returning_1mil_rows = $schema->resultset('Foo')->search(...); Yes of course using ->all is more efficient in terms of work done by the where $rows is a bunch of arrayrefs (obtained either via DBI's fetchall_X Currently I can modify this code only for the case of $rs->all, which will
Right, except I do not want to do that (the binding), because I need to
I would love to write some tests, but I have to admit I am not sure what |
Sun Feb 17 04:05:44 2013 h.m.brand [...] xs4all.nl - Correspondence added Date: Sun, 17 Feb 2013 10:05:24 +0100 On Sat, 16 Feb 2013 17:23:39 -0500, "Tim_Bunce via RT"
I don't see how that "fancy example" can be simplified without losing Other than that I might be missing the point completely, and would like
-- |
Sun Feb 17 05:10:47 2013 ribasushi [...] leporine.io - Correspondence added Date: Sun, 17 Feb 2013 21:10:34 +1100 On Sun, Feb 17, 2013 at 04:05:45AM -0500, [email protected] via RT wrote:
Right, good point. I should start from the /achieve. As stated in an where $rows is a bunch of arrayrefs (obtained either via DBI's Currently given what DBI offers me I can delegate the work only for Also note that it is ver possible this entire ticket is moot - it is |
Sun Feb 17 07:51:57 2013 Tim.Bunce [...] pobox.com - Correspondence added Date: Sun, 17 Feb 2013 12:51:39 +0000 At the core of DBI fetching is the idea of an array of field buffers That's simple and fast because it reuses the same SVs each time. What bind_col(umns) does is simply change the pointers in the fbav array This already works (and has for years):
What's needed beyond that? Can't you just ignore what fetch*() returns and 'clone' the structure you bound
The way I see it, there's no need for an arrayref->hashref transition. Tim. |
Sun Feb 17 07:54:55 2013 ribasushi [...] leporine.io - Correspondence added On Sun Feb 17 07:51:57 2013, [email protected] wrote:
As far as I know cloning (especially dclone()) is extremely slow. I will |
Sun Feb 17 12:00:55 2013 Tim.Bunce [...] pobox.com - Correspondence added Date: Sun, 17 Feb 2013 17:00:38 +0000
dclone is too generic. For example, it takes time to construct a Their ought to be a more limited, and therefore faster, clone()-like It would be interesting to see benchmarks of "simple cloners cloning If all else fails, it wouldn't be too hard to implement such a simple Tim. |
Sun Feb 17 13:21:46 2013 TIMB [...] cpan.org - Correspondence added
Well, Sereal should but JSON::XS probably wouldn't. Still, the basic point stands. There are also worth a look and comparing: |
Mon Feb 18 12:05:19 2013 ribasushi [...] leporine.io - Correspondence added On Sun Feb 17 13:21:46 2013, TIMB wrote:
You will be very very surprised then. Attached is the benchmark script I will do a DBI-based benchmark later. Although given that even a plain
|
Mon Feb 18 13:21:41 2013 smueller [...] cpan.org - Correspondence added On Mon Feb 18 12:05:19 2013, RIBASUSHI wrote:
Things like Sereal and Storable do a lot more work that you want here. A a) ALWAYS use the OO interface if you care about performance. This makes b) The larger the structure, the better Sereal will look. It has c) Due to the nature of the data, it seems like you'll never care about But in the end, even for the OO interface, the best you can hope out of https://github.com/Sereal/Sereal/wiki/Sereal-Comparison-Graphs You can see that JSON::XS beats Sereal on an empty hash here but Sereal A dedicated "build a copy of this data structure" approach to cloning --Steffen |
Mon Feb 18 14:46:35 2013 smueller [...] cpan.org - Correspondence added On Mon Feb 18 13:21:41 2013, SMUELLER wrote:
With the above changes (and just running dumbbench slightly less long),
That is very much in line of what I'd expect. If you use bigger data Nonetheless, my comment about serialization vs. cloning holds. Maybe |
Transcribed verbatim from CPAN RT#83378, warts and all.
Sat Feb 16 13:40:13 2013 ribasushi [...] leporine.io - Ticket created
Subject: The slice-to-hash-with-rename feature is incomplete
Changes in DBIC 0.08240 finally allow me to take advantage of the new
feature that laned in DBI 1.620 (
https://metacpan.org/source/TIMB/DBI-1.623/DBI.pm#L6437 ). However I
realized that the feature is incomplete - it is only recognized by the
eager fetchall_arrayref. Neither fetchrow_arrayref nor fetchrow_array
recognize a $slice argument. Therefore I can not use it in DBIC where it
is needed the most - in the case of cursor iteration.
Is this a "brainfart" type omission, or are there underlying design
considerations which make column renaming infeasible in the case of
row-based iterators?
The text was updated successfully, but these errors were encountered: