[tarantool-patches] Re: [PATCH v3 1/4] box: an ability to disable CK constraints

[Nikita Pettik]

On 16 Sep 15:47, Kirill Shcherbatov wrote:
> Now it is possible to disable and enable ck constraints in LUA.
> This option is not persistent. All ck constraints are enabled
> by default when Tarantool is configured. Ck constraints checks
> are not performed during standard recovery, but performed during
> force_recovery - all conflicting tuples are skipped in case of
> ck_constraint conflict.

AFAIU, this patch adds only ability to turn on/off constraints;
it doesn't affect recovery process. What is more, it would be
nice to see any rationale for introducing such ability.

> box: an ability to disable CK constraints

Nit: commit subject really lacks a verb:
box: add an ability to disable CK constraints
 
> Part of #4244
> ---
> diff --git a/extra/exports b/extra/exports
> index 7b84a1452..a24ace4b8 100644
> --- a/extra/exports
> +++ b/extra/exports
> diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
> index 1cde27022..b422c2f3e 100644
> --- a/src/box/ck_constraint.c
> +++ b/src/box/ck_constraint.c
> @@ -28,6 +28,7 @@
>   * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   * SUCH DAMAGE.
>   */
> +#include "memtx_engine.h"

Seems to be redundant include.

>  #include "box/session.h"
>  #include "bind.h"
>  #include "ck_constraint.h"
>  }
>  	}
>  	return NULL;
>  }
> diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
> index f26f77a38..8fc2c5789 100644
> --- a/src/box/ck_constraint.h
> +++ b/src/box/ck_constraint.h
> @@ -214,6 +219,19 @@ struct ck_constraint *
>  space_ck_constraint_by_name(struct space *space, const char *name,
>  			    uint32_t name_len);
>  
> +/**
> + * Find check constraint object in a given space by a given name
> + * and change it's is_enabled state.

Nit: it's -> its.

> + * @param space The space to lookup check constraint.
> + * @param name The check constraint name.
> + * @param new_state The new is_enabled state for ck constraint.
> + * @return 0 if ck constraint is successfully found and new
> + *         is_enabled value is set, -1 otherwise.
> + */
> +int
> +space_ck_constraint_set_state(struct space *space, const char *name,
> +			      bool new_state);
> +
>  #if defined(__cplusplus)
>  } /* extern "C" { */
>  #endif
> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
> index d0a7e7815..2c686e818 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -205,6 +205,9 @@ lbox_push_ck_constraint(struct lua_State *L, struct space *space, int i)
>  		lua_pushstring(L, ck_constraint->def->expr_str);
>  		lua_setfield(L, -2, "expr");
>  
> +		lua_pushboolean(L, ck_constraint->is_enabled);
> +		lua_setfield(L, -2, "is_enabled");
> +
>  		lua_setfield(L, -2, ck_constraint->def->name);
>  	}
>  	lua_pop(L, 1);
> diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
> index eb11346c1..b78034217 100644
> --- a/src/box/memtx_engine.c
> +++ b/src/box/memtx_engine.c
> @@ -35,6 +35,7 @@
>  #include <small/small.h>
>  #include <small/mempool.h>
>  
> +#include "ck_constraint.h"

Same: redundant include (?).