Igniters,

The initial implementation is ready for review.
To limit the PR size, I've only implemented insert and get operations.

https://issues.apache.org/jira/browse/IGNITE-14970
https://github.com/apache/ignite-3/pull/191

On Wed, Jul 7, 2021 at 1:56 PM Pavel Tupitsyn <ptupit...@apache.org> wrote:

> Thanks everyone, I've moved the proposal to Active status.
> Working on the implementation.
>
> On Tue, Jul 6, 2021 at 10:31 PM Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
>> The proposal looks good to me.
>>
>> -Val
>>
>> On Tue, Jul 6, 2021 at 2:24 AM Ivan Daschinsky <ivanda...@gmail.com>
>> wrote:
>>
>> > I suppose, that general idea is great. Some details are missing, but I
>> > suppose during implementation of clients we will add more details and
>> > redefine some parts.
>> >
>> > вт, 6 июл. 2021 г., 09:59 Pavel Tupitsyn <ptupit...@apache.org>:
>> >
>> > > Ivan, Val, and others - are there any open objections or questions?
>> > > Can we accept the proposal?
>> > >
>> > > On Mon, Jul 5, 2021 at 1:28 PM Pavel Tupitsyn <ptupit...@apache.org>
>> > > wrote:
>> > >
>> > > > Igniters,
>> > > >
>> > > > I've updated the IEP to support "Live Schema" [1] from IEP-54.
>> > > > Some operations now have schemaless variants, where tuples are
>> > serialized
>> > > > as maps (String -> val).
>> > > >
>> > > > [1]
>> > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/IGNITE/IEP-54%3A+Schema-first+Approach#IEP54:SchemafirstApproach-Dynamicschemaexpansion(Live-schema)
>> > > >
>> > > > On Thu, Jul 1, 2021 at 8:31 PM Ivan Daschinsky <ivanda...@gmail.com
>> >
>> > > > wrote:
>> > > >
>> > > >> Val, my understanding about it was exactly the same as yours, but,
>> > > again,
>> > > >> I
>> > > >> heard a different opinion.
>> > > >>
>> > > >> But nevertheless, binary protocol should not be about objects,
>> records
>> > > aka
>> > > >> tuples are the best varii, simple and powerful.
>> > > >>
>> > > >> As for me, I want to take part in implementing python and golang
>> thin
>> > > >> clients for ignite 3, so consider my remarks using this info. I am
>> not
>> > > >> just
>> > > >> a rude critic,
>> > > >> I am just interested in consice and universal binary prorocol
>> > > >> чт, 1 июл. 2021 г., 20:23 Valentin Kulichenko <
>> > > >> valentin.kuliche...@gmail.com
>> > > >> >:
>> > > >>
>> > > >> > Ivan,
>> > > >> >
>> > > >> > KV view does work over the tuples. Nested objects and arbitrary
>> > > >> structures
>> > > >> > can be stored as blobs. So if you need a basic KV cache, you can
>> > > always
>> > > >> > create a table with two blob fields - one for key and one for
>> value
>> > -
>> > > >> and
>> > > >> > store anything there.
>> > > >> >
>> > > >> > -Val
>> > > >> >
>> > > >> > On Thu, Jul 1, 2021 at 9:55 AM Ivan Daschinsky <
>> ivanda...@gmail.com
>> > >
>> > > >> > wrote:
>> > > >> >
>> > > >> > > Val, am I right, that kv view over the tuples is just simple
>> > mapping
>> > > >> from
>> > > >> > > POJO to tuple? No collections, no nested objects? I have
>> discussed
>> > > >> this
>> > > >> > in
>> > > >> > > private with Igor and Pavel and they told me different info.
>> > > >> > >
>> > > >> > > чт, 1 июл. 2021 г., 19:43 Valentin Kulichenko <
>> > > >> > > valentin.kuliche...@gmail.com
>> > > >> > > >:
>> > > >> > >
>> > > >> > > > Ivan,
>> > > >> > > >
>> > > >> > > > I was answering your question about the KV API. The API I
>> > provided
>> > > >> has
>> > > >> > > been
>> > > >> > > > discussed and agreed upon. One of the goals of the protocol
>> is
>> > to
>> > > >> > > implement
>> > > >> > > > this API, so it should give you a clear idea of what we're
>> > looking
>> > > >> for.
>> > > >> > > >
>> > > >> > > > Of course, I agree with you that the protocol should be
>> simple
>> > and
>> > > >> > > flexible
>> > > >> > > > enough to allow other implementations for different languages
>> > and
>> > > >> > > > platforms.
>> > > >> > > >
>> > > >> > > > -Val
>> > > >> > > >
>> > > >> > > > On Thu, Jul 1, 2021 at 9:38 AM Ivan Daschinsky <
>> > > ivanda...@gmail.com
>> > > >> >
>> > > >> > > > wrote:
>> > > >> > > >
>> > > >> > > > > Andrey, yep, you are right. This was just a quick idea. As
>> for
>> > > >> me, I
>> > > >> > > just
>> > > >> > > > > don't want to repeat the same problem with compactFooter in
>> > thin
>> > > >> > client
>> > > >> > > > api
>> > > >> > > > > of ignite 2.x.
>> > > >> > > > >
>> > > >> > > > > чт, 1 июл. 2021 г., 19:22 Andrey Mashenkov <
>> > > >> > andrey.mashen...@gmail.com
>> > > >> > > >:
>> > > >> > > > >
>> > > >> > > > > > >
>> > > >> > > > > > > I suppose that we should describe this more verbose and
>> > > >> > explicit. I
>> > > >> > > > > > > nevertheless suggest to also consider writing values
>> this
>> > > way:
>> > > >> > > > > > > - arr of fields names (if name is missed, corresponding
>> > > field
>> > > >> is
>> > > >> > > nil)
>> > > >> > > > > > > - arr of rows (row as array, length equal to fields
>> array)
>> > > >> > > > > >
>> > > >> > > > > >
>> > > >> > > > > > Ivan,
>> > > >> > > > > > I think GET and PUT operation parameters should be
>> > consistent.
>> > > >> > > > > > With PUT operation this way may be tricky.
>> > > >> > > > > >
>> > > >> > > > > > SQL INSERT operation (which is similar PUT operation)
>> > semantic
>> > > >> > allows
>> > > >> > > > > > skipping columns that have a default value.
>> > > >> > > > > > Assume we have smth like this:
>> > > >> > > > > >
>> > > >> > > > > > CREATE TABLE t1 (
>> > > >> > > > > >    'id' INT;
>> > > >> > > > > >    'colname' VARCHAR DEFAULT "abc";
>> > > >> > > > > > )
>> > > >> > > > > > INSERT INTO t1 VALUES(1)
>> > > >> > > > > >
>> > > >> > > > > > Actually, this will add a row (1, "abc")
>> > > >> > > > > >
>> > > >> > > > > > Your suggestion related to missed fields will not work
>> this
>> > > way
>> > > >> as
>> > > >> > it
>> > > >> > > > is
>> > > >> > > > > > impossible to distinct
>> > > >> > > > > > case with 'null' value from the case with a default
>> value.
>> > > >> > > > > >
>> > > >> > > > > >
>> > > >> > > > > > On Thu, Jul 1, 2021 at 5:51 PM Ivan Daschinsky <
>> > > >> > ivanda...@gmail.com>
>> > > >> > > > > > wrote:
>> > > >> > > > > >
>> > > >> > > > > > > > Here is the description of TUPLE_GET_ALL:
>> > > >> > > > > > > - UUID: table ID
>> > > >> > > > > > > - int: schema ID
>> > > >> > > > > > > - arr of arr: array of rows with values for all
>> columns in
>> > > >> given
>> > > >> > > > schema
>> > > >> > > > > > >
>> > > >> > > > > > > I suppose that we should describe this more verbose and
>> > > >> > explicit. I
>> > > >> > > > > > > nevertheless suggest to also consider writing values
>> this
>> > > way:
>> > > >> > > > > > > - arr of fields names (if name is missed, corresponding
>> > > field
>> > > >> is
>> > > >> > > nil)
>> > > >> > > > > > > - arr of rows (row as array, length equal to fields
>> array)
>> > > >> > > > > > >
>> > > >> > > > > > > It is quite simple and if we use str8 (it is more than
>> > > enough
>> > > >> for
>> > > >> > > any
>> > > >> > > > > > utf-8
>> > > >> > > > > > > reasonable field name), overhead will be negligible,
>> but
>> > > >> > > realization
>> > > >> > > > > of a
>> > > >> > > > > > > client will be way simpler
>> > > >> > > > > > >
>> > > >> > > > > > > чт, 1 июл. 2021 г., 16:57 Pavel Tupitsyn <
>> > > >> ptupit...@apache.org>:
>> > > >> > > > > > >
>> > > >> > > > > > > > > No it isn't, I have carefully read code and IEP, in
>> > your
>> > > >> code
>> > > >> > > you
>> > > >> > > > > > write
>> > > >> > > > > > > > > schema id in each tuple.
>> > > >> > > > > > > >
>> > > >> > > > > > > > There is no code for batch operations yet.
>> > > >> > > > > > > >
>> > > >> > > > > > > > Here is the description of TUPLE_GET_ALL:
>> > > >> > > > > > > > - UUID: table ID
>> > > >> > > > > > > > - int: schema ID
>> > > >> > > > > > > > - arr of arr: array of rows with values for all
>> columns
>> > in
>> > > >> > given
>> > > >> > > > > schema
>> > > >> > > > > > > > (nil when value is missing for a column)
>> > > >> > > > > > > >
>> > > >> > > > > > > > As you can see, schema ID is written once for all
>> rows.
>> > > >> > > > > > > > A row is just a set of values according to the
>> schema.
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > > Also, my biggest concern -- extra serde step. I
>> > suppose
>> > > we
>> > > >> > > should
>> > > >> > > > > > pass
>> > > >> > > > > > > > > bytearray to internal api, and use msgpack
>> throughout
>> > > all
>> > > >> > wire
>> > > >> > > > > > > protocols,
>> > > >> > > > > > > > > as tarantool does.
>> > > >> > > > > > > >
>> > > >> > > > > > > > I agree. But this was decided before in IEP-54, and
>> is
>> > out
>> > > >> of
>> > > >> > > scope
>> > > >> > > > > for
>> > > >> > > > > > > > current IEP.
>> > > >> > > > > > > > Would you like to start a separate thread to discuss
>> > this?
>> > > >> Or I
>> > > >> > > can
>> > > >> > > > > do
>> > > >> > > > > > > this
>> > > >> > > > > > > > a bit later.
>> > > >> > > > > > > >
>> > > >> > > > > > > > On Thu, Jul 1, 2021 at 4:41 PM Ivan Daschinsky <
>> > > >> > > > ivanda...@gmail.com>
>> > > >> > > > > > > > wrote:
>> > > >> > > > > > > >
>> > > >> > > > > > > > > > This is described in all operations that include
>> > > >> multiple
>> > > >> > > > tuples.
>> > > >> > > > > > > > > No it isn't, I have carefully read code and IEP, in
>> > your
>> > > >> code
>> > > >> > > you
>> > > >> > > > > > write
>> > > >> > > > > > > > > schema id in each tuple.
>> > > >> > > > > > > > >
>> > > >> > > > > > > > > Also, my biggest concern -- extra serde step. I
>> > suppose
>> > > we
>> > > >> > > should
>> > > >> > > > > > pass
>> > > >> > > > > > > > > bytearray to internal api, and use msgpack
>> throughout
>> > > all
>> > > >> > wire
>> > > >> > > > > > > protocols,
>> > > >> > > > > > > > > as tarantool does.
>> > > >> > > > > > > > >
>> > > >> > > > > > > > > чт, 1 июл. 2021 г., 16:15 Pavel Tupitsyn <
>> > > >> > ptupit...@apache.org
>> > > >> > > >:
>> > > >> > > > > > > > >
>> > > >> > > > > > > > > > Ivan,
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > >  that there is not neccesary to write schema
>> > > versions
>> > > >> in
>> > > >> > > each
>> > > >> > > > > row
>> > > >> > > > > > > > > > > in collectionof tuples
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > This is described in all operations that include
>> > > >> multiple
>> > > >> > > > tuples.
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > > it is not clear from your code (probably
>> > > >> > > > > > > > > > > mistake?) how differ key tuples and value
>> tuples
>> > > from
>> > > >> > each
>> > > >> > > > > other
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > Key tuples include only key columns. Key columns
>> > come
>> > > >> first
>> > > >> > > in
>> > > >> > > > > the
>> > > >> > > > > > > > > schema.
>> > > >> > > > > > > > > > Value tuples include all columns, key and value.
>> > Added
>> > > >> "Key
>> > > >> > > > > tuples"
>> > > >> > > > > > > > > > section.
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > > As for me, these excercises with schema's
>> doesn't
>> > > >> worth a
>> > > >> > > lot
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > I'll add a benchmark and we'll see.
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > On Thu, Jul 1, 2021 at 3:17 PM Ivan Daschinsky <
>> > > >> > > > > > ivanda...@gmail.com>
>> > > >> > > > > > > > > > wrote:
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > > > > I suppose, that there is not neccesary to write
>> > > schema
>> > > >> > > > versions
>> > > >> > > > > > in
>> > > >> > > > > > > > each
>> > > >> > > > > > > > > > row
>> > > >> > > > > > > > > > > in collectionof tuples. Also it is not clear
>> from
>> > > your
>> > > >> > code
>> > > >> > > > > > > (probably
>> > > >> > > > > > > > > > > mistake?) how differ key tuples and value
>> tuples
>> > > from
>> > > >> > each
>> > > >> > > > > other.
>> > > >> > > > > > > In
>> > > >> > > > > > > > > > > readTuple you always read full schema and check
>> > for
>> > > >> full
>> > > >> > > > > length.
>> > > >> > > > > > As
>> > > >> > > > > > > > for
>> > > >> > > > > > > > > > me,
>> > > >> > > > > > > > > > > these excercises with schema's doesn't worth a
>> > lot.
>> > > >> I.e.
>> > > >> > > > > postgres
>> > > >> > > > > > > > just
>> > > >> > > > > > > > > > > writes field names and then simpy rows with
>> data.
>> > > >> Saving
>> > > >> > > few
>> > > >> > > > > > bytes
>> > > >> > > > > > > > > > doesn't
>> > > >> > > > > > > > > > > make much deal. Btw, msgpack has special types
>> for
>> > > >> short
>> > > >> > > > > strings
>> > > >> > > > > > > > (i.e.
>> > > >> > > > > > > > > > > str8). It is much easier use it and write field
>> > name
>> > > >> as
>> > > >> > is.
>> > > >> > > > > > > > > > >
>> > > >> > > > > > > > > > > чт, 1 июл. 2021 г., 14:56 Pavel Tupitsyn <
>> > > >> > > > ptupit...@apache.org
>> > > >> > > > > >:
>> > > >> > > > > > > > > > >
>> > > >> > > > > > > > > > > > Ivan, tuple serialization section added to
>> the
>> > > IEP,
>> > > >> let
>> > > >> > > me
>> > > >> > > > > know
>> > > >> > > > > > > if
>> > > >> > > > > > > > it
>> > > >> > > > > > > > > > is
>> > > >> > > > > > > > > > > > clear enough.
>> > > >> > > > > > > > > > > >
>> > > >> > > > > > > > > > > > Thanks!
>> > > >> > > > > > > > > > > >
>> > > >> > > > > > > > > > > > On Thu, Jul 1, 2021 at 2:06 PM Ivan
>> Daschinsky <
>> > > >> > > > > > > > ivanda...@gmail.com>
>> > > >> > > > > > > > > > > > wrote:
>> > > >> > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > I can't find any description of tuple
>> > > >> serialization
>> > > >> > in
>> > > >> > > > IEP,
>> > > >> > > > > > > only
>> > > >> > > > > > > > in
>> > > >> > > > > > > > > > > code
>> > > >> > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > чт, 1 июл. 2021 г., 13:59 Pavel Tupitsyn <
>> > > >> > > > > > ptupit...@apache.org
>> > > >> > > > > > > >:
>> > > >> > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > Ivan,
>> > > >> > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > 0. The IEP is not in progress, it is
>> ready
>> > for
>> > > >> > review
>> > > >> > > > and
>> > > >> > > > > > > > > > discussion.
>> > > >> > > > > > > > > > > > > > 1. Tuple serialization is described in
>> the
>> > IEP
>> > > >> and
>> > > >> > > > > > > demonstrated
>> > > >> > > > > > > > > in
>> > > >> > > > > > > > > > > the
>> > > >> > > > > > > > > > > > > PoC
>> > > >> > > > > > > > > > > > > > (see ClientMessageHandler#readTuple),
>> let me
>> > > >> know
>> > > >> > if
>> > > >> > > > more
>> > > >> > > > > > > > details
>> > > >> > > > > > > > > > are
>> > > >> > > > > > > > > > > > > > required
>> > > >> > > > > > > > > > > > > > 2. Tuple schema serialization is
>> described
>> > in
>> > > >> > > > SCHEMAS_GET
>> > > >> > > > > > > > > section.
>> > > >> > > > > > > > > > > > Table
>> > > >> > > > > > > > > > > > > > schema (configuration) needs more
>> details,
>> > you
>> > > >> are
>> > > >> > > > right
>> > > >> > > > > -
>> > > >> > > > > > > I'll
>> > > >> > > > > > > > > add
>> > > >> > > > > > > > > > > > them.
>> > > >> > > > > > > > > > > > > > 3. This IEP is about tables (tuple-based)
>> > API
>> > > >> only,
>> > > >> > > > since
>> > > >> > > > > > it
>> > > >> > > > > > > is
>> > > >> > > > > > > > > the
>> > > >> > > > > > > > > > > > only
>> > > >> > > > > > > > > > > > > > API that we have right now, as noted in
>> > Risks
>> > > >> and
>> > > >> > > > > > > Assumptions.
>> > > >> > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > On Thu, Jul 1, 2021 at 1:53 PM Ivan
>> > > Daschinsky <
>> > > >> > > > > > > > > > ivanda...@gmail.com>
>> > > >> > > > > > > > > > > > > > wrote:
>> > > >> > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > > Also, is there any clear information
>> about
>> > > KV
>> > > >> > api?
>> > > >> > > Is
>> > > >> > > > > > there
>> > > >> > > > > > > > any
>> > > >> > > > > > > > > > > plan
>> > > >> > > > > > > > > > > > to
>> > > >> > > > > > > > > > > > > > > implement it? Or is there any proposal
>> > about
>> > > >> it?
>> > > >> > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > > чт, 1 июл. 2021 г., 13:51 Ivan
>> Daschinsky
>> > <
>> > > >> > > > > > > > ivanda...@gmail.com
>> > > >> > > > > > > > > >:
>> > > >> > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > > > Pavel, but IEP is in progress, isn't
>> it?
>> > > >> > > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > > > 1. There is not any information about
>> > > tuple
>> > > >> > > > > > > serialization.
>> > > >> > > > > > > > > And
>> > > >> > > > > > > > > > > > there
>> > > >> > > > > > > > > > > > > > > isn't
>> > > >> > > > > > > > > > > > > > > > a clear consensus about it.
>> > > >> > > > > > > > > > > > > > > > 2. There is not any information about
>> > > schrma
>> > > >> > > > > > > serialization
>> > > >> > > > > > > > > > > format.
>> > > >> > > > > > > > > > > > > And
>> > > >> > > > > > > > > > > > > > > > AFAIK, there isn't a clear consensus
>> > also.
>> > > >> > > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > > > чт, 1 июл. 2021 г., 13:26 Pavel
>> > Tupitsyn <
>> > > >> > > > > > > > > ptupit...@apache.org
>> > > >> > > > > > > > > > >:
>> > > >> > > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > > >> Igniters,
>> > > >> > > > > > > > > > > > > > > >>
>> > > >> > > > > > > > > > > > > > > >> Please review the IEP for thin
>> client
>> > > >> protocol
>> > > >> > > in
>> > > >> > > > > 3.0
>> > > >> > > > > > > [1].
>> > > >> > > > > > > > > > > > > > > >> PoC is in progress [2]
>> > > >> > > > > > > > > > > > > > > >>
>> > > >> > > > > > > > > > > > > > > >> [1]
>> > > >> > > > > > > > > > > > > > > >>
>> > > >> > > > > > > > > > > > > > > >>
>> > > >> > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > >
>> > > >> > > > > > > > > > > >
>> > > >> > > > > > > > > > >
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >> >
>> > > >>
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/IGNITE/IEP-76+Thin+Client+Protocol+for+Ignite+3.0
>> > > >> > > > > > > > > > > > > > > >>
>> > > >> > > > > > > > > > > > > > > >> [2]
>> > > >> > https://github.com/apache/ignite-3/pull/191
>> > > >> > > > > > > > > > > > > > > >>
>> > > >> > > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > > >
>> > > >> > > > > > > > > > > > >
>> > > >> > > > > > > > > > > >
>> > > >> > > > > > > > > > >
>> > > >> > > > > > > > > >
>> > > >> > > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > > >
>> > > >> > > > > >
>> > > >> > > > > > --
>> > > >> > > > > > Best regards,
>> > > >> > > > > > Andrey V. Mashenkov
>> > > >> > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >> >
>> > > >>
>> > > >
>> > >
>> >
>>
>

Reply via email to