[tarantool-patches] Re: [PATCH v4 8/8] sql: remove sqlErrorMsg()
n.pettik
korablev at tarantool.org
Tue Mar 26 16:34:26 MSK 2019
>> @@ -1039,8 +1036,8 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages
>> const char *err = "Error at ORDER BY in place %d: "\
>> "term does not match any column in "\
>> "the result set";
>> - err = tt_sprintf(err, i + 1);
>> - diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
>> + diag_set(ClientError, ER_SQL_PARSER_GENERIC,
>> + tt_sprintf(err, i + 1));
>>
>>> @@ -1444,10 +1469,37 @@ resolveSelectStep(Walker * pWalker, Select * p)
>>> * number of expressions in the select list.
>>> */
>>> if (p->pNext && p->pEList->nExpr != p->pNext->pEList->nExpr) {
>>> - sqlSelectWrongNumTermsError(pParse, p->pNext);
>>> + if (p->pNext->selFlags & SF_Values) {
>>> + diag_set(ClientError, ER_SQL_PARSER_GENERIC,
>>> + "all VALUES must have the same "\
>>> + "number of terms");
>>> + } else {
>>> + const char *err_msg =
>>> + "SELECTs to the left and right of %s "\
>>> + "do not have the same number of "\
>>> + "result columns";
>>> + switch (p->pNext->op) {
>>> + case TK_ALL:
>>> + err_msg = tt_sprintf(err_msg,
>>> + "UNION ALL");
>>> + break;
>>
>> Why don’t use selectOpName?
>> Like it was in sqlSelectWrongNumTermsError().
>>
> Function selectOpName defined in different file and is static.
> I think it isn't worth to share it for one error.
I believe it is worth doing. For instance, you can return
sqlSelectWrongNumTermsError() which was placed in select.c
and called that static func.
> Diff between versions:
>
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index e26596a..1077285 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -666,13 +666,7 @@ codeVectorCompare(Parse * pParse, /* Code generator context */
> int regRight = 0;
> u8 op = pExpr->op;
> int addrDone = sqlVdbeMakeLabel(v);
> -
> - if (nLeft != sqlExprVectorSize(pRight)) {
> - diag_set(ClientError, ER_SQL_PARSER_GENERIC,
> - "row value misused");
> - pParse->is_aborted = true;
> - return;
> - }
Here the problem is that function is never called in our test suite:
I placed assert(0); and no one test failed. Please, add at least
simple tests like SELECT (1, 2) == (1, 2) to make sure that
codeVectorCompare() can be processed. Tests which you’ve added
fails before this func is called.
Added comment:
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 107728590..397f8209c 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -666,6 +666,11 @@ codeVectorCompare(Parse * pParse, /* Code generator context */
int regRight = 0;
u8 op = pExpr->op;
int addrDone = sqlVdbeMakeLabel(v);
+ /*
+ * Situation when vectors have different dimensions is
+ * filtred way before - during expr resolution:
+ * see resolveExprStep().
+ */
> + assert(nLeft == sqlExprVectorSize(pRight));
> assert(pExpr->op == TK_EQ || pExpr->op == TK_NE
> || pExpr->op == TK_LT || pExpr->op == TK_GT
> || pExpr->op == TK_LE || pExpr->op == TK_GE);
>
>
> @@ -4384,10 +4378,8 @@ sqlWhereBegin(Parse * pParse, /* The parser context */
> * equal to pTabList->nSrc but might be shortened to 1 if the
> * WHERE_OR_SUBCLAUSE flag is set.
> */
> - for (ii = 0; ii < pTabList->nSrc; ii++) {
> + for (ii = 0; ii < pTabList->nSrc; ii++)
> createMask(pMaskSet, pTabList->a[ii].iCursor);
> - sqlWhereTabFuncArgs(pParse, &pTabList->a[ii], &pWInfo->sWC);
Move removal of this func to a separate patch pls alongside with mentions of
table-valued funcs. It is not related to errors.
More information about the Tarantool-patches
mailing list