I don't thing it's OK to show our internals for users. I was confused to see that insert returns "bucket_id".

https://github.com/tarantool/crud#insert

Actually I can do nothing with such information. For performance reasons we could return this field when work with raw tuple, but I see no reasons to preserve it in result object.

Verify (and fix if it's not so) cartridge roles (and crud as separate module) supports hot-reload.

See also tarantool/cartridge#1105

Given this space:

local customers = box.schema.space.create(
        'customers',
        {
            format = {
                {name = 'uuid', type = 'uuid'},
                {name = 'bucket_id', type = 'unsigned'},
                {name = 'name', type = 'string'},
                {name = 'tags', type = 'array'},
            },
            if_not_exists = true,
        }
    )

    customers:create_index('uuid', {
        parts = {'uuid'},
        if_not_exists = true,
    })

    customers:create_index('bucket_id', {
        parts = {'bucket_id'},
        unique = false,
        if_not_exists = true,
    })

It is impossible to perform crud.select('customers'), since it fails with an error like

E> LuajitError: /app/.rocks/share/tarantool/crud/select/comparators.lua:53: attempt to compare 'struct tt_uuid' with 'struct tt_uuid'

If there's custom shard group defined, operations will break at code like vshard.router.routeall().
It could be fixed by passing shard group name in opts argument and using first group in list if it's not provided/not found.

Even Tarantool 1.10 supports value extraction via jsonpath. So we could patch our filter generator and add jsonpath's support of jsonpath conditions.

Let's add a method called crud.get_routers() that will return a list of connection strings to all existing routers. It will allow connectors to request this list and use it to balance requests. This way, the users will have to only put a couple of addresses to the connect() call, and the connector will then discover the rest.

RFC

Select plan is a table that contains all information we need to know to perform select.
It's created based on user-specified conditions and such options as first and after.
Select plan is created on both storage and router.

Now select plan looks like this:

plan = {
    scanner = {
        space_name = scan_space_name,
        index_id = scan_index.id,
        index_name = scan_index.name,
        iter = scan_iter,
        value = scan_value,
        condition_num = scan_condition_num,
        operator = scan_operator, -- redundant
        limit = scan_limit,
        after_tuple = scan_after_tuple,
    },
    filter_conditions = { ... }, -- used by storage,
    is_scan_by_full_sharding_key_eq, -- used by router
}

I don't like current plan structure for some reasons:

• we compute filter_conditions on router, but it's used only by storage;
• storage knows everything about the whole select query that is bad (I would like storage to be very simple - it just selects specified number of tuples (batch_size, not limit of first) from specified space with specified iterator and filters it by filter conditions;
• storage- and router-side logic has many intersections that causes changing both router and storage code to implement new features (which are often affects router logic).

I suggest next approach:

• select plan is separated into three parts: common part for router and storage;
• "common" part is computed by router and is passed to storage on the call;
• storage uses it to compute filter_conditions
• computing after_tuple for storage is a router responsibility (in fact, it's a cursor for all calls except the first one, but now storage selects one value between scan_value and after_tuple on every call);
• storage accepts batch_size option that specifies the count of tuples to be returned;
• router uses "common" part to check is_scan_by_full_sharding_key_eq and only_one_value_is_needed conditions;
• first option is processed by router (described below)

In details:

• user calls crud.select(space_name, conditions, { first = ..., after = ... })

• router computes plan based on conditions:

  plan = {
       conditions = ...,
       space_name = ...,
       index_id = ...,
       scan_value = ...,
       scan_condition_num = ...,
       iter = ..., -- based on scan condition
  }

• router applies specified first:

  • if first is negative, plan.iter is inversed;
  • limit (the whole number of tuples to select) is set to first absolute value;
  • if first is negative, reversed flag is set to true

• router checks if only one value is needed and sets limit to 1 and plan.iter to REQ;

• router checks if select by fully specified sharding key is performed and decides if only one storage should be called;

So, router has the final version of the plan and some values that are used only by router.

• router computes batch_size on each call (min(batch_size, limit - count_tuples))
• router calls select on storages with (plan, { batch_size = batch_size }) and aggregates received tuples.

Use checks 3.1.0 that provides tuple checker.
Enable checks only in dev.

The first part of #53

In this part we will support partial results for all operations except select/pairs.
This is much more simple for "1-tuple" operations than for "many-tuples" operations when we need to merge results on router, so I sure that it should be done in two patches.

Use case: customer wants to filter out tuples by values of an enum (e.g. filter orders by status).

Proposed API variants:

a) New operator '[]' (or any other signature)

    crud.select('test_space', {{'[]', 'status', {"IN_PROGRESS", "NEW"}}})

b) Extend operator '=' for taking multiple values

    crud.select('test_space', {{'=', 'status', {"IN_PROGRESS", "NEW"}}})

This will be equivalent to having equal conditions combined with OR.

This operator should be available in all operations taking the conditions.

Consider a space with a field of type 'map' and a secondary index over a nested field:

appeals = box.schema.create_space('appeals', { format = {
    { name = 'session_id', type = 'uuid', is_nullable = false },
    { name = 'agent', type = 'map', is_nullable = true },
}})
appeals:create_index('primary', {
    parts = { 'session_id' },
    if_not_exists = true
})
appeals:create_index('agent_id', {
    parts = { { field = 'agent', type = 'string', path = '.agent_id' } },
    unique = false,
    if_not_exists = true
})

When trying to select tuples directly via box API on the storages, it works as expected:

When trying to select via CRUD API on a router, the request hangs:

crud.select('appeals', { { '==', 'agent_id', 'denis.tulupov' } }, { use_tomap = true })

The second and complex part of #53

"Replace" operation is more effective than "insert" for batch operations and is used in high-level connectors like Spring Data connector.

Upsert operation is a more effective counterpart for the "update" operation for the same purposes.

Both operations are supported in the box API, so they should exist for closer compatibility with this API.

After #46 an ability to specify bucket_id was added, but some problems with API still remain.

Now CRUD will try to calculate bucket_id using PK and return an empty resultset.

There are such possible options:
1 do clusterwide (map-reduce) query in such case;
2 add a note to README that if sharding is done by custom field and bucket_id is not specified by user then crud.get will not work as expected.
3 detect that space is not sharded by PK and throw an exception if bucket_id is not specified

I'd love to see more complex examples regarding select.
How do I do paginated select easily? Snippet would be lovely
How do I select using composite index?
How do I perform select using partial key (for composite indexes)?
Can I use luafun (filter/take/_while/etc) with pairs? (An example would be lovely as well)

Currently, as far as I understand, we create fibers for every request, no matter if it's necessary.

See: https://github.com/tarantool/crud/blob/master/crud/common/call.lua#L88-L94

This is not required when you do a call to single storage. Creating fibers takes time, as well as communication through channels. This is only needed when you do select with map/reduce.

Sometimes we need to truncate all data in all spaces.

local ok, err = crud.truncate(space_name, opts)

where:

• space_name (string) - name of the space
• opts:
  • timeout (?number) - vshard.call timeout (in seconds)

This is from @Gerold103 feedback.

Be careful when splitting vshard operations in 2: getting a replicaset and sending operation to the replicaset.
Inside, vshard performs bucket pinning to make sure that the bucket doesn't migrate anywhere while the operation is being processed. When you split the operation into 2 stages, it is possible that the bucket will migrate between them.

Let me demonstrate the problem in the current codebase:

In this case, you probably should just do vshard.callrw().

Suppose we have a sharded space with customer profiles and want to access them by IPROTO (for example, from a connector). In this case, we have to write a lot of boilerplate code like that:

...
local function profile_storage_select(conditions)
    return crud.select('profiles', conditions)
end

local function profile_storage_get(id)
    return crud.get('profiles', id)
end

local function profile_storage_insert(tuple)
    return crud.insert('profiles', tuple)
end
...
local function init(opts)
    if opts.is_master then
        box.schema.func.create('profile_storage_insert', {if_not_exists = true})
        box.schema.func.create('profile_storage_get', {if_not_exists = true})
        box.schema.func.create('profile_storage_select', {if_not_exists = true})
        ....
    end

    rawset(_G, 'profile_storage_insert', profile_storage_insert)
    rawset(_G, 'profile_storage_get', profile_storage_get)
    rawset(_G, 'profile_storage_select', profile_storage_select)
    ...

    return true
end

return {
    role_name = 'app.roles.api_storage',
    init = init,
    utils = {
        profile_storage_get = profile_storage_get,
        profile_storage_insert = profile_storage_insert,
        profile_storage_select = profile_storage_select,
        ...
    },
    dependencies = {
        ....
    }
}

And we need to do this for each space we want to access using CRUD.

I suggest adding a small module, which encapsulates all this boilerplate logic so that all we need to write is something like

local proxy = require('proxy').new()

local function init(opts)
    if opts.is_master then
        proxy.register('profiles', 'customers')
    ...
    end
    ...
    return true
end

return {
    role_name = 'app.roles.api_storage',
    init = init,
    utils = table.merge(proxy.api(), {
        ...
    }),
    dependencies = {
        ....
    }
}

This module can be automatically enabled in a special "proxy" role when ddl is installed.

There are many scenarios for filtering, mapping and reducing the returned data and the most comfortable way of doing it is using the LuaFun library. Simple Lua iterator is not enough.

Consider the case

primary key:

• x: integer
• y: integer

put objects:

• [0, 1]
• [0, 2]
• [1, 2]

selects:

• box.select({0}) — returns [0, 1], [0, 2]
• crud.select({'=', 'primary', {0}}) — returns [0, 1]

tests related #88

e5eb7ca#diff-8afead6b3cb6ef4117be97c1bf3ac723a454b7ddff094d300e42c20431b37bf0R42

A problem is following:

box.cfg{}
box.schema.space.create('space')
box.space.space:create_index('i1')
box.space.space:create_index('i2')
box.space.space:create_index('i3')
box.space.space:create_index('i4')
box.space.space.index.i2:drop()
box.space.space.index.i3:drop()

tarantool> #box.space.space.index  -- Ooops
---
- 0
...
tarantool> box.space.space.index[0]
---
- unique: true
  parts:
  - type: unsigned
    is_nullable: false
    fieldno: 1
  type: TREE
  id: 0
  space_id: 512
  name: i1
...

tarantool> box.space.space.index[1]
---
- null
...

tarantool> box.space.space.index[2]
---
- null
...

tarantool> box.space.space.index[3]
---
- unique: true
  parts:
  - type: unsigned
    is_nullable: false
    fieldno: 1
  id: 3
  type: TREE
  space_id: 512
  name: i4
...

Seems crud should consider it. See table.maxn

Use cases:
a) customer wants to see counts of templates in message template catalogs in application UI. The templates and catalogs are stored in vshard, and operated via CRUD through a connector.
b) pagination -- we need to know the total amount of results for displaying it in the UI.

