[
https://issues.apache.org/jira/browse/IGNITE-4575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16012378#comment-16012378
]
Vladimir Ozerov commented on IGNITE-4575:
-----------------------------------------
[~skalashnikov], my comments:
1) {{BinaryClassDescriptor.enumMap}} - I am not sure this is correct place to
store enum map, because it essentially duplicate to what is stored in metadata.
I walked through it's usages, and it is only used in two places immediately
after descriptor creation, so it is better to remove this field;
2) {{BinaryEnumObjectImpl.ctor}} - {{arr\[0\] ==
GridBinaryMarshaller.BINARY_ENUM}} looks wrong to me; you should pass only
unwrapped array here, or array+offset, which points to start of real enum. See
{{BinaryObjectImpl}} where the same idea is utilized.
3) {{BinaryEnumObjectImpl.enumName}} - as this method might be on hot paths, it
should generate little to no garbage and be as lightweight as possible; it is
better to expose some more internals to avoid {{BinaryMetadata -> BinaryType}}
conversion.
4) {{BinaryEnumObjectImpl.enumName}} - IDEA shows me a warning about possible
NPE in case type is null;
5) {{BinaryTypeImpl}} - cached values should be in {{BinaryMetadata}}, which is
created only once, not in {{BinaryType}}, which can be created multiple times
in different places
6) {{BinaryMetadata.VERSION}} - let's change it to byte, so that it will take
only 1 byte instead of 4 (it will not change frequently)
7) {{BinaryMetadata.getEnumOrdinalByName}} - check for empty name is invalid
here; empty or null names are not allowed, so we must thrown an exception, but
not here, but rather somewhere in {{BinaryEnumObjectImpl}}.
8) {{BinaryWriteMode}} - I think {{BINARY_ENUM}} must be mapped to {{ENUM}}
just like {{BinaryObjectImpl}} mapped to {{OBJ}} (see
{{BINARY_OBJ(GridBinaryMarshaller.OBJ)}})
9) {{BinaryUtils.mergeMetadata}} - {{changed}} flag should be set to {{true}}
only in case something really changed
10) {{BinaryWriterExImpl.doWriteBinaryEnum}} - if {{typeId !=
GridBinaryMarshaller.UNREGISTERED_TYPE_ID}} which is the most common case, we
can unsafely allocate 9 bytes intead of 5, and write ordinal in unsafe manner
as well
> Implement in Ignite wrapper for enums based on H2 user value type
> -----------------------------------------------------------------
>
> Key: IGNITE-4575
> URL: https://issues.apache.org/jira/browse/IGNITE-4575
> Project: Ignite
> Issue Type: Task
> Components: sql
> Reporter: Alexander Paschenko
> Assignee: Sergey Kalashnikov
> Labels: important
> Fix For: 2.1
>
>
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
