[GitHub] [tinkerpop] spmallette opened pull request #1266: TINKERPOP-2351 Fixed bug in Order when enum is a key in Map

["spmallette (GitHub)" <gi...@apache.org>]

https://issues.apache.org/jira/browse/TINKERPOP-2351

This seemed like an easy fix. Can't think of any bad side-effects. Not sure if there are other special cases we should consider, but at least this common case is solved.

```text
gremlin> g.V().valueMap(true).order(local).by(keys,desc)
==>[name:[marko],label:person,id:1,age:[29]]
==>[name:[vadas],label:person,id:2,age:[27]]
==>[name:[lop],lang:[java],label:software,id:3]
==>[name:[josh],label:person,id:4,age:[32]]
==>[name:[ripple],lang:[java],label:software,id:5]
==>[name:[peter],label:person,id:6,age:[35]]
gremlin> g.V(1).valueMap(true).order(local).by(keys,desc).next()[T.id]
==>1
gremlin> g.V(1).valueMap(true).order(local).by(keys,desc).next()[T.label]
==>person
```

When this merges forward to `3.4-dev` I will modify the test to use `elementMap()` rather than `valueMap(true)` since that method is deprecated.

All tests pass with `docker/build.sh -t -i`

VOTE +1

[ Full content available at: https://github.com/apache/tinkerpop/pull/1266 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette closed pull request #1266: TINKERPOP-2351 Fixed bug in Order when enum is a key in Map

["spmallette (GitHub)" <gi...@apache.org>]

[ pull request closed by spmallette ]

[ Full content available at: https://github.com/apache/tinkerpop/pull/1266 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] dkuppitz commented on issue #1266: TINKERPOP-2351 Fixed bug in Order when enum is a key in Map

["dkuppitz (GitHub)" <gi...@apache.org>]

My first thought was that the general rule rather be: If `a` and `b` have different (non-numeric) types, use the `toString()` value for both, otherwise:

```
gremlin> g.V().map(properties().group().by(value).by(key()))
==>[29:age,marko:name]
==>[vadas:name,27:age]
==>[java:lang,lop:name]
==>[32:age,josh:name]
==>[java:lang,ripple:name]
==>[35:age,peter:name]
gremlin> g.V().map(properties().group().by(value).by(key)).order(local).by(keys, desc)
java.lang.String cannot be cast to java.lang.Integer
```

But then, taking Groovy as a reference, it probably shouldn't work this way.

```
gremlin> [id:'1','b':2,'z':3].sort()
==>b=2
==>id=1
==>z=3
gremlin> [id:'1','b':2,3:3].sort()
java.lang.String cannot be cast to java.lang.Integer
Type ':help' or ':h' for help.
Display stack trace? [yN]
```

Thus, +1 on that change.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1266 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org