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 #10

Open
lain-dono opened this issue Apr 19, 2024 · 9 comments
Open

Remove #10

lain-dono opened this issue Apr 19, 2024 · 9 comments
Labels
question Further information is requested

Comments

@lain-dono
Copy link

Sometimes you may need to remove some records from a table. This can be done through an additional optional filter removed.

@GlenDC
Copy link
Member

GlenDC commented Apr 19, 2024

The scope of the project is to be append-only and immutable after insertion. Technically we could support pop without much effort, but random removals is out of scope and even popping is not something I am inclined to support due to it leaking implementation details.

Optional filters are already possible.

On the query result you'll have a method:

EmployeeInMemDBQueryResult::filter<F>(&self, predicate: F) -> Option<#EmployeeInMemDBQueryResult> where F: Fn(&#name) -> bool

It return Some(_) EmployeeInMemDBQueryResult with the same reference data, but containing (and owning) only the indexes for which the linked row matches according to the given Fn predicate.

This is documented in https://github.com/plabayo/venndb?tab=readme-ov-file#generated-code-summary-method-api

Does that work for you? Otherwise I might either misunderstand you, so please do elaborate, or perhaps this project is not what you were looking for. Looking forward to your response nad thank you for your interest.

@GlenDC GlenDC added the question Further information is requested label Apr 19, 2024
@lain-dono
Copy link
Author

Does that work for you?

Yes. However, it would be more convenient to use something like:

#[derive(Debug, venndb::VennDB)]
#[venndb(removable_via = id)]
pub struct Table {
    #[venndb(key)]
    pub id: i64,
    // ....
}

using HashSet, or

#[derive(Debug, venndb::VennDB)]
#[venndb(removable)]
pub struct Table {
    // ....
}

using BitVec

I think it can be done without mutable access via atomic bitset.

@GlenDC
Copy link
Member

GlenDC commented Apr 19, 2024

Feel free to play with the idea in a branch and make a draft PR for it. It does intrigue me. We can have a quick feedback loop and work together in a capacity you see fit.

I'm ok with it being a breaking change and have it as a new release (e.g. 0.5), but of course let's not break just to break, so do try to fit it as much as possible within the original (current) implementation, unless there are good reasons not to. And perhaps first try to get a quick PoC working in the codebase and we can iterate on this first, prior to adding documentation, plenty of tests etc...

@GlenDC
Copy link
Member

GlenDC commented Apr 19, 2024

The reason I was currently supporting filtering is because sometimes you might want to (temporarely) not want certain rows (e.g. because they are disabled for some time). It's for that purpose that I currently use the ::filter method already available. I do get however that it's not the most convenient, but it does cover these things.

Removing of data is tbh not in the scope of this project. But now that we talk about it. It seems to me more like you want to provide support for atomic mutations of bit proporties (foo: bool). So if we can make that work and if that's indeed your intent, then sure, I am all for that.

@GlenDC
Copy link
Member

GlenDC commented Apr 19, 2024

@lain-dono feel free to get the ball rolling just by showing anon-macro isolated example of the code you have in mind. It doesn't have to be macro or integrated code, should it make it easier for you to show how more or less you see this. It can be enough to kick the ball towards this becoming a reality.

@lain-dono
Copy link
Author

I did a little research and found out there's an easier way.

If you use SlotMap instead of Vec, you get the ability to use remove at the cost of an extra 4 bytes per item.

About AtomicBitSet. I haven't found a complete implementation. hibitset can't grow. uniset does not implement test/get. Note that any implementation can have either set or clear as atomic. Not both. And of course you can't have filter and filter_not at the same time preserving atomic.

@GlenDC
Copy link
Member

GlenDC commented Apr 20, 2024

Slotmap can remove, sure. But it requires mut access, which is out of scope.

What's the usecase where you need something to be removed? As you might be trying to go for the wrong tool here.

Removal will require mut access, meaning it will need to be locked if used in a concurrent environment, for which it is designed.

An atomic bitset for mutating bits is something I might be open too, and I have even thought about an implementation based on a Vec that can probably work. But dunno, haven't found the need for it atm and neither does there seem to be an implementation available to use.

If you do find one that can be used and has the requirements we need (&self only), then we can start evaluating it for sure.

@GlenDC
Copy link
Member

GlenDC commented Apr 20, 2024

By the way itself not a big issue that an implementation cannot grow. Can be worked around by creating a new one double in size when we need more capacity and copy old data over.

@GlenDC
Copy link
Member

GlenDC commented Apr 20, 2024

That said, that mutation in this story does not end with the bit , as that would make the property in the row itself out of sync. So mutation involves the affected row(s) AND the instance the vec itself.

Which will require something like a mutex which is not desired.

Unless of course we create the rows on the fly when getting them. That might enable such mutations (overwrites).

Feel free to come up with a POC, even if just theoretical on paper here. Happy to help where desired as well.

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

No branches or pull requests

2 participants