Syntax of specifying index with getAll()

[fiLLLip]

While waiting for the cursor troubles to get implemented, I used a workaround to get results with getAll at all:

            var result = (IEnumerator<object>)_rethinkDb.db(dbName).table(tableName).getAll(id1, id2, id3).run(_connection);
            List<Foo> all = new List<Foo>();
            try
            {
                while (result.MoveNext())
                {
                    all.Add(((JObject)result.Current).ToObject<Foo>());
                }
            }
            catch (InvalidOperationException)
            {
                // Swallow the error
            }
            return all;

This returns three objects in a list. If I want to query a secondary index I have tried to change the var result to the following with no results:

var result = (IEnumerator<object>)_rethinkDb.db(dbName).table(tableName).getAll(value, $"{{index={nameof(Foo.Value)}}}").run(_connection);

And if I try to change it to the following:

var result = (IEnumerator<object>)_rethinkDb.db(dbName).table(tableName).getAll(value, new {index=nameof(Foo.Value)}).run(_connection);

I get an error:

ReqlQueryLogicError: Primary keys must be either a number, string, bool, pseudotype or array (got type OBJECT):
{
"index":    "Url"
}

How do I query this in the right way?

[bchavez]

Almost done... actually, working on making the Cursor enumerator work:

Cursor<Foo> all = r.db(DbName).table(TableName).getAll("a", "b", "c").run<Foo>(conn);

all.BufferedItems.Dump();

foreach( var foo in all )
{
    foo.Dump();
}

Right now, tracking down a "no such element" exception.

[fiLLLip]

Are you looking at the option to specify the index like the first sample on http://www.rethinkdb.com/api/javascript/get_all/ ?

Scratch that... Reread your commet 👍

[bchavez]

Ah, sorry, didn't fully scroll horizontally and read your query.

As with the Java driver, to specify optional arguments like {index:'code_name'} I belive we do the following:

Cursor<Foo> all = r.table('marvel').getAll('man_of_steel').optArg("index", "code_name")
                   .run<Foo>(conn);

[bchavez]

If you're using Visual Studio, soon as you hit .optArg( you should get some IntelliSense about what optional arguments are available.

You can also chain up multiple opt args:

Cursor<Foo> all = r.db(DbName).table(TableName).getAll("a", "b", "c")
                .optArg("index", "code_name")
                .optArg("other", "param")
                .run<Foo>(conn);

[fiLLLip]

That worked like a charm!

[bchavez]

👍 😃

[fiLLLip]

The only downside with doing it like this, is that it don't follow the official API. Should the API try to parse the params if supplied as getAll(value, new {index=nameof(Foo.Value)}), or would the performance hit/complexity be a problem?

[bchavez]