The count() method must accept conditions.

Proposed API variant:

    local count = crud.count({{'=', 'status', 'NEW'}})

• Tuple comparator should use scan key merged w/ primary key
• Comparator operator should be one of > or < for tuples sorting
• Seems that every select by equal condition is interpreted as a select with a fully specified sharding key

Add support for getting only a subset of fields. Like this:

crud.get('customers', 1, {fields={'id', 'name'}})
---
- metadata:
  - {'name': 'id', 'type': 'unsigned'}
  - {'name': 'name', 'type': 'string'}
  rows:
  - [1, 'Elizabeth']
...

Write test branch for 2.4+ Tarantool versions until tarantool/tarantool#5273 and tarantool/tarantool#5384 are not implemented

Calling a select with a condition in which the type of the column and the type of the value do not match causes the task to freeze and does not return any result.

Steps to reproduce the issue

Call select
require('crud').select('space_name', {{'=', 'id', 'not_number'}})

What's the expected result?

error message

What's the actual result?

does not respond

Additional details

cartridge log:
myapp.router | 2020-10-14 12:10:47.946 [757] main/132/lua replicaset.lua:257 E> Exception during calling '__call' on 'localhost:3304(admin@localhost:3304)': Supplied key type of part 0 does not match index part type: expected unsigned myapp.router | 2020-10-14 12:10:47.946 [757] main/131/lua utils.c:1028 E> LuajitError: /opt/myapp/crud/select/iterator.lua:104: attempt to call method 'new' (a nil value)

