[
https://issues.apache.org/jira/browse/TINKERPOP-3035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17841266#comment-17841266
]
ASF GitHub Bot commented on TINKERPOP-3035:
-------------------------------------------
FlorianHockmann commented on PR #2565:
URL: https://github.com/apache/tinkerpop/pull/2565#issuecomment-2079581200
The problem is however that the Gherkin tests already include this step but
they already passed before my change here as @spmallette noted in the issue
description.
This got me curious however as I was wondering how this could actually work
without an overload taking a dictionary in C# since I couldn't get it to work
in C# without my change here. (I did write a test case when I implemented this.
I only deleted it before committing to not impact other tests.)
Stephens assumption was:
> It works because it probably piggybacks on property(object?, object?,
objects[]?) which has all nullable arguments.
Turns out that this is not the case. The `DotNetTranslator` produces a C#
traversal with individual `Property` steps for each map entry:
https://github.com/apache/tinkerpop/blob/e3ced38b2108f3edb29f11fbfa4f6950427bcbd9/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs#L539
So, as a next attempt I wanted to change the translator to instead produce a
C# traversal using the new `Dictionary` overload.
But then I learned that that's now even possible since Gremlin-Java already
produces Bytecode without this overload:
https://github.com/apache/tinkerpop/blob/e3ced38b2108f3edb29f11fbfa4f6950427bcbd9/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java#L3221-L3234
and the translator only gets the Bytecode produced from the traversal.
So I don't see any elegant way to test this using Gherkin tests without
changing the Gremlin-Java overload (which is a bigger change than what makes
sense here in my opinion). We could add a hard-coded translation for one
feature test to ensure that the traversal in C# will use the new overload, but
I don't really like maintaining such hard-coded translations.
However, this got me to the idea to instead use the translation from C# to
Groovy for a test. I just pushed an updated commit with such a test. The test
does not compile without the added overload and the translation also includes
the successful construction of Bytecode from the traversal as the translator
also uses the Bytecode.
> Add explicit property(IDictionary) for .NET
> -------------------------------------------
>
> Key: TINKERPOP-3035
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3035
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
> Affects Versions: 3.6.6
> Reporter: Stephen Mallette
> Priority: Minor
>
> There is no {{property(IDictionary<object,object>)}} method for .NET to
> accompany the {{property(Map)}} step. It works because it probably piggybacks
> on {{property(object?, object?, objects[]?)}} which has all nullable
> arguments. The explicit overload with the {{IDictionary}} should be added.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
