kenhuuu commented on code in PR #3485:
URL: https://github.com/apache/tinkerpop/pull/3485#discussion_r3531725938
##########
gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/GraphBinaryWriterPdtTest.java:
##########
@@ -22,8 +22,8 @@
import org.apache.tinkerpop.gremlin.structure.io.Buffer;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryReader;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter;
-import org.apache.tinkerpop.gremlin.structure.io.pdt.ProviderDefined;
-import org.apache.tinkerpop.gremlin.structure.io.pdt.ProviderDefinedType;
+import org.apache.tinkerpop.gremlin.structure.io.binary.TypeSerializerRegistry;
+import org.apache.tinkerpop.gremlin.structure.io.pdt.*;
Review Comment:
Nit: don't use wildcard imports
##########
docs/src/dev/provider/index.asciidoc:
##########
@@ -1771,6 +1577,117 @@ serialization and the registry handles inbound
reconstruction.
For driver users consuming PDTs, see the <<gremlin-variants,Gremlin Variants>>
reference documentation for
each language driver.
+[[primitive-provider-defined-types]]
+==== Primitive Provider Defined Types
Review Comment:
Nit: this section doesn't flow well in the PDT section. It needs to be
better integrated. The main problem I get reading this is the lack of
introduction of primitive and composite PDT. That should be done right at the
top as the rest of the section applies to both.
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/translator/DotNetTranslateVisitor.java:
##########
@@ -1255,11 +1209,19 @@ public Void visitDurationLiteral(final
GremlinParser.DurationLiteralContext ctx)
@Override
public Void visitPdtLiteral(final GremlinParser.PdtLiteralContext ctx) {
- sb.append("new ProviderDefinedType(");
- sb.append(ctx.stringLiteral().getText());
- sb.append(", ");
- visitGenericMapLiteral(ctx.genericMapLiteral());
- sb.append(")");
+ if (ctx.genericMapLiteral() != null) {
+ sb.append("new ProviderDefinedType(");
+ sb.append(ctx.stringLiteral(0).getText());
+ sb.append(", ");
+ visitGenericMapLiteral(ctx.genericMapLiteral());
+ sb.append(")");
+ } else {
+ sb.append("new PrimitiveProviderDefinedType(");
Review Comment:
Its odd for composite PDT to be `ProviderDefinedType` and primitive to be
`PrimitiveProviderDefinedType`. This suggests a IS-A relationship, which
doesn't exist since primitive PDT is not a composite PDT.
--
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]
