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

JavaScript driver - RqlCompileError with r.row and indexCreate #1555

Closed
neumino opened this issue Oct 19, 2013 · 13 comments
Closed

JavaScript driver - RqlCompileError with r.row and indexCreate #1555

neumino opened this issue Oct 19, 2013 · 13 comments
Assignees
Milestone

Comments

@neumino
Copy link
Member

neumino commented Oct 19, 2013

r.db('test').table('test').indexCreate("test", [r.row("x"), r.row("y"), r.row("z")])

Throws

RqlCompileError: Unrecognized optional argument `0`. in:
r.db("test").table("test").indexCreate("test", {0: r.row("x"), 1: r.row("y"), 2: r.row("z")})
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

On rethinkdb 1.10.0 (GCC 4.8.1) -- on Arch

@jdoliner
Copy link
Contributor

This is the problem we discussed at length in #915. To fix this we're probably going to have to change the way we do optargs in the javascript driver.

@jdoliner
Copy link
Contributor

This is also almost certainly an issue in the ruby driver.

@wojons
Copy link
Contributor

wojons commented Oct 19, 2013

@neumino was helping me and thats we came across it i was on 1.10 ubuntu. also it worked when i did it in an anonymous funcction

@jdoliner
Copy link
Contributor

Actually ruby users are pretty unlikely to hit this because they don't have r.row

@mlucy
Copy link
Member

mlucy commented Oct 19, 2013

@coffeemug -- who's in charge of the JS driver these days?

@jdoliner
Copy link
Contributor

@mlucy no one is officially. We should figure out what we can do to get around this problem. I think we're going to need to make a pretty big change to how we do optargs.

@neumino
Copy link
Member Author

neumino commented Oct 19, 2013

I think I'm the one in charge of the JS driver.

About this bug, I don't think it's related to how we do optargs.
It's just a bug in ast.coffee line 219. We check if the second argument is an object with instanceof Object instead of calling Object.prototype.toString, and since an array is an instance of Object, we get the current bug.

I have a fix that seems to work. I'll add some tests on Monday before pushing it.

@ghost ghost assigned neumino Oct 19, 2013
@neumino
Copy link
Member Author

neumino commented Oct 19, 2013

Assigning to myself too since I have a fix coming.

@coffeemug
Copy link
Contributor

There is no one person officially responsible for the Javascript driver, but most fixes have been done by @neumino and @AtnNn, so Javascript bugs should be redirected to them.

I have a fix that seems to work. I'll add some tests on Monday before pushing it.

Just a friendly reminder, please observe the standard code review process for the JS driver.

@neumino
Copy link
Member Author

neumino commented Oct 20, 2013

Yep, by "pushing it" I meant opening a code review first.

@neumino
Copy link
Member Author

neumino commented Oct 25, 2013

As far as I know, these queries are also broken

r.table("foo").getAll( [1,2,3] ).run( connection, callback) // Throws when it should work
r.table("foo").orderBy( [1,2,3] ).run( ..) // Doesn't throw the good error

@neumino
Copy link
Member Author

neumino commented Oct 25, 2013

In code review 993 assigned to @AtnNn

@neumino
Copy link
Member Author

neumino commented Oct 31, 2013

Review 993
Merged in next as 51b4e41
Cherry picked in v1.10.x

@neumino neumino closed this as completed Oct 31, 2013
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

5 participants