Skip to content
This repository was archived by the owner on Feb 21, 2024. It is now read-only.

Store() function#1666

Merged
travisturner merged 5 commits intoFeatureBaseDB:masterfrom
travisturner:store-function
Oct 5, 2018
Merged

Store() function#1666
travisturner merged 5 commits intoFeatureBaseDB:masterfrom
travisturner:store-function

Conversation

@travisturner
Copy link
Member

@travisturner travisturner commented Sep 25, 2018

Overview

This PR implements Store(<row_query>, <fieldname>=10). Currently, it's only supported on set fields. It's not clear to me yet how we would support this on mutex and bool fields. And I don't think it makes sense to store a row result into int or time fields (although I suppose if Store() took an optional timestamp, it could apply the row to all time quantums).

Also, the PQL function is Store(), but internally everything is SetRow() because it seemed like a complement to ClearRow().

Fixes #637 (at least the first phase of it)

TODO:

  • Docs
  • Decide if we want to support time fields.

Pull request checklist

Code review checklist

This is the checklist that the reviewer will follow while reviewing your pull request. You do not need to do anything with this checklist, but be aware of what the reviewer will be looking for.

  • Ensure that any changes to external docs have been included in this pull request.
  • If the changes require that minor/major versions need to be updated, tag the PR appropriately.
  • Ensure the new code is properly commented and follows Idiomatic Go.
  • Check that tests have been written and that they cover the new functionality.
  • Run tests and ensure they pass.
  • Build and run the code, performing any applicable integration testing.

Copy link
Contributor

@tgruben tgruben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few minor tweaks

@jaffee
Copy link
Member

jaffee commented Sep 26, 2018

I vote "no" on time fields - keep things simple for now.

@tgruben
Copy link
Contributor

tgruben commented Sep 26, 2018

I vote only set fields

@travisturner travisturner changed the title WIP: Store() function Store() function Sep 26, 2018
@travisturner
Copy link
Member Author

Since we're only supporting set fields as the destination field, I'm taking this PR out of WIP.

@alanbernstein
Copy link
Contributor

This supports an arbitrary bitmap call, right? I would suggest adding a second docs example explicitly showing something simple like Store(Intersect(x, y), z), to demonstrate that you can sort of perform computations this way. Maybe a test that does something like that as well.

I agree on only supporting set fields for now, but some thoughts on future directions... Storing to mutex seems reasonable - doesn't it make sense (conceptually at least) to write some bits to one row of a mutex field, then clear any conflicting bits as necessary?

I can imagine performing arithmetic operations on int fields and storing the entire result into another int field. This is obviously far-future kind of work though, and its not clear if it would even make sense to use Pilosa for this.

@travisturner
Copy link
Member Author

@alanbernstein I added another example to the query docs.

I think that mutex support is do-able, but it will likely require that we handle the individual columns, checking for existing values on each.

I like the idea of storing arithmetic operations performed on int fields. That made me wonder about doing arithmetic operations on an int field in place. For example, add 7 to all columns in int field f. I think this would require us to do multiple passes and re-writing of rows, and I'm not even sure that's useful, but something we could likely do fairly easily.

tgruben
tgruben previously approved these changes Oct 5, 2018
@travisturner travisturner merged commit 87a2bff into FeatureBaseDB:master Oct 5, 2018
@travisturner travisturner deleted the store-function branch October 5, 2018 15:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Store bitmap result directly

4 participants