[tarantool-patches] Re: [PATCH v6 3/4] sql: space_def* instead of Table* in Expr

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hello. See below 3 minor comments.

On 15/05/2018 20:03, Kirill Shcherbatov wrote:
> This patch allows to remove Checks from SQL to
> server as sqlite3ResolveSelfReference requires
> Expr structure pointer.
> 
> Part of #3272.
> ---
>   src/box/field_def.c     |   1 +
>   src/box/field_def.h     |  14 +++++
>   src/box/sql.c           |  20 +++---
>   src/box/sql/alter.c     |   2 +-
>   src/box/sql/analyze.c   |   2 +-
>   src/box/sql/build.c     | 100 +++++++++++-------------------
>   src/box/sql/delete.c    |   4 +-
>   src/box/sql/expr.c      | 158 ++++++++++++++++++++++++++----------------------
>   src/box/sql/fkey.c      |  18 +++---
>   src/box/sql/insert.c    |  47 ++++++++------
>   src/box/sql/pragma.c    |  10 +--
>   src/box/sql/resolve.c   |  12 ++--
>   src/box/sql/select.c    |  26 +++++---
>   src/box/sql/sqliteInt.h |  44 +++-----------
>   src/box/sql/update.c    |  35 +++++------
>   src/box/sql/vdbe.c      |  26 ++++----
>   src/box/sql/vdbeaux.c   |  37 ------------
>   src/box/sql/vdbemem.c   |  18 +++---
>   src/box/sql/where.c     |  15 +++--
>   src/box/sql/wherecode.c |  35 ++++++-----
>   src/box/sql/whereexpr.c |   6 +-
>   21 files changed, 293 insertions(+), 337 deletions(-)
> 
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index 119940c..9a8f045 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -3519,45 +3526,48
> -/*
> +/**
>    * Generate code to extract the value of the iCol-th column of a table.
> + * @param v  The VDBE under construction.
> + * @param space_def Space definition.
> + * @param iTabCur The PK cursor.
> + * @param iCol Index of the column to extract.
> + * @param regOut  Extract the value into this register.

1. Lets move this comment to the header.

>    */
>   void
> -sqlite3ExprCodeGetColumnOfTable(Vdbe * v,	/* The VDBE under construction */
> -				Table * pTab,	/* The table containing the value */
> -				int iTabCur,	/* The PK cursor */
> -				int iCol,	/* Index of the column to extract */
> -				int regOut	/* Extract the value into this register */
> -    )
> +sqlite3ExprCodeGetColumnOfTable(Vdbe *v, struct space_def *space_def,
> +				int iTabCur, int iCol, int regOut)
>   {
>   	sqlite3VdbeAddOp3(v, OP_Column, iTabCur, iCol, regOut);
>   	if (iCol >= 0) {
> -		sqlite3ColumnDefault(v, pTab, iCol, regOut);
> +		sqlite3ColumnDefault(v, space_def, iCol, regOut);
>   	}
>   }
>   
> -/*
> +/**
>    * Generate code that will extract the iColumn-th column from
>    * table pTab and store the column value in a register.
>    *
> - * An effort is made to store the column value in register iReg.  This
> - * is not garanteeed for GetColumn() - the result can be stored in
> - * any register.  But the result is guaranteed to land in register iReg
> - * for GetColumnToReg().
> + * An effort is made to store the column value in register iReg.
> + * This is not garanteeed for GetColumn() - the result can be
> + * stored in any register.  But the result is guaranteed to land
> + * in register iReg for GetColumnToReg().
> + * @param pParse Parsing and code generating context.
> + * @param space_def Space definition.
> + * @param iColumn Index of the table column.
> + * @param iTable The cursor pointing to the table.
> + * @param iReg Store results here.
> + * @param p5 P5 value for OP_Column + FLAGS.
> + * @return iReg value.

2. Same.

>    */
>   int
> -sqlite3ExprCodeGetColumn(Parse * pParse,	/* Parsing and code generating context */
> -			 Table * pTab,	/* Description of the table we are reading from */
> -			 int iColumn,	/* Index of the table column */
> -			 int iTable,	/* The cursor pointing to the table */
> -			 int iReg,	/* Store results here */
> -			 u8 p5	/* P5 value for OP_Column + FLAGS */
> -    )
> +sqlite3ExprCodeGetColumn(Parse *pParse, struct space_def *space_def,
> +			 int iColumn, int iTable, int iReg, u8 p5)
>   {
>   	Vdbe *v = pParse->pVdbe;
>   	int i;
> @@ -3572,7 +3582,7 @@ sqlite3ExprCodeGetColumn(Parse * pParse,	/* Parsing and code generating context
>   		}
>   	}
>   	assert(v != 0);
> -	sqlite3ExprCodeGetColumnOfTable(v, pTab, iTable, iColumn, iReg);
> +	sqlite3ExprCodeGetColumnOfTable(v, space_def, iTable, iColumn, iReg);
>   	if (p5) {
>   		sqlite3VdbeChangeP5(v, p5);
>   	} else {
> @@ -3581,16 +3591,22 @@ sqlite3ExprCodeGetColumn(Parse * pParse,	/* Parsing and code generating context
>   	return iReg;
>   }
>   
> +/**
> + * Generate code that will extract the iColumn-th column from
> + * table pTab and store the column value in a register, copy the
> + * result.
> + * @param pParse Parsing and code generating context.
> + * @param space_def Space definition.
> + * @param iColumn Index of the table column.
> + * @param iTable The cursor pointing to the table.
> + * @param iReg Store results here.

3. Same.