Valya,

Normally setters do not throw exceptions. It is perfectly fine to override
something. This is how 99% of our configuration works. In this case
exception is thrown because indexed types are converted to query entities
inside the setter, so the next call to this setter cannot understand what
to do.

As per QueryEntity, I do not think we should throw exceptions. This
constructor is merely a shortcut which extracts data from classes. It
appears perfectly valid to me to, for example, add another index or column
after that. Especially provided that we already have dynamic index
create/drop, and will have CREATE/ALTER/DROP table later.

Vladimir.

On Thu, Apr 20, 2017 at 10:17 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> Exception is thrown for safety if setIndexedTypes is called more than once.
> Otherwise second call will override the first one and Key1-Value1 will be
> ignored. The correct code should look like this:
>
> setIndexedTypes(Key1.class, Value1.class, Key2.class, Value2.class);
>
> Generally, I agree with this change, but with one minor concern. What will
> happen if I use new QueryEntity constructor (QueryEntity(Class keyCls,
> Class valCls)), and then use setters to modify this entity? I think we
> should throw an exception in this case.
>
> -Val
>
> On Thu, Apr 20, 2017 at 2:02 AM, Dmitriy Setrakyan <dsetrak...@apache.org>
> wrote:
>
> > Vladimir, I am not sure I understand the issue here. Why would the
> Example
> > 1 result in an exception?
> >
> > On Wed, Apr 19, 2017 at 1:22 PM, Vladimir Ozerov <voze...@gridgain.com>
> > wrote:
> >
> > > Igniters,
> > >
> > > I'd like to propose to deprecate infamous
> CacheConfiguration.setIndexedT
> > > ypes
> > > method. It has subtle semantics and clashes with
> > > CacheConfiguration.setQueryEntities method. Several examples.
> > >
> > > Example 1
> > > setIndexedTypes(Key1.class, Value1.class);
> > > setIndexedTypes(Key2.class, Value2.class);
> > > => exeption
> > >
> > > Example 2
> > > setIndexedTypes(Key1.class, Value1.class);
> > > setQueryEntitities(new QueryEntity(...));
> > > => all ok, two query entities
> > >
> > > Proposal:
> > > 1) Deprecate CacheConfiguration.setIndexedTypes
> > > 2) Add new constructor QueryEntity(Class keyCls, Class valCls)
> > > 3) Remove all non-obvious semanrics from
> > > CacheConfiguration.setQueryEntities method
> > > and make it plain setter.
> > >
> > > Thoughts?
> > >
> >
>

Reply via email to