Interactive transactions have made it into the core recently. They allow us to open a transaction and do selects/updates with yield. I suggest that we add support for them to crud.

Unfortunately, we won't get multi-node transactions, but since we have an understanding of which shard the operation will go to, we can still allow transactional operations if they touch only one storage.

This should be pretty easy to implement: if we open a transaction, record which node the first operation goes to, then make sure all other operations in the transaction go to the same node. If not, abort the operation or the whole transaction.

I suspect the reason this has leaked into the CI config, but CRUD API should work with open-source tarantool version, so we should rather get rid of Enterprise specifics here

# cat /etc/centos-release
CentOS Linux release 7.9.2009 (Core)

# tarantool -version
Tarantool 2.5.2-60-g46d2ddf
Target: Linux-x86_64-RelWithDebInfo

dependencies = {
'tarantool',
'lua >= 5.1',
'checks == 3.0.1-1',
'cartridge == 2.3.0-1',
'metrics == 0.5.0-1',
'cartridge-cli-extensions == 1.0.0-1',
'crud',
}

crud.get("msisdn_statuses", "123").rows[1]:tomap()
---
- error: '[string "return crud.get("msisdn_statuses", "123").row..."]:1: attempt to
    call method ''tomap'' (a nil value)'

Please add functionality like for box.tuple https://www.tarantool.io/ru/doc/latest/reference/reference_lua/box_tuple/#box-tuple-tomap

