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

Erroneous handling of projection for relation already restricted by another relation #129

Closed
eywalker opened this issue Jul 17, 2015 · 4 comments

Comments

@eywalker
Copy link
Contributor

When projecting a relation (call it A) that is restricted by another relation (B), it results incomplete SQL string if B heading contains attributes that are not inside projected attributes of A. For more concrete example, assume that relation A have pk attributes (a, b) and non-pk attributes (x, y, z). Also assume that (x) is a pk attribute in B. Given this, following pseudo-code was giving error:

(A & B).project()

This turns out to be due to recent optimization in Project that expanded the restrictions on the argument of project (in this case A) if there are no aliasing operation during project. This essentially resulted in the restriction (B) to be applied after the projection operation, giving (essentially)

A.project() & B

Note that A.project() only contains (a, b) and not x, thus resulting in improper restriction by B which only contains attribute x.

I believe that this change was introduced in an attempt to speed up some projections where you could truly expand restrictions: for example, if in above case B contained attributes (a,b) instead, then expansion would have worked. However, as this case showed, checking for alias is not enough. I've implemented a compromise where I check for B's heading and see if all attributes of B belongs to projected attributes for A. If true, then restriction is expanded. Otherwise, Subquery is used. It is quite possible that a better optimization can be performed. I just wanted to make sure all were aware of the changes and the reason behind it.

@eywalker
Copy link
Contributor Author

Current implementation of Project is also still bug laden, as the extra clause I inserted does not work. We will have to carefully visit the implementation and see what performance improvements could be made. We should probably add more tests to verify that we got a perfectly functioning (though possibly slow) version implemented prior to making tweaks.

@dimitri-yatsenko
Copy link
Member

I am fixing this on the development branch. Please add a very specific example that reproduces the problem.

@dimitri-yatsenko
Copy link
Member

Fixed in e0aadc0?

@dimitri-yatsenko dimitri-yatsenko added this to the Release v0.1 milestone Aug 14, 2015
dimitri-yatsenko added a commit to dimitri-yatsenko/datajoint-python that referenced this issue Aug 18, 2015
@dimitri-yatsenko
Copy link
Member

Fixed by enclosing restricted relations in subquery. There is certainly a more efficient solution, but this is simple and it works.

fabiansinz added a commit that referenced this issue Aug 18, 2015
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

2 participants