[tarantool-patches] Re: [PATCH v1 4/4] sql: get rid of Column structure

  • From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
  • To: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>, tarantool-patches@xxxxxxxxxxxxx, Nikita Pettik <korablev@xxxxxxxxxxxxx>
  • Date: Wed, 18 Jul 2018 11:04:37 +0300

LGTM. Nikita, please, review.

On 18/07/2018 10:25, Kirill Shcherbatov wrote:

1. Please, rebase on the latest 2.0 and use pk->def + key_def_find.
-       if (index_has_column(pk, pNew->def->field_count - 1)) {
+       assert(pk != NULL);
+       struct key_def *pk_key_def = pk->def->key_def;
+       if (key_def_find(pk_key_def, pNew->def->field_count - 1) != NULL) {

2. As I said verbally, you should not add this redundant scan of primary
index columns. You already have the primary index scan in
convertToWithoutRowidTable, that is called few lines above.

When you merge this scan into convertToWithoutRowidTable, you can
inline the rest of the function into EndTable and remove it together
with index_has_column.
I can't understand your suggestion.
convertToWithoutRowidTable iterates by index field;
but this check is a about field with ON CONFLICT REPLACE could be only a part
of index. We iterate over all fields.

I've get rid off index_has_column.

                 if (field->nullable_action == ON_CONFLICT_ACTION_REPLACE &&
-                   !index_has_column(pk, i))
+                   (pk == NULL || key_def_find(pk->def->key_def, i) == NULL))


-bool
-index_has_column(struct Index *index, uint32_t  column_idx)
etc.


Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018