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

Remove dependency on activerecord-import #10

Open
nullstyle opened this issue Mar 16, 2015 · 1 comment
Open

Remove dependency on activerecord-import #10

nullstyle opened this issue Mar 16, 2015 · 1 comment

Comments

@nullstyle
Copy link

I'd love to use this gem in my latest project, but unfortunately I'm using JRuby and Postgres, which is non-compatible according the README (and my test suite).

As mentioned in the readme, this gem is dependent upon activerecord-import, which seems dubious given the point of the low card system is that you're only dealing with small-ish number of permutations.


Would you be open to accepting a PR that removes this dependency? I may be able to find some time this weekend to bang it out.

Thanks,
Scott

P.S. I hope everything is going well with you AG :)

@ageweke
Copy link
Owner

ageweke commented Apr 3, 2015

Scott!

How are you? Drop me a line ([email protected]) sometime.

I’m sorry it took me so long to respond — I’ve been furiously preparing for a through-hike of the Pacific Crest Trail this summer. (Sounds like something you’d do!) And I’ve been negligent in maintaining many of my open-source projects for that same reason (also, that most of my effort has gone into Fortitude).

Anyway, yes, absolutely, I’d be open to this. I think originally I used activerecord-import to try to use its “insert ignore” functionality to avoid race conditions, but then changed it so that was no longer how it worked or necessary (since I had to lock the whole table anyway, and, once you’re doing that, why bother?). If you submit a patch that works, I’ll definitely take it.

Finally, I’ve realized that low_card_tables needs a major rejiggering to make a number of cases work that don’t right now — hence the bugs. In short, right now it’s resolving constraints to low-card IDs eagerly, when those constrains are specified — i.e., where(:deleted => true) gets resolved to where(:status_id => [ 2, 4, 6, 8 ]) right when it gets called. But this is incorrect, because you want to do something like User.where(:deleted => true).new, and have it work even if there are no deleted users right. This means devolving resolution until query time, which I’m pretty sure means plugging much deeper into AREL, and that’s a pretty major change. I want to do it, but it’s going to have to wait until late fall or winter this year, once I’m back from my trip. It’s not majorly broken or anything right now (we use it at Swiftype in production all the time, and have had no problems), but there are just certain cases that should work that don’t right now and which it’s impossible to fix without this major change.

Hope that helps — drop me a line!

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