Guys,
QueryEntity is already too complex. We will deprecate it soon in favor of
brand new SQL API. No need to design FieldConstraint or something similar
at the moment, let's just stick to "Set<String> notNullFields".
On Fri, Jul 21, 2017 at 7:08 PM, Sergey Chugunov <sergey.chugu...@gmail.com>
wrote:
> Sergey,
>
> It may be a good idea to distinguish between field constraints (like "not
> null" one) which can be applied to only one field; and more complex
> constraints that involves more than one field.
>
> In case of field constraints it is better to simplify our model and allow
> only one field to appear inside constraint entity class.
>
> Something like this:
> class QueryEntity {
> ...
>
> /** All constraints to be enforced on this QueryEntity. */
> Set<FieldConstraint> fieldConstraints;
> }
>
> /** Describes all constraints that affect the field. */
> class FieldConstraint {
> /** Field name to be examined against all its constraints. */
> String fieldName;
>
> /** Indicates whether check for null is required. */
> boolean notNull;
>
> ... here we would add more flags corresponding to other constraints ...
> }
>
> This model has a flaw that if we want to enforce the same constraint on
> many fields we must define FieldConstraint entity for each of these fields.
> But it has its advantages too: first of all, its simplicity and therefore
> obvious usage patterns; and easy to implement validation.
>
> Thanks,
> Sergey Ch.
>
>
> On Fri, Jul 21, 2017 at 6:43 PM, Pavel Tupitsyn <ptupit...@apache.org>
> wrote:
>
> > 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
> > > >>
> > > >>
> > >
> >
>