It's not clear that CRUD returns onlyl DEFAULT_BATCH_SIZE = 100 records by default.
Please add this and other SIGNIFICANT defaults to API doc.

Failed jobs:

• https://github.com/tarantool/crud/runs/1159713619?check_suite_focus=true
• https://github.com/tarantool/crud/runs/1155584650

Expected:

crud.pairs() returns empty iterator (next() returns nil)

Actual:

crud.pairs() return non-empty iterator with one nil value

Steps to reproduce:

1. Create an empty sharded space
2. Call crud.pairs() on the router in a for loop and check for the elements returned

    for _, record in crud.pairs('test_space', {use_tomap = true}) do log.info(record) end

Now this tests are flaky and non-informative.

Steps to reproduce:

Original issue: see https://github.com/vrogach2020/mdm-sandbox/pull/1

Given the following setup: a Cartridge application + tarantool/crud + tarantool/migrations + integration tests in luatest, an integration test trying to call crud.insert('test', {...}) fails with error Failed to insert test, error: Insert: Space "test" doesn't exist, although the space is initialized from migrations and does exist at the time of call, which is checked beforehand with a code like:

--wait until migrations are applied
    for _, server in pairs(helper.cluster.servers) do
        --spaces may be created with a slight delay on replicas
        helper.cluster:retrying({ timeout = 1 }, function()
            t.assert_not(server.net_box:eval('return box.space.test == nil'), server.alias)
        end)
    end

    helper.cluster:wait_until_healthy()

CRUD checks the space existence before performing operations on it using internal vshard connections, the simplified code looks like :

_, replicaset = next(vshard.router.routeall())
replicaset.master.conn.space[space_name]

Investigated by @rosik:
Vshard connection is established before migrations, and until the first insert, there are no other communications. Thus crud uses outdated schema and the request fails.

The suggested W/A is explicitly calling ping() for forcing schema update:

helper.cluster.main_server.net_box:eval([[
        local vshard = require('vshard')
        for _, r in pairs(vshard.router.routeall()) do
            r.master.conn:ping()
        end
    ]])

Actual result

CRUD may fail to work with spaces if vshard schema in internal connections is outdated, returning a fake "space doesn't exist" error

Expected result

CRUD doesn't fail to work with spaces when spaces actually exist

Proposed solution

1. @racktear: do not check for space existence at all.
   May lead to errors when attempting to use the space format/metadata.

2. Do not use vshard connections for checking the space existence.
   Probably requires the cluster DDL to be implemented

• Setup cartridge cluster with failover
• Setup separated crud-router and crud-storage
• Make a write load
• Switch master under load
• One time

LuajitError: ...rk/starwars/.rocks/share/tarantool/crud/common/utils.lua:35: attempt to index field 'master' (a nil value)

I have a case when field used to calculate bucket_id is not stored (it is not a part of primary key). Therefore, I'd like to pass bucket_id explicitly to crud.insert and, probably, crud.select forcibly. I am totally ok that crud.get will not work for such cases.

API proposal:
crud.insert({bucket_id,primary_key,secondary_key,payload}, {external_bucket_id=true})bucket_id (possibly via opts.external_bucket_id or smth like that)
crud.select({'secondary_key','=',42},{bucket_id=1})

Seems it's quite complex task - multikey index support for several reasons:

• Tarantool doesn't provide user-friendly ways to merge tuples from multikey indexes (see tarantool/tarantool#5270)
• Multikey index could return single tuple several times that could confuse users and we don't have "distinct option"
• Usage of multikey indexes requires FFI magic with Tarantool tree iterator - and it's different in different Tarantool versions
• May be it will be great to support "ALL IN" or "ANY IN" operators for multikey indexes (e.g. I have indexed array [1, 2, 3]. If I specify {"> ANY", 'array', 2} - it's true, because "3 > 2" matches, but {"> ALL", 'array', 2} because "2 > 2" - false and "1 > 2" - false)

It is possible that space format changes during the query (for example, during insert). Then, the user will get format mismatch error.

There are 2 primary cases we need to consider.

Format changes when inserting objects

In this case, crud.insert_object() should probably re-pack the object and retry, if the schema is still compatible with the object.

Format changes when inserting tuples

This can happen, for example, when the user adds a column on the right. Then there are 2 more cases:

• The column is nullable. In this case, we may want to re-pack and treat this column as null in our object. Though, I'm not that certain about it.
• The column is not nullable, or the schema is incompatible in other significant ways. In this case, we need to return an error.

Returning errors

When we return errors about format mismatch, we need to include a specific error type, that will allow connectors to make a decision whether they should retry or not. In many cases, connectors will be written in a way when they accept objects of their host programming language. If this is true, they may want to re-pack the tuple on their side and retry.

Use case: customer wants to update a single field value in a large sharded space with some conditions. For example, change statuses of all orders with status "PAYMENT_RECEIVED" to "CLOSED".

Proposed API variants:
a) Allow update() method to receive conditions instead of key parts:

    crud.update('test_space', {{'=', 'status', 'PAYMENT_RECEIVED'}}, {{'=', 'status', 'CLOSED'}})

