Cole-Greer commented on PR #3636: URL: https://github.com/apache/tinkerpop/pull/3636#issuecomment-5626894041
Hi @danielbodart, thanks for the submission and the response. Your federated queries use case sounds quite interesting, although there may be a connection there that I am missing. I'm not quite understanding why the map cannot be inlined into the InjectStep as a `genericMapLiteral` instead of passing via a `variable`. I'll share some context below on reasonings behind `literal`/`argument` divide with steps in the first place, but in short I'm not understanding the need for map variables in inject() at this time. The entire purpose of passing `argument`'s to steps instead of `literal`'s is to take advantage of query caching mechanisms which providers may optionally implement. The intent is that the raw query string (including the variable name) can be used as a key for this cache, and then providers can swap in specific parameter values later upon cache hits. Unless a provider leveraging such a caching strategy, there's generally not any advantage to passing step arguments as a `variable` instead of directly in-lining it as a `literal`. With this in mind, supporting variables in a step is actually a much more involved process than simply enabling the functionality in the grammar and parser. There needs to be an ability for the variables to be preserved inside the parsed `GraphTraversal`, and those variables must be able to survive optimizing `TraversalStrategy` application. The established pattern for this is to box variables into a `GValue`, which gets passed into a special `GValueHolder` placeholder step. You can reference a class like [GraphStepPlaceholder](https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStepPlaceholder.java) as an example. The preservation of GValues in the parser is controlled by the `VariableResolver` which is configured. We typically use `DirectVariableResolver` for most of our testing, which immediately reduces all variables and replaces them with their literals. In this configuration, there is no meaningful distinction between an argument being passed as a `variable` or a `literal`. If you were to instead configure `DefaultVariableResolver`, you would find that these variables will instead manifest as `GValue` objects in the parsed `GraphTraversal`, which would break the semantics of `InjectStep` without further modifications to follow the `GValueHolder` pattern. As-is, this would result in a `InjectStep<GValue<Map>>` instead of an `InjectStep<Map>` as desired. A GValue should never survive to the point of traversal execution. The set of steps which are currently permitted to accept variables was carefully curated to target steps which showed the greatest need and upside for this query caching use case. Due to the complexity of the `GValueHolder` pattern, we deliberately withheld such behaviour from steps which we did not find justified the complexity. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
