Skip to content
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

RDF::Trine::Store::SPARQL::_group_bulk_ops doesn't build array correctly #146

Closed
minusdavid opened this issue May 9, 2017 · 1 comment
Closed

Comments

@minusdavid
Copy link
Contributor

In RDF::Trine 1.016, RDF::Trine::Store::SPARQL::_group_bulk_ops produces an @aggops array that contains an arrayref where the 0th element is the type (e.g. '_add_statements'), and the 1st element is an array (which has a quad for the 0th element, undef for the 1st element, and then a series of arrays containing a quad and a context as expected).

So that's weird...

We iterate through @aggops, and dreference $aggop into ($type,@ops).

This creates another problem. @ops should probably be $ops there, because it seems we've now put an arrayref within an array.

So when we iterate through @ops, we only get one $op and $op->[0] gives us $st and $op->[1] gives us $context... but only for 1 operation. All the other operations are lost because they're part of this arrayref living in this single $op.

If we did:
my ($type, $ops) = @$aggop;

Instead of:
my ($type, @ops) = @$aggop;

Then we'd have an arrayref with many more operations, but it wouldn't work because the first 2 elements are a RDF::Trine::Statement::Quad object and undef. Those two elements should be wrapped up into an arrayref as well.

So ultimately... I think RDF::Trine::Store::SPARQL::_group_bulk_ops isn't creating @aggops correctly during RDF::Trine::Store::SPARQL::_end_bulk_ops, and $aggop isn't being dereferenced correctly in RDF::Trine::Store::SPARQL::_end_bulk_ops.

The problem is this line:
push(@bulkops, [$type, [ @{$op}[1 .. $#{ $op }] ]]);

It should be:
push(@bulkops, [$type, [ [ @{$op}[1 .. $#{ $op }] ]]]);

That also means the later line:
push(@bulkops, [$type, [ @{$op}[1 .. $#{ $op }] ]]);

Should be:
push(@bulkops, [$type, [[ @{$op}[1 .. $#{ $op }] ]]]);

I think this code could actually be a bit cleaner, but it's 8:17pm on a Tuesday, so I think a pull request might have to wait until tomorrow.

minusdavid added a commit to minusdavid/perlrdf that referenced this issue May 10, 2017
…tly kasei#146

As per issue kasei#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).
@minusdavid
Copy link
Contributor Author

Marking this as resolved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant