Sergey, looks good to me!
I thought of something a bit different, where there is a base class
and each constraint type inherits it, but your design is actually better.
Pavel
On Fri, Jul 21, 2017 at 6:38 PM, Sergey Kalashnikov <zkilling...@gmail.com>
wrote:
> Hi Pavel,
>
> Good point! What if we make it the following way?
>
> class QueryEntity {
> ...
>
> /** All constraints to be enforced on this QueryEntity. */
> Set<QueryConstraint> constraint;
> }
>
> /** Describes constraints that affect one or more fields. */
> class QueryConstraint {
> /** Names of fields to be checked. */
> Set<String> fields;
>
> /** Indicates whether check for null is required. */
> boolean notNull;
>
> ... here we would add more flags corresponding to other constraints ...
> }
>
> --
> Best Regards,
> Sergey
>
>
> 2017-07-21 10:54 GMT+03:00 Pavel Tupitsyn <ptupit...@apache.org>:
> > Hi Sergey,
> >
> > This one looks not very good to me:
> >
> >> class QueryEntity {
> >> ...
> >> Set<String> notNullFields;
> >> }
> >
> > What if there are more constraints in future? UNIQUE, CHECK, etc etc?
> >
> > Instead we could do something like
> >
> > Set<QueryConstraint> constraints;
> >
> > Which is easily extendable in future.
> >
> > Thoughts?
> >
> > Pavel
> >
> > On Thu, Jul 20, 2017 at 11:17 PM, Denis Magda <dma...@apache.org> wrote:
> >
> >> Hi Sergey,
> >>
> >> That will be an excellent addition to DDL features set.
> >>
> >> The proposed changes look good to from the public API standpoint.
> >>
> >> Alexander P., Sergi, Vovan, please share your thoughts.
> >>
> >> —
> >> Denis
> >>
> >> > On Jul 20, 2017, at 12:27 PM, Sergey Kalashnikov <
> zkilling...@gmail.com>
> >> wrote:
> >> >
> >> > Hi Igniters,
> >> >
> >> > I am working on the ticket
> >> > https://issues.apache.org/jira/browse/IGNITE-5648, which purpose is
> to
> >> > add support for NOT NULL constraint for any fields of key or value
> >> > stored in Ignite cache.
> >> >
> >> > I would appreciate your comments on the design approach I have
> described
> >> below.
> >> >
> >> > In my opinion, such checks should be enforced both by SQL DML and
> >> > cache API to be consistent.
> >> >
> >> > Here is my analysis of what needs to be done.
> >> >
> >> > 1. First, the SQL CREATE table will not throw exception anymore
> >> > whenever it encounters a column with "not null" modifier.
> >> > Instead, the resulting QueryEntity will now indicate which fields have
> >> > such modifier.
> >> >
> >> > The proposed way of doing this is the following:
> >> > class QueryEntity {
> >> > ...
> >> > Set<String> notNullFields;
> >> > }
> >> >
> >> > Since QueryEntity is a part of public api, it becomes possible to
> >> > configure this constraint not only via DDL CREATE TABLE.
> >> >
> >> > 2. Then we need a special method on GridQueryProcessor that for the
> >> > given cacheName, key and value would perform the following things:
> >> > - Get a GridQueryTypeDescriptor that corresponds to given value type;
> >> > - Delegate that GridQueryTypeDescriptor a task to validate given key
> and
> >> value;
> >> > - Type descriptor would itself delegate the validation to its
> >> > GridQueryProperties that have "not null" constraint.
> >> >
> >> > 3. To enforce the constraints, the validation method should be called
> >> > - In GridNearAtomicSingleUpdateFuture.mapSingleUpdate() and
> >> > GridNearAtomicUpdateFuture.mapUpdate() when operation is CREATE or
> >> > UPDATE.
> >> > That would cover putIfAbsent(), getAndPut(), getAndPutIfAbsent(),
> >> > replace(), getAndReplace(), putAll() operations on atomic cache.
> >> > And in GridNearTxLocal.enlistWriteEntry() when operation is CREATE or
> >> > UPDATE for the case of transactional cache.
> >> >
> >> > - Right after EntryProcessor.process() in
> >> > GridCacheMapEntry.AtomicCacheUpdateClosure.runEntryProcessor() as
> part
> >> > of invoke(), invokeAll() operations on atomic cache.
> >> > And in GridDhtTxPrepareFuture.onEntriesLocked() for the case of
> >> > transactional cache.
> >> >
> >> > 4. DML processor changes
> >> > The DMLStatementProcessor methods doInsert(), doUpdate(), doMerge()
> >> > must also incorporate such checks to avoid attempts
> >> > to insert values that are known to be rejected by cache.
> >> >
> >> > Thoughts?
> >> >
> >> > --
> >> > Best regards,
> >> > Sergey
> >>
> >>
>