Ah, I see. That's true for the javascript, python and ruby docs. I should clarify, we should be compatible with the official java API docs (but the official Java API docs aren't published yet, or written yet?). The official Java docs will have optArgs calls (if it doesn't change from now until release).

[fiLLLip]

Okey, I see! That makes sense 😄 Since this works by calling the .optArg("index", "code_name"), I'm just going to close this issue 👍

[bchavez]

I do have an issue #1 to explore using allow anon types for opt args. I think it would also be pretty cool, and help syntactically, but AFAIK the only way to enumerate properties on Anonymous types is to use reflection (or possibly direct serialization from Newtonsoft). We'd have to investigate the performance bit more IMHO for serializing anonymous type opt args.

Complexity probably wont be a big issue if we stick to using .optArg(). Perhaps, we can additionally support:

Cursor<Foo> all = r.db(DbName).table(TableName).getAll("a", "b", "c")
                .optArg(new {index = "code_name", other="param"})
                .run<Foo>(conn);

[deontologician]

The usual way in the other drivers is to use named/optional arguments if the language supports it. I think the .optArg thing we do in the Java driver is the best of several bad choices, but if C# has optional arguments I'd say go for it

[bchavez]

I looked a bit more into getting optional arguments in the same line as the ReQL term call:

Option A

[Test]
public void anonymous_type_optargs()
{
    /* With Optional Arguments  */
     var all = r.db("marvel").table("actors")
                .getAll(new { index="code_name"}, "man_of_steel")
                .run<Actor>(conn);

    /* Without Optional Arguments */
    var all = r.db("marvel").table("actors")
               .getAll("man_of_steel")
               .run<Actor>(conn);
}
public GetAll getAll (object optArgs = null, params object[] exprs  )
{
   .....
}

In C#, the compiler forces us to always have params object[] parameters to always be last.

Ultimately, this means any terms with optArgs (like getAll) must come first whenever we encounter a term with argument Object.... All the ReQL drivers have their optArgs last, after the required args.

JavaScript

     r.db("marvel").table('actors')
      .getAll('man_of_steel', {index:'code_name'}).run()

Java

     r.db("marvel").table('actors')
      .getAll('man_of_steel').optArg("index", "code_name").run()

In the spirit of consistency and convention, if we decide to go this route (optArg first), we would probably require all terms with optArg to be optionally be defined first instead of last.

Thoughts? 💭

[bchavez]

Also, asked the question in IRC. AtnNn suggested using a structure....

Option B

It appears we have enough metadata from json_term_info.json to generate some "optarg" type information. For Example, .http's optArg:

    "optargs": {
      "timeout": "T_NUM",
      "reattempts": "T_NUM",
      "redirects": "T_NUM",
      "verify": "T_BOOL",
      "result_format": "E_RESULT_FORMAT",
      "method": "E_HTTP_METHOD",
      "auth": {
        "type": "E_AUTH_TYPE",
        "user": "T_STR",
        "pass": "T_STR"
      },
      "params": "T_OBJECT",
      "header": [
        "T_ARRAY",
        "T_OBJECT"
      ],
      "data": [
        "T_STR",
        "T_OBJECT"
      ]
    }

The ReQL for this would probably look like:

var all = r.db("marvel").table("actors")
                .getAll(opts =>
                    {
                        opts.index = "code_name";
                        opts.other = "param";
                    }, "man_of_steel")
                .run<Actor>(conn);

Some questions

Could we rely on optargs in java_term_info.json to generate typed optArgs? is it complete enough? danielmewes suggested this could add some extra maintainability issues since optArgs are constantly being added.

I think the only benefit here is we'd get code-completion to help us navigate optArgs. Not sure I like Option B's Action<OptArg> lambda though.... hm. Feels a bit weird writing action code in a section for optArg... 💭

[bchavez]

💭 also, thinking about this a bit more... and reviewing the overall architecture.

It might be best to drop the whole OptArg infrastructure the Java driver is using and just use Option A. There's a lot of complexity in dealing with OptArg.

Mainly, we'd have to do two conversions to support this anonymous optArgs. Take an anonymous optArg convert it to a real OptArg like Java is using, then convert it back to a JObject for serialization:

[Writing Query       ]  [Serialization   ]
Anonymous -> JObject -> Opt Arg -> JObject

If we removed OptArg infrastructure, we could simplify everything and keep it:

[Writing Query]  [Serialization   ]
Anonymous    ->    JObject

Also, the API-ness of the Java OptArg just doesn't feel at home when writing ReQL.

r.db().table().
   .getAll()
   .optArg("index","foo")
   .otherTerm()
   .optArg("other","param")
   .run()

[fiLLLip]

Disclaimer: I have not worked a lot with reflection, nor RethinkDB in general.
When using a getAll(), would you ever use another types than basic types to match on specified indices? If not, could we do getAll("man_of_steel", new { index="code_name"}) and use reflection on params and set the dynamic typed object as optArg behind the scenes?
Would this cause a lot of performance overhead?

[bchavez]

As far as I know, there's no way during run-time to know specifically if an object is an anonymous type or not; unless maybe reflection is done on the object.GetType().Name.Contains("AnonymousType").

It's a bit ugly and not exactly proof. :/

[bchavez]

I started a local branch with Option A; getting a feel for it's overall architecture. It does dramatically simplify the use of optional arguments and internal workings.

[fiLLLip]

I found a method for matching on Anonymous type on StackOverflow. It looks kind of similar to your method. Would it be possible to test the performance overhead?

[bchavez]

More on Option A:

Just regenerated the AST using anonymous type optArgs; and it looks like we'll have a situation:

Table Term

public Get get ( Object expr, object optArgs = null)
{
      Arguments arguments = new Arguments(this);
      arguments.CoerceAndAdd(expr);
      return new Get (arguments , optArgs );
}

public GetAll getAll ( object optArgs = null, params object[] exprs )
{
     Arguments arguments = new Arguments(this);
     arguments.CoerceAndAddAll(exprs);
     return new GetAll (arguments , optArgs );
}

So, when prams is present, optArgs must be first; when params is not present, optArgs must be last.

We'd have this awkward situation where optArgs could be first or last depending on get or getAll (and other terms like it).

//get
var hero = r.db("marvel").table("actors")
               .get("id", new{ other="param" })
               .run();
//getAll
var all = r.db("marvel").table("actors")
               .getAll(new{ index="code_name" }, "id")
               .run();

[bchavez]

LoL. Another Option C, abuse the language a bit and use operator indexer overloading...

var all = r.db(DbName).table(TableName)
                .getAll("a", "b", "c")[new {index = "code_name"}]
                .run<Foo>();

Anders would probably scream, but does have an interesting ReQL syntax feel to it. ... and in a weird way, kinda makes sense. Feels right. Like you're injecting an anonymous optArg type into getAll.

Pros

• OptArgs will always appear last.
• Keeps the compiler happy. Avoids first/last issues with the compiler.
• Kinda makes sense.

Cons

• A bit of language abuse.

💭 thoughts?

[deontologician]

It looks neat to me. There's sort of an interesting trade-off between "does this look like ReQL" and "does this look like C#". As far as optargs, my intention is to keep them up to date in the json data. It shouldn't be too hard since reql changes only happen in major versions, and we tag all of the issues with changes in them. While the java driver doesn't use the optarg info, I think it could provide a nice interface in other static languages

[bchavez]

I thought about it a lot over the weekend. I'm liking Option C indexer OptArg's.

We can also support additional strongly typed OptArgs the same way with Option C if we ever decide to use fully typed optional arguments.

My gut feeling about Option C is it might be worth deviating from the Java driver to get really cool syntax for ReQL's OptArgs here with C#. I'm going to try to finish this out in my local branch to see how it goes.

If anyone has any objections, please let me know.

[bchavez]

I decided to implement Option C while keeping Option A.

I got the point of converting all the ReQL unit tests, and going solo with Option C was just too complex converting .optArg("key", r.array().hash().with("",ff))) into an anonymous typed arg without some modification to convert_tests.py.

So, in all, the C# driver supports both: getAll(...).optArg() similar to the Java driver method calls and getAll(..)[new { opt='arg'}] anonymous types via indexers.

https://github.com/bchavez/RethinkDb.Driver#optional-arguments-with-getall