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

ReQL proposal: add an optarg step to r.range #3148

Open
VeXocide opened this issue Oct 6, 2014 · 11 comments · May be fixed by #5545
Open

ReQL proposal: add an optarg step to r.range #3148

VeXocide opened this issue Oct 6, 2014 · 11 comments · May be fixed by #5545

Comments

@VeXocide
Copy link
Member

VeXocide commented Oct 6, 2014

Currently r.range as proposed in #875 always increments by one, which I propose to be specified through an optarg named step.

> r.range(step=10)
[0, 10, 20, ...]
> r.range(4, step=2).run
[0, 2]
> r.range(5, 2, step=-1).run
[5, 4, 3]
@danielmewes danielmewes added this to the 1.16-polish milestone Oct 6, 2014
@danielmewes
Copy link
Member

Makes sense.
I suggest we wait for two days, just in case there are any objections.

@marshall007
Copy link
Contributor

It would be really useful if you could also pass it a unit of time, i.e. second|minute|hour|day|week|month|year. This would make time series stuff way easier.

@danielmewes
Copy link
Member

@marshall007 I believe the current implementation of r.range() is only for numbers. Would you mind opening a separate issue for extending it to dates (there's also the question of whether these terms should honor leap years etc.)?

@VeXocide
Copy link
Member Author

VeXocide commented Oct 6, 2014

@marshall007 I've just opened #3149 which proposes to support dates and floating point numbers.

@VeXocide
Copy link
Member Author

VeXocide commented Oct 6, 2014

@mlucy just suggested naming the optarg stride instead of step, jotting it down here so I won't forget.

@mlucy
Copy link
Member

mlucy commented Oct 8, 2014

I would prefer to hold off on this until the next ReQL discussion period, just because it can be a distraction to settle ReQL proposals in the middle of a release cycle, but if people think it's essential to the command then I guess it's OK to do it as part of 1.16.

I have no strong opinion on stride vs. step. We should use whatever seems most common.

@gchpaco
Copy link
Contributor

gchpaco commented Oct 8, 2014

I'm used to that parameter being named stride, but as @mlucy says it's not critical. The user can always work around it with r.map.

@VeXocide
Copy link
Member Author

VeXocide commented Oct 8, 2014

@mlucy The initial implementation of #875 is enough to support #2574 thus this proposal is non-essential in my opinion. Also, due to its intricate relationship with #3149, which will require a bit of discussion, I agree we should hold it off until the next ReQL discussion period.

@danielmewes
Copy link
Member

I originally thought of this as a straight-forward ammendment to the range command. But since at least the name of the opt arg needs a little discussion, I'm going to move this into subsequent.

@danielmewes danielmewes modified the milestones: subsequent, 1.16-polish Oct 8, 2014
@danielmewes danielmewes modified the milestones: subsequent, backlog Aug 27, 2015
@marshall007
Copy link
Contributor

I have a table that stores documents with an effective date range and I'd like to be able to efficiently query for the active ones on a given date. The step optarg proposed here + date support from #3149 would make this a lot easier.

table.indexCreate(
  'day',
  r.range(r.row('date_start').date(), r.row('date_end').date(), { step: 'day' }),
  { multi: true }
)

table.getAll(r.now().date(), { index: 'day' })

I think this is a fairly compelling use-case. I imagine there are plenty of other interesting cases where the step optarg would be useful in secondary index functions.

@thelinuxlich
Copy link

Interesting use

@marshall007 marshall007 linked a pull request Mar 19, 2016 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants