Re: CachePojoStore data loading with binary marshaller

2017-04-13 Thread Dmitriy Setrakyan 
On Thu, Apr 13, 2017 at 12:16 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> It's not illegal in general, but it doesn't make sense for out of the box
> implementation of cache store.\
>

Ouch. I didn't realize that we deserialize in our own cache store. Any
reason for this behavior? Can this be fixed before the release?

Re: CachePojoStore data loading with binary marshaller

2017-04-13 Thread Valentin Kulichenko 
It's not illegal in general, but it doesn't make sense for out of the box
implementation of cache store.

-Val

On Wed, Apr 12, 2017 at 11:27 PM, Dmitriy Setrakyan 
wrote:

> On Wed, Apr 12, 2017 at 12:24 AM, Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
> > Hi Dmitry,
> >
> > I can't advise a lot on implementation details, but generally I don't
> think
> > that CacheJdbcPojoStore should ever work with POJO classes in case binary
> > marshaller is enabled. It should always work directly with binary objects
> > even if POJOs are on server classpath, and switch to POJOs only when
> other
> > marshaller is used.
> >
> > Makes sense?
> >
>
> Valentin, this makes sense from the performance stand point. However, why
> should it be illegal to deserialize a binary object into class, if the
> class it on the classpath?
>

Re: CachePojoStore data loading with binary marshaller

2017-04-12 Thread Dmitriy Setrakyan 
On Wed, Apr 12, 2017 at 12:24 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> Hi Dmitry,
>
> I can't advise a lot on implementation details, but generally I don't think
> that CacheJdbcPojoStore should ever work with POJO classes in case binary
> marshaller is enabled. It should always work directly with binary objects
> even if POJOs are on server classpath, and switch to POJOs only when other
> marshaller is used.
>
> Makes sense?
>

Valentin, this makes sense from the performance stand point. However, why
should it be illegal to deserialize a binary object into class, if the
class it on the classpath?

Re: CachePojoStore data loading with binary marshaller

2017-04-12 Thread Valentin Kulichenko 
Yes, that's my thinking as well.

-Val

On Wed, Apr 12, 2017 at 10:59 AM, Dmitriy Govorukhin <
dmitriy.govoruk...@gmail.com> wrote:

> Hi Valentin,
>
> Other word, are you agree with me? My point that we always must use binary
> representation if binary marshaller enable.
>
> On Wed, Apr 12, 2017 at 10:24 AM, Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
> > Hi Dmitry,
> >
> > I can't advise a lot on implementation details, but generally I don't
> think
> > that CacheJdbcPojoStore should ever work with POJO classes in case binary
> > marshaller is enabled. It should always work directly with binary objects
> > even if POJOs are on server classpath, and switch to POJOs only when
> other
> > marshaller is used.
> >
> > Makes sense?
> >
> > -Val
> >
> > On Tue, Apr 11, 2017 at 12:06 PM, Dmitriy Govorukhin <
> > dmitriy.govoruk...@gmail.com> wrote:
> >
> > > Hi all,
> > >
> > > I found problem related to CacheJdbcPojoStore. If try to load data,
> with
> > > enabled binary marshaller,
> > > error may occur. Problem in entryMappings (this map helps resolve
> stored
> > > type when data loading).
> > > If binary marshaller disable we just  put entry with java type in this
> > map,
> > > if enable we put java type + binary type.
> > >
> > > ..
> > >
> > >checkTypeConfiguration(cacheName, valKind, valType,
> > > type.getValueFields());
> > >
> > >entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> > type,
> > > keyKind, valKind, sqlEscapeAll));
> > >
> > >// Add one more binding to binary typeId for POJOs,
> > >// because object could be passed to store in binary format.
> > >if (binarySupported && keyKind == TypeKind.POJO) {
> > >keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
> > >
> > >valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
> > >
> > >entryMappings.put(keyTypeId, new EntryMapping(cacheName,
> dialect,
> > > type, TypeKind.BINARY, valKind, sqlEscapeAll));
> > > }
> > >
> > > ...
> > >
> > >
> > > I think it is wrong, because after we use this map like  iterator for
> > load,
> > > and final result dependent from which mapping will be first java or
> > binary.
> > >
> > > Collection processedKeyTypes = new HashSet<>();
> > >
> > > for (EntryMapping em : mappings.values()) {
> > > String keyType = em.keyType();
> > >
> > > if (processedKeyTypes.contains(keyType))
> > > continue;
> > >
> > > processedKeyTypes.add(keyType);
> > >
> > > .
> > >
> > > I think we must add only binary mapping for user pojo if binary
> > marshaller
> > > enable.
> > >
> >
>

Re: CachePojoStore data loading with binary marshaller

2017-04-12 Thread Dmitriy Govorukhin 
Hi Valentin,

Other word, are you agree with me? My point that we always must use binary
representation if binary marshaller enable.

On Wed, Apr 12, 2017 at 10:24 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> Hi Dmitry,
>
> I can't advise a lot on implementation details, but generally I don't think
> that CacheJdbcPojoStore should ever work with POJO classes in case binary
> marshaller is enabled. It should always work directly with binary objects
> even if POJOs are on server classpath, and switch to POJOs only when other
> marshaller is used.
>
> Makes sense?
>
> -Val
>
> On Tue, Apr 11, 2017 at 12:06 PM, Dmitriy Govorukhin <
> dmitriy.govoruk...@gmail.com> wrote:
>
> > Hi all,
> >
> > I found problem related to CacheJdbcPojoStore. If try to load data, with
> > enabled binary marshaller,
> > error may occur. Problem in entryMappings (this map helps resolve stored
> > type when data loading).
> > If binary marshaller disable we just  put entry with java type in this
> map,
> > if enable we put java type + binary type.
> >
> > ..
> >
> >checkTypeConfiguration(cacheName, valKind, valType,
> > type.getValueFields());
> >
> >entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> type,
> > keyKind, valKind, sqlEscapeAll));
> >
> >// Add one more binding to binary typeId for POJOs,
> >// because object could be passed to store in binary format.
> >if (binarySupported && keyKind == TypeKind.POJO) {
> >keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
> >
> >valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
> >
> >entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> > type, TypeKind.BINARY, valKind, sqlEscapeAll));
> > }
> >
> > ...
> >
> >
> > I think it is wrong, because after we use this map like  iterator for
> load,
> > and final result dependent from which mapping will be first java or
> binary.
> >
> > Collection processedKeyTypes = new HashSet<>();
> >
> > for (EntryMapping em : mappings.values()) {
> > String keyType = em.keyType();
> >
> > if (processedKeyTypes.contains(keyType))
> > continue;
> >
> > processedKeyTypes.add(keyType);
> >
> > .
> >
> > I think we must add only binary mapping for user pojo if binary
> marshaller
> > enable.
> >
>

CachePojoStore data loading with binary marshaller

2017-04-11 Thread Dmitriy Govorukhin 
Hi all,

I found problem related to CacheJdbcPojoStore. If try to load data, with
enabled binary marshaller,
error may occur. Problem in entryMappings (this map helps resolve stored
type when data loading).
If binary marshaller disable we just  put entry with java type in this map,
if enable we put java type + binary type.

..

   checkTypeConfiguration(cacheName, valKind, valType,
type.getValueFields());

   entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect, type,
keyKind, valKind, sqlEscapeAll));

   // Add one more binding to binary typeId for POJOs,
   // because object could be passed to store in binary format.
   if (binarySupported && keyKind == TypeKind.POJO) {
   keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);

   valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;

   entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
type, TypeKind.BINARY, valKind, sqlEscapeAll));
}

...


I think it is wrong, because after we use this map like  iterator for load,
and final result dependent from which mapping will be first java or binary.

Collection processedKeyTypes = new HashSet<>();

for (EntryMapping em : mappings.values()) {
String keyType = em.keyType();

if (processedKeyTypes.contains(keyType))
continue;

processedKeyTypes.add(keyType);

.

I think we must add only binary mapping for user pojo if binary marshaller
enable.