Just a quick follow-up, I'm planning to make a small change that I thought
you all might want to be aware of but it's probably small enough to not
need its own thread. In GraphSON, edge had a "inVLabel", "outVLabel",
"inV", and "outV" to refer to the vertices in both directions. However,
this isn't the same as the format for a vertex which is an object that
contains an "id" and "label". I think it makes more sense to modify "inV"
and "outV" to be reference vertices that contain a "id" and a "label"
without "properties". I've made a PR for that at
https://github.com/apache/tinkerpop/pull/2810

On Thu, Sep 12, 2024 at 1:42 PM Yang Xia <xia...@apache.org> wrote:

> Thanks Ken! Given consensus on this discussion, I've began implementing
> some of the changes in Java and Python to align the GraphBinary serializers
> with the spec, starting with removing old serializers and updating
> DateTime: https://github.com/apache/tinkerpop/pull/2776. Reviews and
> comments are welcome.
>
> Cheers,
>
> Yang
>
> On 2024/08/28 04:01:51 Ken Hu wrote:
> > Here are some other updates that I think should be made for
> GraphBinaryV4.
> >
> > There are steps that can return a distinct ordering in Maps like
> > order(local). However, these aren't necessarily deserialized in GLVs as
> > Map-types that preserve this ordering. I think that we should add a
> > value_flag to the Map type in GraphBinary that signals that the
> > deserialized version of this Map should maintain the ordering. Similar to
> > the BulkSet change, we can use a value_flag with the value 0x02 to denote
> > ordering. The value itself would remain unchanged, but the order of the
> > items in that list should be the insertion/iteration order of the
> > deserialized Map.
> >
> > All values that the Date type can represent are covered by OffsetDateTime
> > so I think it makes sense to remove even the Date type. Date is also
> > outdated in Java and its use isn't recommended.
> >
> > The current GraphBinary standard states that properties for elements like
> > Vertex, Edge and VertexProperty should be set to null. The reasoning for
> > this was that properties were never returned in remote traversals.
> However,
> > this changed starting in 3.7, and properties can now be returned.
> > Properties should instead be a List of Properties and if there are no
> > properties then an empty list should be returned.
> >
> > Labels for elements are currently serialized as String for both
> GraphBinary
> > and GraphSON. However, we may want to support multi-labels or no-label
> > scenarios in the future. To prevent needing a breaking change if this
> were
> > to happen, I propose that "label" become a List of String.
> >
> > In the new HTTP API, a ReponseMessage is returned per HTTP chunk and the
> > HTTP response could end with a trailer. The GLVs may need some way to
> > distinguish between chunks and trailers which could be accomplished
> using a
> > one byte buffer in between the last ResponseMessage and the trailing
> > header. The GLVs may need information to recreate a chunk as some HTTP
> > clients transparently dechunk the body. A 4-byte header that indicates
> the
> > size of ResponseMessage could be added to help GLVs recognize chunk
> > boundaries which simplifies deserialization.
> >
> > On Thu, Aug 22, 2024 at 2:34 PM Ken Hu <kenhu...@gmail.com> wrote:
> >
> > > Hi All,
> > >
> > > I've noticed that there are some serializers/deserializers that exist
> for
> > > GraphBinary and GraphSON that may not serve a real purpose and I'm
> > > proposing that we remove them in V4. In general, I think we should move
> > > away from providing serializers for all types in the Java Standard
> Library,
> > > as there are many types that other GLVs simply won't support. The
> types in
> > > GraphSONv3/GraphBinaryV3 that I would remove include:
> > >
> > > Barrier, Cardinality, Column, DT, Merge, Operator, Order, Pick, Pop,
> > > Scope, T
> > >
> > > Because the GLVs will now use GremlinLang (instead of Bytecode), these
> > > values will be translated to Strings so they don't need their own
> specific
> > > serializers. T and Direction are the two exceptions because they are
> > > returned in the valueMap() and elementMap() steps, although it's likely
> > > they will be replaced in the future as well. For similar reasons, P and
> > > TextP should also be removed since they will also be Strings in
> GremlinLang.
> > >
> > > Period, Timestamp, Instant, ZonedDateTime, OffsetTime, LocalDateTime,
> > > LocalDate, LocalTime, MonthDay, YearMonth, Year, ZoneOffset
> > >
> > > These are from the java.time package but are all variants of
> > > OffsetDateTime and Date. I don't think it makes sense to provide
> > > serializers for these Java-specific types that are mostly unsupported
> by
> > > the other GLVs and can be represented by either an OffsetDateTime
> and/or
> > > Date, so I think we should only have OffsetDateTime and Date.
> > >
> > > BulkSet and Traverser are two optimizations that exist to increase
> > > performance but that the user shouldn't be able to interact with. I'm
> > > suggesting that we remove these serializers, however, to keep the
> > > performance optimizations, I would suggest that we add a flag to the
> List
> > > type so that it can achieve the same sort of bulking that those types
> > > provided. GraphSON would simply serialize these as arrays without a
> bulk so
> > > it would be the expanded BulkSet. GraphBinary would have an additional
> > > value_flag for List with the value 0x02 to denote bulking. This would
> mean
> > > the item_i is a fully qualified item followed by a Long value that is
> the
> > > bulk.
> > >
> > > Class and InetAddress are highly Java-specific and can't be used by the
> > > other GLVs. Binding and Bytecode have both been removed so it doesn't
> make
> > > sense to keep their serializers around. Metrics, TraversalMetrics can
> be
> > > represented as a list of Strings so they shouldn't require their own
> > > serializers.
> > >
> > > The last type that I think should be removed is Custom. The most common
> > > complaint that I've seen from end-users regarding custom types is that
> they
> > > lack serializers in their GLV of choice. Custom serializers require
> extra
> > > effort from providers as it is they that need to maintain and publish
> them.
> > > Users often don't realize that they can't just use the GLV without
> adding
> > > the custom serializer from their provider which can lead to a
> frustrating
> > > initial experience. So, instead, to provide a more seamless user
> experience
> > > I'm proposing we replace Custom with a Provider Defined Type (PDT).
> > > Actually, two PDTs, one for primitive types and one for composite
> types.
> > > These PDTs will also serve as a mechanism to allow reading of values
> whose
> > > serializer has been removed in V4. This would allow a provider that
> > > supports one of those Java-specific types to still return the value as
> a
> > > PDT.
> > >
> > > The PrimitivePdt will contain two fields: a String for the type name
> and a
> > > String for the actual value. The CompositePdt will contain two fields:
> a
> > > String for the type name and a Map that contains the fields of the
> > > composite type (most likely a class). PDTs will come with default
> > > serializers and deserializers. On the user-side, the values will be
> > > deserialized into either PrimitivePdt or CompositePdt. In order to
> write a
> > > PDT to the server, the user would have to look up the required fields
> for
> > > that specific type name. This information would likely need to exist
> in the
> > > provider's documentation. On the server, there will be an API (probably
> > > attached to the Graph interface) that will allow the provider to add
> types
> > > that should be able to be serialized/deserialized from PDT. In order to
> > > achieve this, there will need to be a TinkerPopCustomType interface
> that
> > > has two methods: serialize() and deserialize(). Then, any type that
> > > implements TinkerPopCustomType could automatically be
> > > serialized/deserialized with the provided PDT serializers for both
> > > GraphSONv4 and GraphBinaryV4.
> > >
> > > Does anyone have any comments or concerns about these proposed changes?
> > >
> > > Thanks,
> > > Ken
> > >
> >
>

Reply via email to