b) Add a new method updateall() (the method name may be different):

    crud.updateall('test_space', {{'=', 'status', 'PAYMENT_RECEIVED'}}, {{'=', 'status', 'CLOSED'}})

The same variants may be used for the upsert() operation.

Found by customer (again).

Expected behavior

When adding a field via update() to the end of the tuple, if there are nullable fields in between which are not present yet, the operation fails with an error like Field ''statistics'' was not found in the tuple.

Actual behavior

Such operation completes successfully, as in the case if the user adds the absent intermediate fields.

Additional notes

Related to tarantool/tarantool#3378 -- already affects SQL and there are no forecasts about its destiny. So, it is possible to add a w/a on the CRUD module level, for not multiplying the clusterfuck effect of the bug.

In Tarantool 1.10 conn.space contains collation ID instead of collation
Selecting by field w/ collations isn't tested properly now

Use case: data models are often organized in such a way that some aggregates are built using LEFT JOINs (speaking of SQL syntax). Consider two cases:

1. a customer wants to select all accounts with theirs billing information. Although one way to build such aggregates is de-normalizing the data and saving it in one space, some linked data may be used and changed independently, but linked to one id and therefore shared by the same key. Let's call this "local joins", thus having in mind that the possible "join" will be performed on a storage node with data from one bucket.
2. a customer wants to select all orders in a special status for all accounts with a positive balance. Both spaces have the account_id field but are sharded by a different key. This looks like a more general case, although it can be shrunk to a simple pattern of operations: local joins with filtration + merge of the results with joining the other spaces + final filtration. Of course, in the real world, there will be a lot of optimizations, but a simple "straight" solution will already save a lot of boilerplate code for customers and allow the support for joins in Cartridge connectors.

The proposed API variants:
a) The one proposed by Dmitry -- https://github.com/dimoffon/vshard-cluster-api#joins. Although it looks like a general case, it doesn't specify the kind of join and looks too SQL-ish to my taste. Also implementing the partial cases like the general-purpose API may be both a good way and not so good one.
b) More flexible variation of the syntax:

    crud.join('accounts', {{'>', 'balance', 0}}):left_join('orders', 'account_id', {{'=', 'status', 'NEW'}}):select({limit = -10})

A more complex case:

    crud.join('accounts', {{'>', 'balance', 0}}):left_join('orders', {'accounts.id', 'account_id'}, {{'=', 'status', 'NEW'}}):left_join('payments', {'accounts.id', 'account_id'}, {{'=', 'status', 'CLOSED'}}):select({limit = -10})

The set of conditions on the first space will determine the used indexes, as for the select() operation.

Now the results of the operations are returned as maps (Lua tables):

crud.insert('customers', {
    id = 1, name = 'Elizabeth', age = 23,
})
---
- bucket_id: 7614
  age: 23
  name: Elizabeth
  id: 1
...

If the result is further used in some connectors, the tuple-to-map conversion may be unnecessary, especially for the purpose of partial compliance with the box API, which has an option for returning maps and returns tuples by default.

Proposal: implement an option "tuples_as_map" with default value "true", which may be passed along with the options of each operation. In case if this option is set to "false", the tuple-to-map conversion is not performed and the result tuples are returned as-is.

Currently, the 'after' parameter in select supports only maps (objects). It should be possible to pass tuple into this parameter.

Possible variants:

a) Automatic checking if the passed object has number keys starting from 1 (it is effectively a tuple)
b) Separate the option into two: after_tuple and after_object

• Rename limit -> first
• Support negative values of first:
  • For {1, 2, 3, 4, 5, 6} select w/ options {after = 5, first = -2} returns {3, 4} (XXX: how should it work for pairs?)
  • Deny to use negative first w/o after

Call select with conditions by primary index returns error:

SelectError: Failed to plan select: SelectPlanError: Passed bad conditions: ValidateConditionsError: No field or index "primary" found

Use DDL to define space schema in README in tests.