[tarantool-patches] Re: [PATCH] sql: remove unnecessary MakeRecord opcodes

[n.pettik]

Hello. Only minor remark concerning commit message.
Rephrase it little bit, please. Lets mention, that you are saying about
bytecode which is generated to process constraint checks.

> On 4 Apr 2018, at 15:38, Bulat Niatshin <niatshin at tarantool.org> wrote:
> 
> OP_MakeRecord creates a record from a given range of registers
> and stores that record in a specified register.

This is obvious and can be dropped.

> But now even
> secondary indexes with non-default ON CONFLICT clause doesn't
> need it,

Good place to explain why.

> it is necessary only for primary key indexes. However
> that unnecesary opcodes appear in INSERT listings. This patch
> contains a fix for that.
> 
> Closes #3317
> ---
> src/box/sql/insert.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 4a57b23f5..701fae412 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -1309,18 +1309,10 @@ sqlite3GenerateConstraintChecks(Parse * pParse,		/* The parser context */
> 					sqlite3VdbeResolveLabel(v, skip_if_null);
> 				}
> 			}
> -			if (IsPrimaryKeyIndex(pIdx) || uniqueByteCodeNeeded) {
> -				sqlite3VdbeAddOp3(v, OP_MakeRecord, regNewData + 1,
> -						  pTab->nCol, aRegIdx[ix]);
> -				VdbeComment((v, "for %s", pIdx->zName));
> -			}
> -		} else {
> -			/* kyukhin: for Tarantool, this should be evaluated to NOP.  */
> -			if (IsPrimaryKeyIndex(pIdx) || uniqueByteCodeNeeded) {
> -				sqlite3VdbeAddOp3(v, OP_MakeRecord, regIdx,
> -						  nIdxCol, aRegIdx[ix]);
> -				VdbeComment((v, "for %s", pIdx->zName));
> -			}
> +
> +			sqlite3VdbeAddOp3(v, OP_MakeRecord, regNewData + 1,
> +					pTab->nCol, aRegIdx[ix]);
> +			VdbeComment((v, "for %s", pIdx->zName));
> 		}
> 
> 		/* In an UPDATE operation, if this index is the PRIMARY KEY
> -- 
> 2.14.1
>