kasei / perlrdf

Deprecated in favor of the Attean package
26 stars 25 forks source link

RDF::Trine::Store::SPARQL::_group_bulk_ops doesn't build array correc… #147

Closed minusdavid closed 7 years ago

minusdavid commented 7 years ago

As per issue #146 on Github, there were issues where RDF::Trine::Store::SPARQL::_group_bulk_ops was pushing additional ops into the element of the first op in the @aggops array.

There was also an issue with RDF::Trine::Store::SPARQL::_end_bulk_ops where the ops were being added to a new array rather than assigned as a reference.

This commit fixes these two assignment problems, and adds some tests to make sure that these fixes work.

TEST PLAN: Within 'perlrdf/RDF-Trine' run the following: perl -I lib xt/store-sparql.t

All tests should pass. If you already have RDF::Trine installed on your system, leave off the -I switch and chances are the tests will fail (at least if you're using v1.016).

kasei commented 7 years ago

Hi @minusdavid. Thanks for sending this in, and apologies for any trouble it caused you. I'll admit that RDF::Trine::Store::SPARQL is probably one of the least used/tested pieces of code in RDF::Trine. I'm pretty busy the next few days, but I'll try my best to look at the issue quickly and get it merged and released.

minusdavid commented 7 years ago

No worries @kasei. I'm happy to contribute! And I really appreciate your responsiveness! Thanks for taking a look at this when you can.

minusdavid commented 7 years ago

I've actually been having UTF-8 encoding issues too, but I'm going to investigate that on my end first, and then raise a new issue once I figure it out...

kasei commented 7 years ago

Oh no! We've had utf-8 issues pop up from time to time and they're often a real pain to diagnose and fix! Feel free to drop by #perlrdf on irc.perl.org if you want to run anything by the few of us who hang out there. Not always low latency, but a number of people familiar with the codebase.

minusdavid commented 7 years ago

Ahh, good to know! Sure, I'll pop by in a second. You mentioned that RDF::Trine::Store::SPARQL is one of the least used pieces of code. Does that mean that most people tend to use the RDF::Trine::Store::DBI* backends or something else?

kasei commented 7 years ago

Looks good, thanks!

minusdavid commented 7 years ago

Thanks for the merge, @kasei!

Do you know when a new version might be cut and posted on CPAN?

kasei commented 7 years ago

@minusdavid – I just uploaded a beta version (1.016_01). I'll wait until I see it passing on cpan-testers before doing a full release.

minusdavid commented 7 years ago

@kasei Sounds good. I hadn't heard of cpan-testers before but it seems interesting. So in a few days http://www.cpantesters.org/distro/R/RDF-Trine.html will probably be updated and then there will be a full release? Sounds good to me.

minusdavid commented 7 years ago

Now that this has been pushed to master, I'm going to close issue #146