r.between returns the wrong type (table)

[coffeemug]

> r.table('foo').between('a', 'z').typeOf()
"TABLE"

I believe the type should be SELECTION<STREAM>. We had a similar issue before (#317) -- could we add a polyglot test for this?

[jdoliner]

This is actually intended. If it didn't return a table you wouldn't be
able to chain an indexed ordery_by to it which is an important use case.
It is a bit weird that you can chain an insert after it. But it's not
really dangerous and it would require a new sortable type to disallow it
which isn't worth the complexity.
On Nov 30, 2013 5:42 AM, "coffeemug" notifications@github.com wrote:

r.table('foo').between('a', 'z').typeOf()"TABLE"

I believe the type should be SELECTION. We had a similar issue
before (#317 #317) --
could we add a polyglot test for this?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1728
.

[coffeemug]

Wait, that doesn't make sense. I can chain orderBy to streams:

> r.table('foo').pluck('id').typeOf()
"STREAM"
> r.table('foo').pluck('id').typeOf().orderBy('id') // ok

And arrays:

> r.table('foo').pluck('id').typeOf().orderBy('id').typeOf()
"ARRAY"
> r.table('foo').pluck('id').typeOf().orderBy('id').orderBy('id') // ok

And SELECTION<ARRAY>:

> r.table('foo').getAll("5516c933-1f22-4ea1-9aa9-37b9bad1bfb7").typeOf()
"SELECTION<ARRAY>"

> r.table('foo').getAll("5516c933-1f22-4ea1-9aa9-37b9bad1bfb7").orderBy('id') // ok

And SELECTION<STREAM>:

> r.table('foo').orderBy({index: 'id'}).typeOf()
"SELECTION<STREAM>"

> r.table('foo').orderBy({index: 'id'}).orderBy('id') // ok

Why couldn't we do orderBy after between if it didn't return a table?

[coffeemug]

So, upon further reflection, the only thing I see that we can do by returning a table from between that we couldn't do otherwise is this:

r.table('foo').between('a', 'z').orderBy({index: 'id'}) // ok

But I really don't think we should special case it like this for between, for the following reasons:

• We already tell people they can only use one index at a time for now everywhere else
• If we want to allow using multiple indexes (or the same index multiple times), there are many, many equally important cases where we'd want to do the same. I don't think it's worth special casing it here in exchange for a sloppier return type.

[jdoliner]

Right it's only for indexed order_by that's an important one though I've
had a lot of people requested it and I know for a fact a lot of people are
using this in their code. In fact it's really the only efficient way to do
pagination. Pragmatically I don't think this quirk imposes a big enough
cost on users that it's worth losing the benefit. Especially when we know
there's a straightforward "right" way to implement the behavior it's just
work that doesn't pay high dividends right now.

Also what are these equally important cases where people want to use the
same index twice that we don't support? I made a pretty big effort to make
sure all of them were supported (because I had requests for basically all
of them) and I'm wondering which one's I missed.

On Saturday, November 30, 2013, coffeemug wrote:

So, upon further reflection, the only thing I see that we can do by
returning a table from between that we couldn't do otherwise is this:

r.table('foo').between('a', 'z').orderBy({index: 'id'}) // ok

But I really don't think we should special case it like this for between,
for the following reasons:

• We already tell people they can only use one index at a time for now
  everywhere else
• If we want to allow using multiple indexes (or the same index
  multiple times), there are many, many equally important cases where we'd
  want to do the same. I don't think it's worth special casing it here in
  exchange for a sloppier return type.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1728#issuecomment-29557668
.

[mlucy]

The fact that between returns a table has bugged me a bit ever since we implemented it. I think long-term we're just going to have to add another type to the type system (INDEXABLE_STREAM or TABLE_SLICE or something).

In the short-term, another option here is to just add an optarg order to between, which when set to true causes the resulting stream to be ordered by the same index you're betweening on.

[mlucy]

Also, moving this to 1.12 because it's a ReQL change rather than a bug-fix, and thus shouldn't go in a point release.

[coffeemug]

it's really the only efficient way to do pagination.

Ah!

Also what are these equally important cases where people want to use the same index twice that we don't support? [...] I made a pretty big effort to make sure all of them

Never mind. I was thinking stuff like r.table('foo').get_all('bar', index='baz').order_by(..., index='baz') but relative to pagination it's obviously a lot less important.

Which other ones are you thinking of?

In the short-term, another option here is to just add an optarg order to between

Eh, that would pollute the API in a way that we can't easily undo later without breaking people's code. And it smells messy, like something The Other NoSQL Database Company (tm) would do :-D I'd prefer to wait until we do things right rather than add this hack.

I think long-term we're just going to have to add another type to the type system (INDEXABLE_STREAM or TABLE_SLICE or something).

What about these options:

• Don't denote indexability in the user-facing type. For example, make streams indexable in some cases and not indexable in others, but don't have a way to differentiate between the two by calling type_of() -- just throw a runtime error "this stream is not indexable".
• Add a notion of attributes to types. For example: STREAM[INDEXABLE], SELECTION<STREAM[INDEXABLE]>.

[coffeemug]

Also, I don't think we should worry about this for 1.12. Moving to subsequent.

[danielmewes]

The query now returns "TABLE_SLICE". @coffeemug do you think we should do more about this?

[coffeemug]

I'm ok with that.

[coffeemug]

Meaning, I don't think there is more to do.