* [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form
@ 2019-02-15 13:30 Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 1/4] sql: patch sql_name_from_token to use Parser Kirill Shcherbatov
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Kirill Shcherbatov @ 2019-02-15 13:30 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Perform SQL name normalization: cast name to the upper-case
(via Unicode Character Folding). Casing is locale-dependent
and context-sensitive. The result may be longer or shorter
than the original. For example, ß is converted to SS.
The result is similar to SQL UPPER function.
Performed extensive code refactoring to pass parser instance in
routines that use sql_normalize_name function. This makes
possible to raise an error in case of normalizing failure.
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3991-fix-names-normalization
Issue: https://github.com/tarantool/tarantool/issues/3991
Kirill Shcherbatov (4):
sql: patch sql_name_from_token to use Parser
sql: patch sql_trigger_step_allocate to use Parser
sql: patch sql_expr_create routine to use Parser
sql: store regular identifiers in case-normal form
src/box/lua/lua_sql.c | 11 +-
src/box/sql/alter.c | 2 +-
src/box/sql/analyze.c | 2 +-
src/box/sql/build.c | 212 +++++++++------------
src/box/sql/delete.c | 2 +-
src/box/sql/expr.c | 256 ++++++++++++--------------
src/box/sql/fkey.c | 128 +++++++------
src/box/sql/parse.y | 60 +++---
src/box/sql/pragma.c | 11 +-
src/box/sql/resolve.c | 41 ++---
src/box/sql/select.c | 70 ++++---
src/box/sql/sqlInt.h | 255 +++++++++++++++++++++++--
src/box/sql/trigger.c | 201 +++++++++-----------
src/box/sql/util.c | 42 +++--
src/box/sql/wherecode.c | 3 +-
src/box/sql/whereexpr.c | 10 +-
test/sql-tap/identifier_case.test.lua | 12 +-
17 files changed, 753 insertions(+), 565 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v1 1/4] sql: patch sql_name_from_token to use Parser
2019-02-15 13:30 [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
@ 2019-02-15 13:30 ` Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 2/4] sql: patch sql_trigger_step_allocate " Kirill Shcherbatov
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Kirill Shcherbatov @ 2019-02-15 13:30 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
The code was refactored so that the sql_name_from_token function
would use the parser object instead of database connection. Also
performed some additional names refactoring in adjacent places.
Needed for #3931
---
src/box/sql/alter.c | 2 +-
src/box/sql/analyze.c | 2 +-
src/box/sql/build.c | 162 ++++++++++++++----------------------------
src/box/sql/delete.c | 2 +-
src/box/sql/expr.c | 2 +-
src/box/sql/fkey.c | 4 +-
src/box/sql/parse.y | 10 +--
src/box/sql/pragma.c | 11 ++-
src/box/sql/select.c | 4 +-
src/box/sql/sqlInt.h | 81 ++++++++++++++++++++-
src/box/sql/trigger.c | 4 +-
11 files changed, 152 insertions(+), 132 deletions(-)
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index d49ebb8df..6d9960156 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -43,7 +43,7 @@ sql_alter_table_rename(struct Parse *parse, struct SrcList *src_tab,
{
assert(src_tab->nSrc == 1);
struct sql *db = parse->db;
- char *new_name = sqlNameFromToken(db, new_name_tk);
+ char *new_name = sql_name_from_token(parse, new_name_tk);
if (new_name == NULL)
goto exit_rename_table;
/* Check that new name isn't occupied by another table. */
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 7eabd0c7e..e82dcde0a 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1118,7 +1118,7 @@ sqlAnalyze(Parse * pParse, Token * pName)
sql_analyze_database(pParse);
} else {
/* Form 2: Analyze table named */
- char *z = sqlNameFromToken(db, pName);
+ char *z = sql_name_from_token(pParse, pName);
if (z != NULL) {
struct space *sp = space_by_name(z);
if (sp != NULL) {
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 5dc3d0248..3f4530620 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -294,30 +294,15 @@ sqlDeleteTable(sql * db, Table * pTable)
table_delete(db, pTable);
}
-/*
- * Given a token, return a string that consists of the text of that
- * token. Space to hold the returned string
- * is obtained from sqlMalloc() and must be freed by the calling
- * function.
- *
- * Any quotation marks (ex: "name", 'name', [name], or `name`) that
- * surround the body of the token are removed.
- *
- * Tokens are often just pointers into the original SQL text and so
- * are not \000 terminated and are not persistent. The returned string
- * is \000 terminated and is persistent.
- */
char *
-sqlNameFromToken(sql * db, Token * pName)
+sql_name_from_token(struct Parse *parser, struct Token *name_token)
{
- char *zName;
- if (pName) {
- zName = sqlDbStrNDup(db, (char *)pName->z, pName->n);
- sqlNormalizeName(zName);
- } else {
- zName = 0;
- }
- return zName;
+ if (name_token == NULL || name_token->z == NULL)
+ return NULL;
+ char *name = sqlDbStrNDup(parser->db, (char *)name_token->z,
+ name_token->n);
+ sqlNormalizeName(name);
+ return name;
}
/*
@@ -404,7 +389,7 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr)
goto cleanup;
sqlVdbeCountChanges(v);
- zName = sqlNameFromToken(db, pName);
+ zName = sql_name_from_token(pParse, pName);
pParse->sNameToken = *pName;
if (zName == 0)
@@ -779,7 +764,7 @@ sqlAddCollateType(Parse * pParse, Token * pToken)
return;
uint32_t i = p->def->field_count - 1;
sql *db = pParse->db;
- char *zColl = sqlNameFromToken(db, pToken);
+ char *zColl = sql_name_from_token(pParse, pToken);
if (!zColl)
return;
uint32_t *coll_id = &p->def->fields[i].coll_id;
@@ -1838,7 +1823,7 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child,
rlist_add_entry(&parse_context->new_fkey, fk, link);
}
assert(parent != NULL);
- parent_name = sqlNameFromToken(db, parent);
+ parent_name = sql_name_from_token(parse_context, parent);
if (parent_name == NULL)
goto exit_create_fk;
/*
@@ -1875,10 +1860,12 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child,
new_tab->def->name);
} else {
struct Token *cnstr_nm = &parse_context->constraintName;
- constraint_name = sqlNameFromToken(db, cnstr_nm);
+ constraint_name = sql_name_from_token(parse_context,
+ cnstr_nm);
}
} else {
- constraint_name = sqlNameFromToken(db, constraint);
+ constraint_name =
+ sql_name_from_token(parse_context, constraint);
}
if (constraint_name == NULL)
goto exit_create_fk;
@@ -2011,8 +1998,7 @@ sql_drop_foreign_key(struct Parse *parse_context, struct SrcList *table,
parse_context->nErr++;
return;
}
- char *constraint_name = sqlNameFromToken(parse_context->db,
- constraint);
+ char *constraint_name = sql_name_from_token(parse_context, constraint);
if (constraint_name != NULL)
vdbe_emit_fkey_drop(parse_context, constraint_name,
child->def->id);
@@ -2282,7 +2268,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
*/
if (token != NULL) {
assert(token->z != NULL);
- name = sqlNameFromToken(db, token);
+ name = sql_name_from_token(parse, token);
if (name == NULL)
goto exit_create_index;
if (sql_space_index_by_name(space, name) != NULL) {
@@ -2295,10 +2281,11 @@ sql_create_index(struct Parse *parse, struct Token *token,
}
} else {
char *constraint_name = NULL;
- if (parse->constraintName.z != NULL)
+ if (parse->constraintName.z != NULL) {
constraint_name =
- sqlNameFromToken(db,
- &parse->constraintName);
+ sql_name_from_token(parse,
+ &parse->constraintName);
+ }
/*
* This naming is temporary. Now it's not
@@ -2539,7 +2526,8 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
/* Never called with prior errors. */
assert(parse_context->nErr == 0);
assert(table_token != NULL);
- const char *table_name = sqlNameFromToken(db, table_token);
+ const char *table_name =
+ sql_name_from_token(parse_context, table_token);
if (db->mallocFailed) {
goto exit_drop_index;
}
@@ -2626,30 +2614,25 @@ sqlArrayAllocate(sql * db, /* Connection to notify of malloc failures */
return pArray;
}
-/*
- * Append a new element to the given IdList. Create a new IdList if
- * need be.
- *
- * A new IdList is returned, or NULL if malloc() fails.
- */
-IdList *
-sqlIdListAppend(sql * db, IdList * pList, Token * pToken)
+struct IdList *
+sql_IdList_append(struct Parse *parser, struct IdList *list,
+ struct Token *name_token)
{
+ struct sql *db = parser->db;
int i;
- if (pList == 0) {
- pList = sqlDbMallocZero(db, sizeof(IdList));
- if (pList == 0)
- return 0;
+ if (list == NULL) {
+ list = sqlDbMallocZero(db, sizeof(IdList));
+ if (list == NULL)
+ return NULL;
}
- pList->a = sqlArrayAllocate(db,
- pList->a,
- sizeof(pList->a[0]), &pList->nId, &i);
+ list->a = sqlArrayAllocate(db, list->a, sizeof(list->a[0]),
+ &list->nId, &i);
if (i < 0) {
- sqlIdListDelete(db, pList);
+ sqlIdListDelete(db, list);
return 0;
}
- pList->a[i].zName = sqlNameFromToken(db, pToken);
- return pList;
+ list->a[i].zName = sql_name_from_token(parser, name_token);
+ return list;
}
/*
@@ -2772,62 +2755,26 @@ sql_alloc_src_list(sql *db)
return pList;
}
-/*
- * Append a new table name to the given SrcList. Create a new SrcList if
- * need be. A new entry is created in the SrcList even if pTable is NULL.
- *
- * A SrcList is returned, or NULL if there is an OOM error. The returned
- * SrcList might be the same as the SrcList that was input or it might be
- * a new one. If an OOM error does occurs, then the prior value of pList
- * that is input to this routine is automatically freed.
- *
- * If pDatabase is not null, it means that the table has an optional
- * database name prefix. Like this: "database.table". The pDatabase
- * points to the table name and the pTable points to the database name.
- * The SrcList.a[].zName field is filled with the table name which might
- * come from pTable (if pDatabase is NULL) or from pDatabase.
- * SrcList.a[].zDatabase is filled with the database name from pTable,
- * or with NULL if no database is specified.
- *
- * In other words, if call like this:
- *
- * sqlSrcListAppend(D,A,B,0);
- *
- * Then B is a table name and the database name is unspecified. If called
- * like this:
- *
- * sqlSrcListAppend(D,A,B,C);
- *
- * Then C is the table name and B is the database name. If C is defined
- * then so is B. In other words, we never have a case where:
- *
- * sqlSrcListAppend(D,A,0,C);
- *
- * Both pTable and pDatabase are assumed to be quoted. They are dequoted
- * before being added to the SrcList.
- */
-SrcList *
-sqlSrcListAppend(sql * db, /* Connection to notify of malloc failures */
- SrcList * pList, /* Append to this SrcList. NULL creates a new SrcList */
- Token * pTable /* Table to append */
- )
+struct SrcList *
+sql_SrcList_append(struct Parse *parser, struct SrcList *list,
+ struct Token *name_token)
{
struct SrcList_item *pItem;
- assert(db != 0);
- if (pList == 0) {
- pList = sql_alloc_src_list(db);
- if (pList == 0)
- return 0;
+ struct sql *db = parser->db;
+ if (list == NULL) {
+ list = sql_alloc_src_list(db);
+ if (list == NULL)
+ return NULL;
} else {
- pList = sqlSrcListEnlarge(db, pList, 1, pList->nSrc);
+ list = sqlSrcListEnlarge(db, list, 1, list->nSrc);
}
if (db->mallocFailed) {
- sqlSrcListDelete(db, pList);
+ sqlSrcListDelete(db, list);
return 0;
}
- pItem = &pList->a[pList->nSrc - 1];
- pItem->zName = sqlNameFromToken(db, pTable);
- return pList;
+ pItem = &list->a[list->nSrc - 1];
+ pItem->zName = sql_name_from_token(parser, name_token);
+ return list;
}
/*
@@ -2909,15 +2856,14 @@ sqlSrcListAppendFromTerm(Parse * pParse, /* Parsing context */
);
goto append_from_error;
}
- p = sqlSrcListAppend(db, p, pTable);
+ p = sql_SrcList_append(pParse, p, pTable);
if (p == 0 || NEVER(p->nSrc == 0)) {
goto append_from_error;
}
pItem = &p->a[p->nSrc - 1];
assert(pAlias != 0);
- if (pAlias->n) {
- pItem->zAlias = sqlNameFromToken(db, pAlias);
- }
+ if (pAlias->n != 0)
+ pItem->zAlias = sql_name_from_token(pParse, pAlias);
pItem->pSelect = pSubquery;
pItem->pOn = pOn;
pItem->pUsing = pUsing;
@@ -2951,7 +2897,7 @@ sqlSrcListIndexedBy(Parse * pParse, SrcList * p, Token * pIndexedBy)
pItem->fg.notIndexed = 1;
} else {
pItem->u1.zIndexedBy =
- sqlNameFromToken(pParse->db, pIndexedBy);
+ sql_name_from_token(pParse, pIndexedBy);
pItem->fg.isIndexedBy = (pItem->u1.zIndexedBy != 0);
}
}
@@ -3037,7 +2983,7 @@ sql_transaction_rollback(Parse *pParse)
void
sqlSavepoint(Parse * pParse, int op, Token * pName)
{
- char *zName = sqlNameFromToken(pParse->db, pName);
+ char *zName = sql_name_from_token(pParse, pName);
if (zName) {
Vdbe *v = sqlGetVdbe(pParse);
if (!v) {
@@ -3131,7 +3077,7 @@ sqlWithAdd(Parse * pParse, /* Parsing context */
/* Check that the CTE name is unique within this WITH clause. If
* not, store an error in the Parse structure.
*/
- zName = sqlNameFromToken(pParse->db, pName);
+ zName = sql_name_from_token(pParse, pName);
if (zName && pWith) {
int i;
for (i = 0; i < pWith->nCte; i++) {
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 41daa44ee..2b5d820ab 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -71,7 +71,7 @@ sql_materialize_view(struct Parse *parse, const char *name, struct Expr *where,
{
struct sql *db = parse->db;
where = sqlExprDup(db, where, 0);
- struct SrcList *from = sqlSrcListAppend(db, NULL, NULL);
+ struct SrcList *from = sql_SrcList_append(parse, NULL, NULL);
if (from != NULL) {
assert(from->nSrc == 1);
from->a[0].zName = sqlDbStrDup(db, name);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 4450ac7ee..a6e35df6d 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1583,7 +1583,7 @@ sqlIdListDup(sql * db, IdList * p)
return 0;
}
/* Note that because the size of the allocation for p->a[] is not
- * necessarily a power of two, sqlIdListAppend() may not be called
+ * necessarily a power of two, sql_IdList_append() may not be called
* on the duplicate created by this function.
*/
for (i = 0; i < p->nId; i++) {
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 32e5b3335..69740b39f 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -610,7 +610,7 @@ fkey_emit_check(struct Parse *parser, struct Table *tab, int reg_old,
* table. We need the child table as a SrcList for
* sqlWhereBegin().
*/
- struct SrcList *src = sqlSrcListAppend(db, NULL, NULL);
+ struct SrcList *src = sql_SrcList_append(parser, NULL, NULL);
if (src == NULL)
continue;
struct SrcList_item *item = src->a;
@@ -871,7 +871,7 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
r->on_conflict_action = ON_CONFLICT_ACTION_ABORT;
select = sqlSelectNew(pParse,
sql_expr_list_append(db, NULL, r),
- sqlSrcListAppend(db, NULL, &err),
+ sql_SrcList_append(pParse, NULL, &err),
where, NULL, NULL, NULL, 0, NULL,
NULL);
where = NULL;
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 661e69584..4713d0d09 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -604,7 +604,7 @@ seltablist(A) ::= stl_prefix(A) LP seltablist(F) RP
%type fullname {SrcList*}
%destructor fullname {sqlSrcListDelete(pParse->db, $$);}
fullname(A) ::= nm(X).
- {A = sqlSrcListAppend(pParse->db,0,&X); /*A-overwrites-X*/}
+ {A = sql_SrcList_append(pParse,0,&X); /*A-overwrites-X*/}
%type joinop {int}
join_nm(A) ::= id(A).
@@ -786,9 +786,9 @@ insert_cmd(A) ::= REPLACE. {A = ON_CONFLICT_ACTION_REPLACE;}
idlist_opt(A) ::= . {A = 0;}
idlist_opt(A) ::= LP idlist(X) RP. {A = X;}
idlist(A) ::= idlist(A) COMMA nm(Y).
- {A = sqlIdListAppend(pParse->db,A,&Y);}
+ {A = sql_IdList_append(pParse,A,&Y);}
idlist(A) ::= nm(Y).
- {A = sqlIdListAppend(pParse->db,0,&Y); /*A-overwrites-Y*/}
+ {A = sql_IdList_append(pParse,0,&Y); /*A-overwrites-Y*/}
/////////////////////////// Expression Processing /////////////////////////////
//
@@ -1140,7 +1140,7 @@ expr(A) ::= expr(A) in_op(N) LP select(Y) RP(E). [IN] {
A.zEnd = &E.z[E.n];
}
expr(A) ::= expr(A) in_op(N) nm(Y) paren_exprlist(E). [IN] {
- SrcList *pSrc = sqlSrcListAppend(pParse->db, 0,&Y);
+ SrcList *pSrc = sql_SrcList_append(pParse, 0,&Y);
Select *pSelect = sqlSelectNew(pParse, 0,pSrc,0,0,0,0,0,0,0);
if( E ) sqlSrcListFuncArgs(pParse, pSelect ? pSrc : 0, E);
A.pExpr = sqlPExpr(pParse, TK_IN, A.pExpr, 0);
@@ -1210,7 +1210,7 @@ paren_exprlist(A) ::= LP exprlist(X) RP. {A = X;}
//
cmd ::= createkw(S) uniqueflag(U) INDEX ifnotexists(NE) nm(X)
ON nm(Y) LP sortlist(Z) RP. {
- sql_create_index(pParse, &X, sqlSrcListAppend(pParse->db,0,&Y), Z, &S,
+ sql_create_index(pParse, &X, sql_SrcList_append(pParse,0,&Y), Z, &S,
SORT_ORDER_ASC, NE, U);
}
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index f3e66f506..797224792 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -436,18 +436,17 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
sqlVdbeRunOnlyOnce(v);
pParse->nMem = 2;
- zLeft = sqlNameFromToken(db, pId);
+ zLeft = sql_name_from_token(pParse, pId);
if (!zLeft) {
printActivePragmas(user_session);
return;
}
- if (minusFlag) {
+ if (minusFlag)
zRight = sqlMPrintf(db, "-%T", pValue);
- } else {
- zRight = sqlNameFromToken(db, pValue);
- }
- zTable = sqlNameFromToken(db, pValue2);
+ else
+ zRight = sql_name_from_token(pParse, pValue);
+ zTable = sql_name_from_token(pParse, pValue2);
db->busyHandler.nBusy = 0;
/* Locate the pragma in the lookup table */
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index a6e78d0df..dfd3a375e 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -256,7 +256,7 @@ findRightmost(Select * p)
/**
- * Work the same as sqlSrcListAppend(), but before adding to
+ * Work the same as sql_SrcList_append(), but before adding to
* list provide check on name duplicates: only values with unique
* names are appended. Moreover, names of tables are not
* normalized: it is parser's business and in struct Select they
@@ -4112,7 +4112,7 @@ flattenSubquery(Parse * pParse, /* Parsing context */
} else {
assert(pParent != p); /* 2nd and subsequent times through the loop */
pSrc = pParent->pSrc =
- sqlSrcListAppend(db, 0, 0);
+ sql_SrcList_append(pParse, 0, 0);
if (pSrc == 0) {
assert(db->mallocFailed);
break;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 7f17fd84a..498982e01 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3468,12 +3468,70 @@ void sqlDeleteTable(sql *, Table *);
void sqlInsert(Parse *, SrcList *, Select *, IdList *,
enum on_conflict_action);
void *sqlArrayAllocate(sql *, void *, int, int *, int *);
-IdList *sqlIdListAppend(sql *, IdList *, Token *);
+
+/*
+ * Append a new element to the given IdList. Create a new IdList if
+ * need be.
+ * A new IdList is returned, or NULL if malloc() fails.
+ * @param parser The parse context.
+ * @param list The pointer to existent Id list if exists.
+ * @param name_token The token containing name.
+ * @retval not NULL IdList pointer is returned on success.
+ * @retval NULL otherwise.
+ */
+struct IdList *
+sql_IdList_append(struct Parse *parser, struct IdList *pList,
+ struct Token *pToken);
+
int sqlIdListIndex(IdList *, const char *);
SrcList *sqlSrcListEnlarge(sql *, SrcList *, int, int);
SrcList *
sql_alloc_src_list(sql *db);
-SrcList *sqlSrcListAppend(sql *, SrcList *, Token *);
+/*
+ * Append a new table name to the given list. Create a new
+ * SrcList if need be. A new entry is created in the list even
+ * if name_token is NULL.
+ *
+ * A SrcList is returned, or NULL if there is an OOM error.
+ * The returned SrcList might be the same as the SrcList that was
+ * input or it might be a new one. If an OOM error does occurs,
+ * then the prior value of list that is input to this routine is
+ * automatically freed.
+ *
+ * If pDatabase is not null, it means that the table has an
+ * optional database name prefix. Like this: "database.table".
+ * The pDatabase points to the table name and the pTable points
+ * to the database name. The SrcList.a[].zName field is filled
+ * with the table name which might come from pTable (if pDatabase
+ * is NULL) or from pDatabase.
+ * SrcList.a[].zDatabase is filled with the database name from
+ * name_token, or with NULL if no database is specified.
+ *
+ * In other words, if call like this:
+ *
+ * sql_SrcList_append(D,A,B,0);
+ *
+ * Then B is a table name and the database name is unspecified.
+ * If called like this:
+ *
+ * sql_SrcList_append(D,A,B,C);
+ *
+ * Then C is the table name and B is the database name. If C is
+ * defined then so is B. In other words, we never have a case
+ * where:
+ *
+ * sql_SrcList_append(D,A,0,C);
+ *
+ * Both pTable and pDatabase are assumed to be quoted. They are
+ * dequoted before being added to the SrcList.
+ * @param parser Parse context.
+ * @param list Append to this SrcList. NULL creates a new SrcList.
+ * @param name_token Token representing table name.
+ * @retval not NULL SrcList pointer on success, NULL otherwise.
+ */
+struct SrcList *
+sql_SrcList_append(struct Parse *parse, struct SrcList * pList,
+ struct Token *pTable);
SrcList *sqlSrcListAppendFromTerm(Parse *, SrcList *, Token *,
Token *, Select *, Expr *, IdList *);
void sqlSrcListIndexedBy(Parse *, SrcList *, Token *);
@@ -3656,7 +3714,24 @@ int sqlExprCodeExprList(Parse *, ExprList *, int, int, u8);
void sqlExprIfTrue(Parse *, Expr *, int, int);
void sqlExprIfFalse(Parse *, Expr *, int, int);
-char *sqlNameFromToken(sql *, Token *);
+/*
+ * Given a token, return a string that consists of the text of
+ * that token. Space to hold the returned string is obtained
+ * from sqlMalloc() and must be freed by the calling function.
+ *
+ * Any quotation marks (ex: "name", 'name', [name], or `name`)
+ * that surround the body of the token are removed.
+ *
+ * Tokens are often just pointers into the original SQL text and
+ * so are not \000 terminated and are not persistent. The returned
+ * string is \000 terminated and is persistent.
+ * @param parser The parse context.
+ * @param name_token The source token with text.
+ * @retval not NULL string pointer on success, NULL otherwise.
+ */
+char *
+sql_name_from_token(struct Parse *parser, struct Token *name_token);
+
int sqlExprCompare(Expr *, Expr *, int);
int sqlExprListCompare(ExprList *, ExprList *, int);
int sqlExprImpliesExpr(Expr *, Expr *, int);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 8ad4e2ebd..2f54754fa 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -82,7 +82,7 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
goto trigger_cleanup;
assert(table->nSrc == 1);
- trigger_name = sqlNameFromToken(db, name);
+ trigger_name = sql_name_from_token(parse, name);
if (trigger_name == NULL)
goto trigger_cleanup;
@@ -588,7 +588,7 @@ targetSrcList(Parse * pParse, /* The parsing context */
sql *db = pParse->db;
SrcList *pSrc; /* SrcList to be returned */
- pSrc = sqlSrcListAppend(db, 0, 0);
+ pSrc = sql_SrcList_append(pParse, 0, 0);
if (pSrc) {
assert(pSrc->nSrc > 0);
pSrc->a[pSrc->nSrc - 1].zName =
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v1 2/4] sql: patch sql_trigger_step_allocate to use Parser
2019-02-15 13:30 [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 1/4] sql: patch sql_name_from_token to use Parser Kirill Shcherbatov
@ 2019-02-15 13:30 ` Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 3/4] sql: patch sql_expr_create routine " Kirill Shcherbatov
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Kirill Shcherbatov @ 2019-02-15 13:30 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
The code was refactored so that the sql_trigger_step_allocate
function would use the parser object instead of database
connection. Also performed some additional names refactoring in
adjacent places.
Needed for #3931
---
src/box/sql/parse.y | 8 +-
src/box/sql/sqlInt.h | 69 ++++++++++++++--
src/box/sql/trigger.c | 185 +++++++++++++++++-------------------------
3 files changed, 140 insertions(+), 122 deletions(-)
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 4713d0d09..1a18d5f89 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1397,19 +1397,19 @@ tridxby ::= NOT INDEXED. {
// UPDATE
trigger_cmd(A) ::=
UPDATE orconf(R) trnm(X) tridxby SET setlist(Y) where_opt(Z).
- {A = sqlTriggerUpdateStep(pParse->db, &X, Y, Z, R);}
+ {A = sql_trigger_update_step(pParse, &X, Y, Z, R);}
// INSERT
trigger_cmd(A) ::= insert_cmd(R) INTO trnm(X) idlist_opt(F) select(S).
- {A = sqlTriggerInsertStep(pParse->db, &X, F, S, R);/*A-overwrites-R*/}
+ {A = sql_trigger_insert_step(pParse, &X, F, S, R);/*A-overwrites-R*/}
// DELETE
trigger_cmd(A) ::= DELETE FROM trnm(X) tridxby where_opt(Y).
- {A = sqlTriggerDeleteStep(pParse->db, &X, Y);}
+ {A = sql_trigger_delete_step(pParse, &X, Y);}
// SELECT
trigger_cmd(A) ::= select(X).
- {A = sqlTriggerSelectStep(pParse->db, X); /*A-overwrites-X*/}
+ {A = sql_trigger_select_step(pParse, X); /*A-overwrites-X*/}
// The special RAISE expression that may occur in trigger programs
expr(A) ::= RAISE(X) LP IGNORE RP(Y). {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 498982e01..99c8adbdd 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4154,12 +4154,69 @@ vdbe_code_row_trigger_direct(struct Parse *parser, struct sql_trigger *trigger,
int ignore_jump);
void sqlDeleteTriggerStep(sql *, TriggerStep *);
-TriggerStep *sqlTriggerSelectStep(sql *, Select *);
-TriggerStep *sqlTriggerInsertStep(sql *, Token *, IdList *,
- Select *, u8);
-TriggerStep *sqlTriggerUpdateStep(sql *, Token *, ExprList *, Expr *,
- u8);
-TriggerStep *sqlTriggerDeleteStep(sql *, Token *, Expr *);
+
+/**
+ * Turn a SELECT statement (that the pSelect parameter points to)
+ * into a trigger step. Return a pointer to a TriggerStep
+ * structure.
+ * The parser calls this routine when it finds a SELECT statement in
+ * body of a TRIGGER.
+ * @param parser The parsing context.
+ * @param select The SELECT statement to process.
+ * @retval not NULL TriggerStep object on success, NULL otherwise.
+ */
+struct TriggerStep *
+sql_trigger_select_step(struct Parse *parser, struct Select *select);
+
+/**
+ * Build a trigger step out of an INSERT statement. Return a
+ * pointer to the new trigger step.
+ * The parser calls this routine when it sees an INSERT inside the
+ * body of a trigger.
+ * @param parser The parsing context.
+ * @param table_name Name of the table into which we insert.
+ * @param column_list List of columns in table to insert into.
+ * @param select The SELECT statement that supplies values.
+ * @param orconf The conflict algorithm
+ * (ON_CONFLICT_ACTION_ABORT, _REPLACE, etc.).
+ * @retval not NULL TriggerStep object on success, NULL otherwise.
+ */
+struct TriggerStep *
+sql_trigger_insert_step(struct Parse *parser, struct Token *table_name,
+ struct IdList *column_list, struct Select *select,
+ u8 orconf);
+
+/*
+ * Construct a trigger step that implements an UPDATE statement
+ * and return a pointer to that trigger step. The parser calls
+ * this routine when it sees an UPDATE statement inside the body
+ * of a CREATE TRIGGER.
+ * @param parset The parsing context.
+ * @param table_name Name of the table to be updated.
+ * @param new_list The SET clause: list of column and new values.
+ * @param where The WHERE clause.
+ * @param orconf The conflict algorithm
+ * (ON_CONFLICT_ACTION_ABORT, _REPLACE, etc.).
+ * @retval not NULL TriggerStep object on success, NULL otherwise.
+ */
+struct TriggerStep *
+sql_trigger_update_step(struct Parse *parser, struct Token *table_name,
+ struct ExprList *new_list, struct Expr *where,
+ u8 orconf);
+
+/*
+ * Construct a trigger step that implements a DELETE statement and
+ * return a pointer to that trigger step. The parser calls this
+ * routine when it sees a DELETE statement inside the body of a
+ * CREATE TRIGGER.
+ * @param parser The parsing context.
+ * @param table_name The table from which rows are deleted.
+ * @param where The WHERE clause.
+ * @retval not NULL TriggerStep object on success, NULL otherwise.
+ */
+struct TriggerStep *
+sql_trigger_delete_step(struct Parse *parser, struct Token * table_name,
+ struct Expr *where);
/**
* Triggers may access values stored in the old.* or new.*
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 2f54754fa..a5b6cb4fd 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -249,142 +249,103 @@ cleanup:
sqlDeleteTriggerStep(db, step_list);
}
-/*
- * Turn a SELECT statement (that the pSelect parameter points to) into
- * a trigger step. Return a pointer to a TriggerStep structure.
- *
- * The parser calls this routine when it finds a SELECT statement in
- * body of a TRIGGER.
- */
-TriggerStep *
-sqlTriggerSelectStep(sql * db, Select * pSelect)
+struct TriggerStep *
+sql_trigger_select_step(struct Parse *parser, struct Select *select)
{
- TriggerStep *pTriggerStep =
- sqlDbMallocZero(db, sizeof(TriggerStep));
- if (pTriggerStep == 0) {
- sql_select_delete(db, pSelect);
+ struct sql *db = parser->db;
+ struct TriggerStep *trigger_step =
+ sqlDbMallocZero(db, sizeof(TriggerStep));
+ if (trigger_step == NULL) {
+ sql_select_delete(db, select);
return 0;
}
- pTriggerStep->op = TK_SELECT;
- pTriggerStep->pSelect = pSelect;
- pTriggerStep->orconf = ON_CONFLICT_ACTION_DEFAULT;
- return pTriggerStep;
+ trigger_step->op = TK_SELECT;
+ trigger_step->pSelect = select;
+ trigger_step->orconf = ON_CONFLICT_ACTION_DEFAULT;
+ return trigger_step;
}
/*
* Allocate space to hold a new trigger step. The allocated space
- * holds both the TriggerStep object and the TriggerStep.target.z string.
- *
- * If an OOM error occurs, NULL is returned and db->mallocFailed is set.
+ * holds both the TriggerStep object and the TriggerStep.target.z
+ * string.
+ * @param parser The parsing context.
+ * @param op Trigger opcode.
+ * @param target_name The target name token.
+ * @retval not NULL TriggerStep object on success, NULL otherwise.
*/
-static TriggerStep *
-triggerStepAllocate(sql * db, /* Database connection */
- u8 op, /* Trigger opcode */
- Token * pName /* The target name */
- )
+static struct TriggerStep *
+sql_trigger_step_allocate(struct Parse *parser, u8 op,
+ struct Token *target_name)
{
- TriggerStep *pTriggerStep;
-
- pTriggerStep =
- sqlDbMallocZero(db, sizeof(TriggerStep) + pName->n + 1);
- if (pTriggerStep) {
- char *z = (char *)&pTriggerStep[1];
- memcpy(z, pName->z, pName->n);
+ struct sql *db = parser->db;
+ struct TriggerStep *trigger_step =
+ sqlDbMallocZero(db, sizeof(TriggerStep) + target_name->n + 1);
+ if (trigger_step != NULL) {
+ char *z = (char *)&trigger_step[1];
+ memcpy(z, target_name->z, target_name->n);
sqlNormalizeName(z);
- pTriggerStep->zTarget = z;
- pTriggerStep->op = op;
+ trigger_step->zTarget = z;
+ trigger_step->op = op;
}
- return pTriggerStep;
+ return trigger_step;
}
-/*
- * Build a trigger step out of an INSERT statement. Return a pointer
- * to the new trigger step.
- *
- * The parser calls this routine when it sees an INSERT inside the
- * body of a trigger.
- */
-TriggerStep *
-sqlTriggerInsertStep(sql * db, /* The database connection */
- Token * pTableName, /* Name of the table into which we insert */
- IdList * pColumn, /* List of columns in pTableName to insert into */
- Select * pSelect, /* A SELECT statement that supplies values */
- u8 orconf /* The conflict algorithm
- * (ON_CONFLICT_ACTION_ABORT, _REPLACE,
- * etc.)
- */
- )
+struct TriggerStep *
+sql_trigger_insert_step(struct Parse *parser, struct Token *table_name,
+ struct IdList *column_list, struct Select *select,
+ u8 orconf)
{
- TriggerStep *pTriggerStep;
-
- assert(pSelect != 0 || db->mallocFailed);
-
- pTriggerStep = triggerStepAllocate(db, TK_INSERT, pTableName);
- if (pTriggerStep) {
- pTriggerStep->pSelect =
- sqlSelectDup(db, pSelect, EXPRDUP_REDUCE);
- pTriggerStep->pIdList = pColumn;
- pTriggerStep->orconf = orconf;
+ struct sql *db = parser->db;
+ assert(select != 0 || db->mallocFailed);
+ struct TriggerStep *trigger_step =
+ sql_trigger_step_allocate(parser, TK_INSERT, table_name);
+ if (trigger_step != NULL) {
+ trigger_step->pSelect =
+ sqlSelectDup(db, select, EXPRDUP_REDUCE);
+ trigger_step->pIdList = column_list;
+ trigger_step->orconf = orconf;
} else {
- sqlIdListDelete(db, pColumn);
+ sqlIdListDelete(db, column_list);
}
- sql_select_delete(db, pSelect);
-
- return pTriggerStep;
+ sql_select_delete(db, select);
+ return trigger_step;
}
-/*
- * Construct a trigger step that implements an UPDATE statement and return
- * a pointer to that trigger step. The parser calls this routine when it
- * sees an UPDATE statement inside the body of a CREATE TRIGGER.
- */
-TriggerStep *
-sqlTriggerUpdateStep(sql * db, /* The database connection */
- Token * pTableName, /* Name of the table to be updated */
- ExprList * pEList, /* The SET clause: list of column and new values */
- Expr * pWhere, /* The WHERE clause */
- u8 orconf /* The conflict algorithm.
- * (ON_CONFLICT_ACTION_ABORT, _IGNORE,
- * etc)
- */
- )
+struct TriggerStep *
+sql_trigger_update_step(struct Parse *parser, struct Token *table_name,
+ struct ExprList *new_list, struct Expr *where,
+ u8 orconf)
{
- TriggerStep *pTriggerStep;
-
- pTriggerStep = triggerStepAllocate(db, TK_UPDATE, pTableName);
- if (pTriggerStep) {
- pTriggerStep->pExprList =
- sql_expr_list_dup(db, pEList, EXPRDUP_REDUCE);
- pTriggerStep->pWhere =
- sqlExprDup(db, pWhere, EXPRDUP_REDUCE);
- pTriggerStep->orconf = orconf;
+ struct sql *db = parser->db;
+ struct TriggerStep *trigger_step =
+ sql_trigger_step_allocate(parser, TK_UPDATE, table_name);
+ if (trigger_step != NULL) {
+ trigger_step->pExprList =
+ sql_expr_list_dup(db, new_list, EXPRDUP_REDUCE);
+ trigger_step->pWhere =
+ sqlExprDup(db, where, EXPRDUP_REDUCE);
+ trigger_step->orconf = orconf;
}
- sql_expr_list_delete(db, pEList);
- sql_expr_delete(db, pWhere, false);
- return pTriggerStep;
+ sql_expr_list_delete(db, new_list);
+ sql_expr_delete(db, where, false);
+ return trigger_step;
}
-/*
- * Construct a trigger step that implements a DELETE statement and return
- * a pointer to that trigger step. The parser calls this routine when it
- * sees a DELETE statement inside the body of a CREATE TRIGGER.
- */
-TriggerStep *
-sqlTriggerDeleteStep(sql * db, /* Database connection */
- Token * pTableName, /* The table from which rows are deleted */
- Expr * pWhere /* The WHERE clause */
- )
+struct TriggerStep *
+sql_trigger_delete_step(struct Parse *parser, struct Token * table_name,
+ struct Expr *where)
{
- TriggerStep *pTriggerStep;
-
- pTriggerStep = triggerStepAllocate(db, TK_DELETE, pTableName);
- if (pTriggerStep) {
- pTriggerStep->pWhere =
- sqlExprDup(db, pWhere, EXPRDUP_REDUCE);
- pTriggerStep->orconf = ON_CONFLICT_ACTION_DEFAULT;
+ struct sql *db = parser->db;
+ struct TriggerStep *trigger_step =
+ sql_trigger_step_allocate(parser, TK_DELETE, table_name);
+ if (trigger_step != NULL) {
+ trigger_step->pWhere =
+ sqlExprDup(db, where, EXPRDUP_REDUCE);
+ trigger_step->orconf = ON_CONFLICT_ACTION_DEFAULT;
}
- sql_expr_delete(db, pWhere, false);
- return pTriggerStep;
+ sql_expr_delete(db, where, false);
+ return trigger_step;
}
void
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v1 3/4] sql: patch sql_expr_create routine to use Parser
2019-02-15 13:30 [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 1/4] sql: patch sql_name_from_token to use Parser Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 2/4] sql: patch sql_trigger_step_allocate " Kirill Shcherbatov
@ 2019-02-15 13:30 ` Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 4/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
2019-02-22 12:20 ` [tarantool-patches] Re: [PATCH v1 0/4] " Vladislav Shpilevoy
4 siblings, 0 replies; 8+ messages in thread
From: Kirill Shcherbatov @ 2019-02-15 13:30 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
The code was refactored so that the sql_expr_create function
would use the parser object instead of database connection. Also
performed some additional names refactoring in adjacent places.
Needed for #3931
---
src/box/sql/build.c | 18 ++--
src/box/sql/expr.c | 202 +++++++++++++++-------------------------
src/box/sql/fkey.c | 124 ++++++++++++------------
src/box/sql/parse.y | 16 ++--
src/box/sql/resolve.c | 41 ++++----
src/box/sql/select.c | 46 +++++----
src/box/sql/sqlInt.h | 79 +++++++++++++++-
src/box/sql/wherecode.c | 3 +-
src/box/sql/whereexpr.c | 10 +-
9 files changed, 283 insertions(+), 256 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 3f4530620..a08148a97 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -691,8 +691,8 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */
struct Token token;
sqlTokenInit(&token, pTab->def->fields[iCol].name);
list = sql_expr_list_append(db, NULL,
- sqlExprAlloc(db, TK_ID,
- &token, 0));
+ sql_expr_create(pParse, TK_ID,
+ &token, false));
if (list == NULL)
goto primary_key_exit;
sql_create_index(pParse, 0, 0, list, 0, SORT_ORDER_ASC,
@@ -1455,10 +1455,11 @@ sql_id_eq_str_expr(struct Parse *parse, const char *col_name,
{
struct sql *db = parse->db;
- struct Expr *col_name_expr = sqlExpr(db, TK_ID, col_name);
+ struct Expr *col_name_expr = sql_op_expr_create(parse, TK_ID, col_name);
if (col_name_expr == NULL)
return NULL;
- struct Expr *col_value_expr = sqlExpr(db, TK_STRING, col_value);
+ struct Expr *col_value_expr =
+ sql_op_expr_create(parse, TK_STRING, col_value);
if (col_value_expr == NULL) {
sql_expr_delete(db, col_name_expr, false);
return NULL;
@@ -1480,12 +1481,12 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
if (idx_name != NULL) {
struct Expr *expr = sql_id_eq_str_expr(parse, "idx", idx_name);
if (expr != NULL)
- where = sqlExprAnd(db, expr, where);
+ where = sql_and_expr_create(parse, expr, where);
}
if (table_name != NULL) {
struct Expr *expr = sql_id_eq_str_expr(parse, "tbl", table_name);
if (expr != NULL)
- where = sqlExprAnd(db, expr, where);
+ where = sql_and_expr_create(parse, expr, where);
}
/**
* On memory allocation error sql_table delete_from
@@ -2339,8 +2340,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
uint32_t last_field = def->field_count - 1;
sqlTokenInit(&prev_col, def->fields[last_field].name);
col_list = sql_expr_list_append(parse->db, NULL,
- sqlExprAlloc(db, TK_ID,
- &prev_col, 0));
+ sql_expr_create(parse, TK_ID,
+ &prev_col,
+ false));
if (col_list == NULL)
goto exit_create_index;
assert(col_list->nExpr == 1);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a6e35df6d..42531c107 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -151,8 +151,7 @@ sqlExprAddCollateToken(Parse * pParse, /* Parsing context */
{
if (pCollName->n > 0) {
Expr *pNew =
- sqlExprAlloc(pParse->db, TK_COLLATE, pCollName,
- dequote);
+ sql_expr_create(pParse, TK_COLLATE, pCollName, dequote);
if (pNew) {
pNew->pLeft = pExpr;
pNew->flags |= EP_Collate | EP_Skip;
@@ -795,113 +794,60 @@ sqlExprSetHeightAndFlags(Parse * pParse, Expr * p)
#define exprSetHeight(y)
#endif /* SQL_MAX_EXPR_DEPTH>0 */
-/*
- * This routine is the core allocator for Expr nodes.
- *
- * Construct a new expression node and return a pointer to it. Memory
- * for this node and for the pToken argument is a single allocation
- * obtained from sqlDbMalloc(). The calling function
- * is responsible for making sure the node eventually gets freed.
- *
- * If dequote is true, then the token (if it exists) is dequoted.
- * If dequote is false, no dequoting is performed. The deQuote
- * parameter is ignored if pToken is NULL or if the token does not
- * appear to be quoted. If the quotes were of the form "..." (double-quotes)
- * then the EP_DblQuoted flag is set on the expression node.
- *
- * Special case: If op==TK_INTEGER and pToken points to a string that
- * can be translated into a 32-bit integer, then the token is not
- * stored in u.zToken. Instead, the integer values is written
- * into u.iValue and the EP_IntValue flag is set. No extra storage
- * is allocated to hold the integer text and the dequote flag is ignored.
- */
-Expr *
-sqlExprAlloc(sql * db, /* Handle for sqlDbMallocRawNN() */
- int op, /* Expression opcode */
- const Token * pToken, /* Token argument. Might be NULL */
- int dequote /* True to dequote */
- )
+struct Expr *
+sql_expr_create(struct Parse *parser, int op, const Token *token, bool dequote)
{
- Expr *pNew;
- int nExtra = 0;
- int iValue = 0;
-
- assert(db != 0);
- if (pToken) {
- if (op != TK_INTEGER || pToken->z == 0
- || sqlGetInt32(pToken->z, &iValue) == 0) {
- nExtra = pToken->n + 1;
- assert(iValue >= 0);
+ int extra_sz = 0;
+ int val = 0;
+ if (token != NULL) {
+ if (op != TK_INTEGER || token->z == NULL ||
+ sqlGetInt32(token->z, &val) == 0) {
+ extra_sz = token->n + 1;
+ assert(val >= 0);
}
}
- pNew = sqlDbMallocRawNN(db, sizeof(Expr) + nExtra);
- if (pNew) {
- memset(pNew, 0, sizeof(Expr));
- pNew->op = (u8) op;
- pNew->iAgg = -1;
- if (pToken) {
- if (nExtra == 0) {
- pNew->flags |= EP_IntValue;
- pNew->u.iValue = iValue;
- } else {
- pNew->u.zToken = (char *)&pNew[1];
- assert(pToken->z != 0 || pToken->n == 0);
- if (pToken->n)
- memcpy(pNew->u.zToken, pToken->z,
- pToken->n);
- pNew->u.zToken[pToken->n] = 0;
- if (dequote){
- if (pNew->u.zToken[0] == '"')
- pNew->flags |= EP_DblQuoted;
- if (pNew->op == TK_ID ||
- pNew->op == TK_COLLATE ||
- pNew->op == TK_FUNCTION){
- sqlNormalizeName(pNew->u.zToken);
- }else{
- sqlDequote(pNew->u.zToken);
- }
- }
- }
- }
-#if SQL_MAX_EXPR_DEPTH>0
- pNew->nHeight = 1;
+ struct Expr *expr =
+ sqlDbMallocRawNN(parser->db, sizeof(*expr) + extra_sz);
+ if (expr == NULL)
+ return NULL;
+
+ memset(expr, 0, sizeof(*expr));
+ expr->op = (u8)op;
+ expr->iAgg = -1;
+#if SQL_MAX_EXPR_DEPTH > 0
+ expr->nHeight = 1;
#endif
- }
- return pNew;
-}
+ if (token == NULL)
+ return expr;
-/*
- * Allocate a new expression node from a zero-terminated token that has
- * already been dequoted.
- */
-Expr *
-sqlExpr(sql * db, /* Handle for sqlDbMallocZero() (may be null) */
- int op, /* Expression opcode */
- const char *zToken /* Token argument. Might be NULL */
- )
-{
- Token x;
- x.z = zToken;
- x.n = zToken ? sqlStrlen30(zToken) : 0;
- return sqlExprAlloc(db, op, &x, 0);
+ if (extra_sz == 0) {
+ expr->flags |= EP_IntValue;
+ expr->u.iValue = val;
+ } else {
+ expr->u.zToken = (char *)&expr[1];
+ assert(token->z != NULL || token->n == 0);
+ memcpy(expr->u.zToken, token->z, token->n);
+ expr->u.zToken[token->n] = '\0';
+ if (dequote) {
+ if (expr->u.zToken[0] == '"')
+ expr->flags |= EP_DblQuoted;
+ if (expr->op == TK_ID || expr->op == TK_COLLATE ||
+ expr->op == TK_FUNCTION)
+ sqlNormalizeName(expr->u.zToken);
+ else
+ sqlDequote(expr->u.zToken);
+ }
+ }
+ return expr;
}
-/* Allocate a new expression and initialize it as integer.
- * @param db sql engine.
- * @param value Value to initialize by.
- *
- * @retval not NULL Allocated and initialized expr.
- * @retval NULL Memory error.
- */
-Expr *
-sqlExprInteger(sql * db, int value)
+struct Expr *
+sql_op_expr_create(struct Parse *parser, int op, const char *name)
{
- Expr *ret = sqlExpr(db, TK_INTEGER, NULL);
- if (ret != NULL) {
- ret->flags = EP_IntValue;
- ret->u.iValue = value;
- }
- return ret;
+ struct Token name_token;
+ name_token.z = name;
+ name_token.n = name != NULL ? strlen(name) : 0;
+ return sql_expr_create(parser, op, &name_token, false);
}
/*
@@ -947,8 +893,11 @@ sqlPExpr(Parse * pParse, /* Parsing context */
{
Expr *p;
if (op == TK_AND && pParse->nErr == 0) {
- /* Take advantage of short-circuit false optimization for AND */
- p = sqlExprAnd(pParse->db, pLeft, pRight);
+ /*
+ * Take advantage of short-circuit false
+ * optimization for AND.
+ */
+ p = sql_and_expr_create(pParse, pLeft, pRight);
} else {
p = sqlDbMallocRawNN(pParse->db, sizeof(Expr));
if (p) {
@@ -1017,30 +966,25 @@ exprAlwaysFalse(Expr * p)
return v == 0;
}
-/*
- * Join two expressions using an AND operator. If either expression is
- * NULL, then just return the other expression.
- *
- * If one side or the other of the AND is known to be false, then instead
- * of returning an AND expression, just return a constant expression with
- * a value of false.
- */
-Expr *
-sqlExprAnd(sql * db, Expr * pLeft, Expr * pRight)
+struct Expr *
+sql_and_expr_create(struct Parse *parser, struct Expr *left_expr,
+ struct Expr *right_expr)
{
- if (pLeft == 0) {
- return pRight;
- } else if (pRight == 0) {
- return pLeft;
- } else if (exprAlwaysFalse(pLeft) || exprAlwaysFalse(pRight)) {
- sql_expr_delete(db, pLeft, false);
- sql_expr_delete(db, pRight, false);
- return sqlExprAlloc(db, TK_INTEGER, &sqlIntTokens[0],
- 0);
+ if (left_expr == NULL) {
+ return right_expr;
+ } else if (right_expr == NULL) {
+ return left_expr;
+ } else if (exprAlwaysFalse(left_expr) || exprAlwaysFalse(right_expr)) {
+ sql_expr_delete(parser->db, left_expr, false);
+ sql_expr_delete(parser->db, right_expr, false);
+ return sql_expr_create(parser, TK_INTEGER, &sqlIntTokens[0],
+ false);
} else {
- Expr *pNew = sqlExprAlloc(db, TK_AND, 0, 0);
- sqlExprAttachSubtrees(db, pNew, pLeft, pRight);
- return pNew;
+ struct Expr *new_expr =
+ sql_expr_create(parser, TK_AND, NULL, false);
+ sqlExprAttachSubtrees(parser->db, new_expr, left_expr,
+ right_expr);
+ return new_expr;
}
}
@@ -1054,7 +998,7 @@ sqlExprFunction(Parse * pParse, ExprList * pList, Token * pToken)
Expr *pNew;
sql *db = pParse->db;
assert(pToken);
- pNew = sqlExprAlloc(db, TK_FUNCTION, pToken, 1);
+ pNew = sql_expr_create(pParse, TK_FUNCTION, pToken, true);
if (pNew == 0) {
sql_expr_list_delete(db, pList); /* Avoid memory leak when malloc fails */
return 0;
@@ -2860,9 +2804,9 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
}
if (pSel->pLimit == NULL) {
pSel->pLimit =
- sqlExprAlloc(pParse->db, TK_INTEGER,
- &sqlIntTokens[1],
- 0);
+ sql_expr_create(pParse, TK_INTEGER,
+ &sqlIntTokens[1],
+ false);
if (pSel->pLimit != NULL) {
ExprSetProperty(pSel->pLimit,
EP_System);
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 69740b39f..8edafe149 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -308,50 +308,51 @@ fkey_lookup_parent(struct Parse *parse_context, struct space *parent,
* regBase is the first of an array of register that contains the data
* for pTab. regBase+1 holds the first column.
* regBase+2 holds the second column, and so forth.
+ * @param parser The parsing context.
+ * @param table The table whose content is at r[regBase]...
+ * @param reg_base Contents of table table.
+ * @param column Index of table column is desired.
+ * @retval no NULL expression pointer on success.
+ * @retval NULL otherwise.
*/
-static Expr *
-exprTableRegister(Parse * pParse, /* Parsing and code generating context */
- Table * pTab, /* The table whose content is at r[regBase]... */
- int regBase, /* Contents of table pTab */
- i16 iCol /* Which column of pTab is desired */
- )
+static struct Expr *
+sql_register_expr_create(struct Parse *parser, struct Table *table,
+ int reg_base, int column)
{
- Expr *pExpr;
- sql *db = pParse->db;
-
- pExpr = sqlExpr(db, TK_REGISTER, 0);
- if (pExpr) {
- if (iCol >= 0) {
- pExpr->iTable = regBase + iCol + 1;
- pExpr->type = pTab->def->fields[iCol].type;
- } else {
- pExpr->iTable = regBase;
- pExpr->type = FIELD_TYPE_INTEGER;
- }
+ struct Expr *expr = sql_op_expr_create(parser, TK_REGISTER, NULL);
+ if (expr == NULL)
+ return NULL;
+ if (column >= 0) {
+ expr->iTable = reg_base + column + 1;
+ expr->type = table->def->fields[column].type;
+ } else {
+ expr->iTable = reg_base;
+ expr->type = FIELD_TYPE_INTEGER;
}
- return pExpr;
+ return expr;
}
/**
* Return an Expr object that refers to column of space_def which
* has cursor cursor.
- * @param db The database connection.
+ * @param parser The parsing context.
* @param def space definition.
* @param cursor The open cursor on the table.
* @param column The column that is wanted.
* @retval not NULL on success.
* @retval NULL on error.
*/
-static Expr *
-exprTableColumn(sql * db, struct space_def *def, int cursor, i16 column)
+static struct Expr *
+sql_column_cursor_expr_create(struct Parse *parser, struct space_def *def,
+ int cursor, int column)
{
- Expr *pExpr = sqlExpr(db, TK_COLUMN, 0);
- if (pExpr) {
- pExpr->space_def = def;
- pExpr->iTable = cursor;
- pExpr->iColumn = column;
- }
- return pExpr;
+ struct Expr *expr = sql_op_expr_create(parser, TK_COLUMN, NULL);
+ if (expr == NULL)
+ return NULL;
+ expr->space_def = def;
+ expr->iTable = cursor;
+ expr->iColumn = column;
+ return expr;
}
/*
@@ -430,12 +431,14 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src, struct Table *tab,
for (uint32_t i = 0; i < fkey->field_count; i++) {
uint32_t fieldno = fkey->links[i].parent_field;
struct Expr *pexpr =
- exprTableRegister(parser, tab, reg_data, fieldno);
+ sql_register_expr_create(parser, tab, reg_data,
+ fieldno);
fieldno = fkey->links[i].child_field;
const char *field_name = child_space->def->fields[fieldno].name;
- struct Expr *chexpr = sqlExpr(db, TK_ID, field_name);
+ struct Expr *chexpr =
+ sql_op_expr_create(parser, TK_ID, field_name);
struct Expr *eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr);
- where = sqlExprAnd(db, where, eq);
+ where = sql_and_expr_create(parser, where, eq);
}
/*
@@ -451,15 +454,16 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src, struct Table *tab,
struct Expr *expr = NULL, *pexpr, *chexpr, *eq;
for (uint32_t i = 0; i < fkey->field_count; i++) {
uint32_t fieldno = fkey->links[i].parent_field;
- pexpr = exprTableRegister(parser, tab, reg_data,
- fieldno);
- chexpr = exprTableColumn(db, tab->def,
- src->a[0].iCursor, fieldno);
+ pexpr = sql_register_expr_create(parser, tab, reg_data,
+ fieldno);
+ int cursor = src->a[0].iCursor;
+ chexpr = sql_column_cursor_expr_create(parser, tab->def,
+ cursor, fieldno);
eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr);
- expr = sqlExprAnd(db, expr, eq);
+ expr = sql_and_expr_create(parser, expr, eq);
}
struct Expr *pNe = sqlPExpr(parser, TK_NOT, expr, 0);
- where = sqlExprAnd(db, where, pNe);
+ where = sql_and_expr_create(parser, where, pNe);
}
/* Resolve the references in the WHERE clause. */
@@ -788,12 +792,13 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
*/
struct Expr *to_col =
sqlPExpr(pParse, TK_DOT,
- sqlExprAlloc(db, TK_ID, &t_old, 0),
- sqlExprAlloc(db, TK_ID, &t_to_col, 0));
+ sql_expr_create(pParse, TK_ID, &t_old, false),
+ sql_expr_create(pParse, TK_ID, &t_to_col,
+ false));
struct Expr *from_col =
- sqlExprAlloc(db, TK_ID, &t_from_col, 0);
+ sql_expr_create(pParse, TK_ID, &t_from_col, false);
struct Expr *eq = sqlPExpr(pParse, TK_EQ, to_col, from_col);
- where = sqlExprAnd(db, where, eq);
+ where = sql_and_expr_create(pParse, where, eq);
/*
* For ON UPDATE, construct the next term of the
@@ -812,11 +817,13 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
*/
if (is_update) {
struct Expr *old_val = sqlPExpr(pParse, TK_DOT,
- sqlExprAlloc(db, TK_ID, &t_old, 0),
- sqlExprAlloc(db, TK_ID, &t_to_col, 0));
+ sql_expr_create(pParse, TK_ID, &t_old, false),
+ sql_expr_create(pParse, TK_ID, &t_to_col,
+ false));
struct Expr *new_val = sqlPExpr(pParse, TK_DOT,
- sqlExprAlloc(db, TK_ID, &t_new, 0),
- sqlExprAlloc(db, TK_ID, &t_to_col, 0));
+ sql_expr_create(pParse, TK_ID, &t_new, false),
+ sql_expr_create(pParse, TK_ID, &t_to_col,
+ false));
struct Expr *old_is_null = sqlPExpr(
pParse, TK_ISNULL,
sqlExprDup(db, old_val, 0), NULL);
@@ -829,7 +836,8 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
struct Expr *no_action_needed =
sqlPExpr(pParse, TK_OR, old_is_null,
non_null_eq);
- when = sqlExprAnd(db, when, no_action_needed);
+ when = sql_and_expr_create(pParse, when,
+ no_action_needed);
}
if (action != FKEY_ACTION_RESTRICT &&
@@ -837,21 +845,22 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
struct Expr *new, *d;
if (action == FKEY_ACTION_CASCADE) {
new = sqlPExpr(pParse, TK_DOT,
- sqlExprAlloc(db, TK_ID,
- &t_new, 0),
- sqlExprAlloc(db, TK_ID,
- &t_to_col,
- 0));
+ sql_expr_create(pParse, TK_ID,
+ &t_new, false),
+ sql_expr_create(pParse, TK_ID,
+ &t_to_col,
+ false));
} else if (action == FKEY_ACTION_SET_DEFAULT) {
d = child_fields[chcol].default_value_expr;
if (d != NULL) {
new = sqlExprDup(db, d, 0);
} else {
- new = sqlExprAlloc(db, TK_NULL,
- NULL, 0);
+ new = sql_expr_create(pParse, TK_NULL,
+ NULL, false);
}
} else {
- new = sqlExprAlloc(db, TK_NULL, NULL, 0);
+ new = sql_expr_create(pParse, TK_NULL, NULL,
+ false);
}
list = sql_expr_list_append(db, list, new);
sqlExprListSetName(pParse, list, &t_from_col, 0);
@@ -865,8 +874,9 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
struct Token err;
err.z = space_name;
err.n = name_len;
- struct Expr *r = sqlExpr(db, TK_RAISE, "FOREIGN KEY "\
- "constraint failed");
+ struct Expr *r =
+ sql_op_expr_create(pParse, TK_RAISE,
+ "FOREIGN KEY constraint failed");
if (r != NULL)
r->on_conflict_action = ON_CONFLICT_ACTION_ABORT;
select = sqlSelectNew(pParse,
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1a18d5f89..d7b721695 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -523,12 +523,12 @@ selcollist(A) ::= sclp(A) expr(X) as(Y). {
sqlExprListSetSpan(pParse,A,&X);
}
selcollist(A) ::= sclp(A) STAR. {
- Expr *p = sqlExpr(pParse->db, TK_ASTERISK, 0);
+ struct Expr *p = sql_op_expr_create(pParse, TK_ASTERISK, NULL);
A = sql_expr_list_append(pParse->db, A, p);
}
selcollist(A) ::= sclp(A) nm(X) DOT STAR. {
Expr *pRight = sqlPExpr(pParse, TK_ASTERISK, 0, 0);
- Expr *pLeft = sqlExprAlloc(pParse->db, TK_ID, &X, 1);
+ struct Expr *pLeft = sql_expr_create(pParse, TK_ID, &X, true);
Expr *pDot = sqlPExpr(pParse, TK_DOT, pLeft, pRight);
A = sql_expr_list_append(pParse->db,A, pDot);
}
@@ -868,15 +868,15 @@ term(A) ::= NULL(X). {spanExpr(&A,pParse,@X,X);/*A-overwrites-X*/}
expr(A) ::= id(X). {spanExpr(&A,pParse,TK_ID,X); /*A-overwrites-X*/}
expr(A) ::= JOIN_KW(X). {spanExpr(&A,pParse,TK_ID,X); /*A-overwrites-X*/}
expr(A) ::= nm(X) DOT nm(Y). {
- Expr *temp1 = sqlExprAlloc(pParse->db, TK_ID, &X, 1);
- Expr *temp2 = sqlExprAlloc(pParse->db, TK_ID, &Y, 1);
+ struct Expr *temp1 = sql_expr_create(pParse, TK_ID, &X, true);
+ struct Expr *temp2 = sql_expr_create(pParse, TK_ID, &Y, true);
spanSet(&A,&X,&Y); /*A-overwrites-X*/
A.pExpr = sqlPExpr(pParse, TK_DOT, temp1, temp2);
}
term(A) ::= FLOAT|BLOB(X). {spanExpr(&A,pParse,@X,X);/*A-overwrites-X*/}
term(A) ::= STRING(X). {spanExpr(&A,pParse,@X,X);/*A-overwrites-X*/}
term(A) ::= INTEGER(X). {
- A.pExpr = sqlExprAlloc(pParse->db, TK_INTEGER, &X, 1);
+ A.pExpr = sql_expr_create(pParse, TK_INTEGER, &X, true);
A.pExpr->type = FIELD_TYPE_INTEGER;
A.zStart = X.z;
A.zEnd = X.z + X.n;
@@ -909,7 +909,7 @@ expr(A) ::= expr(A) COLLATE id(C). {
%ifndef SQL_OMIT_CAST
expr(A) ::= CAST(X) LP expr(E) AS typedef(T) RP(Y). {
spanSet(&A,&X,&Y); /*A-overwrites-X*/
- A.pExpr = sqlExprAlloc(pParse->db, TK_CAST, 0, 1);
+ A.pExpr = sql_expr_create(pParse, TK_CAST, NULL, true);
A.pExpr->type = T.type;
sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0);
}
@@ -1099,7 +1099,7 @@ expr(A) ::= expr(A) in_op(N) LP exprlist(Y) RP(E). [IN] {
** regardless of the value of expr1.
*/
sql_expr_delete(pParse->db, A.pExpr, false);
- A.pExpr = sqlExprAlloc(pParse->db, TK_INTEGER,&sqlIntTokens[N],1);
+ A.pExpr = sql_expr_create(pParse, TK_INTEGER, &sqlIntTokens[N], true);
}else if( Y->nExpr==1 ){
/* Expressions of the form:
**
@@ -1421,7 +1421,7 @@ expr(A) ::= RAISE(X) LP IGNORE RP(Y). {
}
expr(A) ::= RAISE(X) LP raisetype(T) COMMA STRING(Z) RP(Y). {
spanSet(&A,&X,&Y); /*A-overwrites-X*/
- A.pExpr = sqlExprAlloc(pParse->db, TK_RAISE, &Z, 1);
+ A.pExpr = sql_expr_create(pParse, TK_RAISE, &Z, true);
if( A.pExpr ) {
A.pExpr->on_conflict_action = (enum on_conflict_action) T;
}
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 029eb4055..22397d6f5 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -479,26 +479,20 @@ lookupName(Parse * pParse, /* The parsing context */
}
}
-/*
- * Allocate and return a pointer to an expression to load the column iCol
- * from datasource iSrc in SrcList pSrc.
- */
-Expr *
-sqlCreateColumnExpr(sql * db, SrcList * pSrc, int iSrc, int iCol)
+struct Expr *
+sql_column_expr_create(struct Parse *parser, struct SrcList *src_list,
+ int src_idx, int column)
{
- Expr *p = sqlExprAlloc(db, TK_COLUMN, 0, 0);
- if (p) {
- struct SrcList_item *pItem = &pSrc->a[iSrc];
- p->space_def = pItem->pTab->def;
- p->iTable = pItem->iCursor;
- p->iColumn = (ynVar) iCol;
- testcase(iCol == BMS);
- testcase(iCol == BMS - 1);
- pItem->colUsed |=
- ((Bitmask) 1) << (iCol >= BMS ? BMS - 1 : iCol);
- ExprSetProperty(p, EP_Resolved);
- }
- return p;
+ struct Expr *expr = sql_expr_create(parser, TK_COLUMN, NULL, true);
+ if (expr == NULL)
+ return NULL;
+ struct SrcList_item *pItem = &src_list->a[src_idx];
+ expr->space_def = pItem->pTab->def;
+ expr->iTable = pItem->iCursor;
+ expr->iColumn = column;
+ pItem->colUsed |= ((Bitmask) 1) << (column >= BMS ? BMS - 1 : column);
+ ExprSetProperty(expr, EP_Resolved);
+ return expr;
}
/*
@@ -999,7 +993,9 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages
/* Convert the ORDER BY term into an integer column number iCol,
* taking care to preserve the COLLATE clause if it exists
*/
- Expr *pNew = sqlExpr(db, TK_INTEGER, 0);
+ Expr *pNew =
+ sql_op_expr_create(pParse, TK_INTEGER,
+ NULL);
if (pNew == 0)
return 1;
pNew->flags |= EP_IntValue;
@@ -1347,9 +1343,8 @@ resolveSelectStep(Walker * pWalker, Select * p)
* restrict it directly).
*/
sql_expr_delete(db, p->pLimit, false);
- p->pLimit =
- sqlExprAlloc(db, TK_INTEGER,
- &sqlIntTokens[1], 0);
+ p->pLimit = sql_expr_create(pParse, TK_INTEGER,
+ &sqlIntTokens[1], false);
} else {
if (sqlResolveExprNames(&sNC, p->pHaving))
return WRC_Abort;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index dfd3a375e..5a1bc0c81 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -165,7 +165,9 @@ sqlSelectNew(Parse * pParse, /* Parsing context */
}
if (pEList == 0) {
pEList = sql_expr_list_append(pParse->db, NULL,
- sqlExpr(db, TK_ASTERISK, 0));
+ sql_op_expr_create(pParse,
+ TK_ASTERISK,
+ NULL));
}
struct session MAYBE_UNUSED *user_session;
user_session = current_session();
@@ -483,7 +485,6 @@ addWhereTerm(Parse * pParse, /* Parsing context */
int isOuterJoin, /* True if this is an OUTER join */
Expr ** ppWhere) /* IN/OUT: The WHERE clause to add to */
{
- sql *db = pParse->db;
Expr *pE1;
Expr *pE2;
Expr *pEq;
@@ -493,8 +494,8 @@ addWhereTerm(Parse * pParse, /* Parsing context */
assert(pSrc->a[iLeft].pTab);
assert(pSrc->a[iRight].pTab);
- pE1 = sqlCreateColumnExpr(db, pSrc, iLeft, iColLeft);
- pE2 = sqlCreateColumnExpr(db, pSrc, iRight, iColRight);
+ pE1 = sql_column_expr_create(pParse, pSrc, iLeft, iColLeft);
+ pE2 = sql_column_expr_create(pParse, pSrc, iRight, iColRight);
pEq = sqlPExpr(pParse, TK_EQ, pE1, pE2);
if (pEq && isOuterJoin) {
@@ -503,7 +504,7 @@ addWhereTerm(Parse * pParse, /* Parsing context */
ExprSetVVAProperty(pEq, EP_NoReduce);
pEq->iRightJoinTable = (i16) pE2->iTable;
}
- *ppWhere = sqlExprAnd(db, *ppWhere, pEq);
+ *ppWhere = sql_and_expr_create(pParse, *ppWhere, pEq);
}
/*
@@ -624,8 +625,8 @@ sqlProcessJoin(Parse * pParse, Select * p)
if (pRight->pOn) {
if (isOuter)
setJoinExpr(pRight->pOn, pRight->iCursor);
- p->pWhere =
- sqlExprAnd(pParse->db, p->pWhere, pRight->pOn);
+ p->pWhere = sql_and_expr_create(pParse, p->pWhere,
+ pRight->pOn);
pRight->pOn = 0;
}
@@ -3314,7 +3315,9 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */
break;
}
if (j == nOrderBy) {
- Expr *pNew = sqlExpr(db, TK_INTEGER, 0);
+ Expr *pNew =
+ sql_op_expr_create(pParse, TK_INTEGER,
+ NULL);
if (pNew == 0)
return SQL_NOMEM_BKPT;
pNew->flags |= EP_IntValue;
@@ -4201,17 +4204,18 @@ flattenSubquery(Parse * pParse, /* Parsing context */
assert(pParent->pHaving == 0);
pParent->pHaving = pParent->pWhere;
pParent->pWhere = pWhere;
- pParent->pHaving = sqlExprAnd(db,
- sqlExprDup(db,
- pSub->pHaving,
- 0),
- pParent->pHaving);
+ pParent->pHaving =
+ sql_and_expr_create(pParse,
+ sqlExprDup(db,
+ pSub->pHaving, 0),
+ pParent->pHaving);
assert(pParent->pGroupBy == 0);
pParent->pGroupBy =
sql_expr_list_dup(db, pSub->pGroupBy, 0);
} else {
pParent->pWhere =
- sqlExprAnd(db, pWhere, pParent->pWhere);
+ sql_and_expr_create(pParse, pWhere,
+ pParent->pWhere);
}
substSelect(pParse, pParent, iParent, pSub->pEList, 0);
@@ -4317,7 +4321,8 @@ pushDownWhereTerms(Parse * pParse, /* Parse context (for malloc() and error repo
pNew = sqlExprDup(pParse->db, pWhere, 0);
pNew = substExpr(pParse, pNew, iCursor, pSubq->pEList);
pSubq->pWhere =
- sqlExprAnd(pParse->db, pSubq->pWhere, pNew);
+ sql_and_expr_create(pParse, pSubq->pWhere,
+ pNew);
pSubq = pSubq->pPrior;
}
}
@@ -4500,7 +4505,8 @@ convertCompoundSelectToSubquery(Walker * pWalker, Select * p)
*pNew = *p;
p->pSrc = pNewSrc;
p->pEList = sql_expr_list_append(pParse->db, NULL,
- sqlExpr(db, TK_ASTERISK, 0));
+ sql_op_expr_create(pParse, TK_ASTERISK,
+ NULL));
p->op = TK_SELECT;
p->pWhere = 0;
pNew->pGroupBy = 0;
@@ -5002,16 +5008,16 @@ selectExpander(Walker * pWalker, Select * p)
continue;
}
}
- pRight =
- sqlExpr(db, TK_ID,
+ pRight = sql_op_expr_create(
+ pParse, TK_ID,
zName);
zColname = zName;
zToFree = 0;
if (longNames
|| pTabList->nSrc > 1) {
Expr *pLeft;
- pLeft =
- sqlExpr(db,
+ pLeft = sql_op_expr_create(
+ pParse,
TK_ID,
zTabName);
pExpr =
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 99c8adbdd..048c814e5 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3292,13 +3292,69 @@ void sqlClearTempRegCache(Parse *);
#ifdef SQL_DEBUG
int sqlNoTempsInRange(Parse *, int, int);
#endif
-Expr *sqlExprAlloc(sql *, int, const Token *, int);
-Expr *sqlExpr(sql *, int, const char *);
-Expr *sqlExprInteger(sql *, int);
+
+/*
+ * This routine is the core allocator for Expr nodes.
+ * Construct a new expression node and return a pointer to it.
+ * Memory for this node and for the token argument is a single
+ * allocation obtained from sqlDbMalloc(). The calling function
+ * is responsible for making sure the node eventually gets freed.
+ *
+ * If dequote is true, then the token (if it exists) is dequoted.
+ * If dequote is false, no dequoting is performed. The deQuote
+ * parameter is ignored if token is NULL or if the token does
+ * not appear to be quoted. If the quotes were of the form "..."
+ * (double-quotes) then the EP_DblQuoted flag is set on the
+ * expression node.
+ *
+ * Special case: If op==TK_INTEGER and token points to a string
+ * that can be translated into a 32-bit integer, then the token is
+ * not stored in u.zToken. Instead, the integer values is written
+ * into u.iValue and the EP_IntValue flag is set. No extra storage
+ * is allocated to hold the integer text and the dequote flag is
+ * ignored.
+ * @param parser The parsing context.
+ * @param op Expression opcode (TK_*).
+ * @param token Source token. Might be NULL.
+ * @param dequote True to dequote string.
+ * @retval not NULL new expression object on success.
+ * @retval NULL otherwise.
+ */
+struct Expr *
+sql_expr_create(struct Parse *parser, int op, const Token *token, bool dequote);
+
+/*
+ * Allocate a new expression node from a zero-terminated token
+ * that has already been dequoted.
+ * @param parser The parsing context.
+ * @param op Expression opcode.
+ * @param name The object name string.
+ * @retval not NULL expression pointer on success, NULL otherwise.
+ */
+struct Expr *
+sql_op_expr_create(struct Parse *parser, int op, const char *name);
+
void sqlExprAttachSubtrees(sql *, Expr *, Expr *, Expr *);
Expr *sqlPExpr(Parse *, int, Expr *, Expr *);
void sqlPExprAddSelect(Parse *, Expr *, Select *);
-Expr *sqlExprAnd(sql *, Expr *, Expr *);
+
+/*
+ * Join two expressions using an AND operator. If either
+ * expression is NULL, then just return the other expression.
+ *
+ * If one side or the other of the AND is known to be false, then
+ * instead of returning an AND expression, just return a constant
+ * expression with a value of false.
+ * @param parser The parsing context.
+ * @param left_expr The left-branch expresion to join.
+ * @param right_expr The right-branch expression to join.
+ * @retval not NULL new expression root node pointer on success.
+ * @retval NULL otherwise.
+ */
+struct Expr *
+sql_and_expr_create(struct Parse *parser, struct Expr *left_expr,
+ struct Expr *right_expr);
+
Expr *sqlExprFunction(Parse *, ExprList *, Token *);
void sqlExprAssignVarNumber(Parse *, Expr *, u32);
ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *);
@@ -4725,7 +4781,20 @@ void sqlAppendChar(StrAccum *, int, char);
char *sqlStrAccumFinish(StrAccum *);
void sqlStrAccumReset(StrAccum *);
void sqlSelectDestInit(SelectDest *, int, int, int);
-Expr *sqlCreateColumnExpr(sql *, SrcList *, int, int);
+
+/*
+ * Allocate and return a pointer to an expression to load the
+ * column from datasource src_idx in SrcList src_list.
+ * @param parser The parsing context.
+ * @param src_list The source list described with FROM clause.
+ * @param src_idx The resource index to use in src_list.
+ * @param column The column index.
+ * @retval not NULL expression pointer on success.
+ * @retval NULL otherwise.
+ */
+struct Expr *
+sql_column_expr_create(struct Parse *parser, struct SrcList *src_list,
+ int src_idx, int column);
int sqlExprCheckIN(Parse *, Expr *);
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index a0838a26b..23af1693d 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1437,7 +1437,8 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W
continue;
testcase(pWC->a[iTerm].wtFlags & TERM_ORINFO);
pExpr = sqlExprDup(db, pExpr, 0);
- pAndExpr = sqlExprAnd(db, pAndExpr, pExpr);
+ pAndExpr = sql_and_expr_create(pParse, pAndExpr,
+ pExpr);
}
if (pAndExpr) {
pAndExpr =
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 611a3896d..79cd09d4e 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -303,7 +303,7 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix,
Expr *pPrefix;
*pisComplete = c == MATCH_ALL_WILDCARD &&
z[cnt + 1] == 0;
- pPrefix = sqlExpr(db, TK_STRING, z);
+ pPrefix = sql_op_expr_create(pParse, TK_STRING, z);
if (pPrefix)
pPrefix->u.zToken[cnt] = 0;
*ppPrefix = pPrefix;
@@ -1297,9 +1297,9 @@ exprAnalyze(SrcList * pSrc, /* the FROM clause */
int idxNew;
WhereTerm *pNewTerm;
- pNewExpr = sqlPExpr(pParse, TK_GT,
- sqlExprDup(db, pLeft, 0),
- sqlExprAlloc(db, TK_NULL, 0, 0));
+ pNewExpr = sqlPExpr(pParse, TK_GT, sqlExprDup(db, pLeft, 0),
+ sql_expr_create(pParse, TK_NULL, NULL,
+ false));
idxNew = whereClauseInsert(pWC, pNewExpr,
TERM_VIRTUAL | TERM_DYNAMIC |
@@ -1495,7 +1495,7 @@ sqlWhereTabFuncArgs(Parse * pParse, /* Parsing context */
pTab->def->name, j);
return;
}
- pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0);
+ pColRef = sql_expr_create(pParse, TK_COLUMN, NULL, false);
if (pColRef == 0)
return;
pColRef->iTable = pItem->iCursor;
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v1 4/4] sql: store regular identifiers in case-normal form
2019-02-15 13:30 [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
` (2 preceding siblings ...)
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 3/4] sql: patch sql_expr_create routine " Kirill Shcherbatov
@ 2019-02-15 13:30 ` Kirill Shcherbatov
2019-02-22 12:20 ` [tarantool-patches] Re: [PATCH v1 0/4] " Vladislav Shpilevoy
4 siblings, 0 replies; 8+ messages in thread
From: Kirill Shcherbatov @ 2019-02-15 13:30 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Introduced a new sql_normalize_name routine performing SQL name
conversion to case-normal form via unicode character folding.
For example, ß is converted to SS. The result is similar to SQL
UPPER function.
Closes #3931
---
src/box/lua/lua_sql.c | 11 ++--
src/box/sql/build.c | 38 ++++++++-----
src/box/sql/expr.c | 78 ++++++++++++++++++---------
src/box/sql/parse.y | 26 +++++++--
src/box/sql/select.c | 20 +++++--
src/box/sql/sqlInt.h | 26 ++++++++-
src/box/sql/trigger.c | 18 +++++--
src/box/sql/util.c | 42 +++++++++------
test/sql-tap/identifier_case.test.lua | 12 +++--
9 files changed, 197 insertions(+), 74 deletions(-)
diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index f5a7b7819..c27ca818e 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -176,13 +176,16 @@ lbox_sql_create_function(struct lua_State *L)
}
size_t name_len;
const char *name = lua_tolstring(L, 1, &name_len);
+ int normalized_name_len = sql_normalize_name(NULL, 0, name, name_len);
+ if (normalized_name_len < 0)
+ return luaT_error(L);
char *normalized_name = (char *) region_alloc(&fiber()->gc,
- name_len + 1);
+ normalized_name_len + 1);
if (normalized_name == NULL)
return luaL_error(L, "out of memory");
- memcpy(normalized_name, name, name_len);
- normalized_name[name_len] = '\0';
- sqlNormalizeName(normalized_name);
+ if (sql_normalize_name(normalized_name, normalized_name_len + 1, name,
+ name_len) < 0)
+ return luaT_error(L);
struct lua_sql_func_info *func_info =
(struct lua_sql_func_info *) malloc(sizeof(*func_info));
if (func_info == NULL)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a08148a97..ac4dbcc51 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -299,10 +299,21 @@ sql_name_from_token(struct Parse *parser, struct Token *name_token)
{
if (name_token == NULL || name_token->z == NULL)
return NULL;
- char *name = sqlDbStrNDup(parser->db, (char *)name_token->z,
- name_token->n);
- sqlNormalizeName(name);
+ char *name = NULL;
+ int name_len =
+ sql_normalize_name(NULL, 0, name_token->z, name_token->n);
+ if (name_len < 0)
+ goto tarantool_error;
+ name = sqlDbMallocRawNN(parser->db, name_len + 1);
+ if (sql_normalize_name(name, name_len + 1, name_token->z,
+ name_token->n) < 0)
+ goto tarantool_error;
return name;
+tarantool_error:
+ sqlDbFree(parser->db, name);
+ parser->rc = SQL_TARANTOOL_ERROR;
+ parser->nErr++;
+ return NULL;
}
/*
@@ -505,17 +516,16 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
(uint32_t) p->def->field_count) == NULL)
return;
struct region *region = &pParse->region;
- z = region_alloc(region, pName->n + 1);
+ int name_len = sql_normalize_name(NULL, 0, pName->z, pName->n);
+ if (name_len < 0)
+ goto tarantool_error;
+ z = region_alloc(region, name_len + 1);
if (z == NULL) {
- diag_set(OutOfMemory, pName->n + 1,
- "region_alloc", "z");
- pParse->rc = SQL_TARANTOOL_ERROR;
- pParse->nErr++;
- return;
+ diag_set(OutOfMemory, name_len + 1, "region_alloc", "z");
+ goto tarantool_error;
}
- memcpy(z, pName->z, pName->n);
- z[pName->n] = 0;
- sqlNormalizeName(z);
+ if (sql_normalize_name(z, name_len + 1, pName->z, pName->n) < 0)
+ goto tarantool_error;
for (i = 0; i < (int)p->def->field_count; i++) {
if (strcmp(z, p->def->fields[i].name) == 0) {
sqlErrorMsg(pParse, "duplicate column name: %s", z);
@@ -535,6 +545,10 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
column_def->type = type_def->type;
p->def->field_count++;
pParse->constraintName.n = 0;
+ return;
+tarantool_error:
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
}
void
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 42531c107..c02dc682c 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -799,15 +799,24 @@ sql_expr_create(struct Parse *parser, int op, const Token *token, bool dequote)
{
int extra_sz = 0;
int val = 0;
+ struct Expr *expr = NULL;
if (token != NULL) {
if (op != TK_INTEGER || token->z == NULL ||
sqlGetInt32(token->z, &val) == 0) {
- extra_sz = token->n + 1;
+ if (op == TK_ID || op == TK_COLLATE ||
+ op == TK_FUNCTION) {
+ extra_sz = sql_normalize_name(NULL, 0, token->z,
+ token->n);
+ if (extra_sz < 0)
+ goto tarantool_error;
+ } else {
+ extra_sz = token->n;
+ }
+ extra_sz += 1;
assert(val >= 0);
}
}
- struct Expr *expr =
- sqlDbMallocRawNN(parser->db, sizeof(*expr) + extra_sz);
+ expr = sqlDbMallocRawNN(parser->db, sizeof(*expr) + extra_sz);
if (expr == NULL)
return NULL;
@@ -826,19 +835,27 @@ sql_expr_create(struct Parse *parser, int op, const Token *token, bool dequote)
} else {
expr->u.zToken = (char *)&expr[1];
assert(token->z != NULL || token->n == 0);
- memcpy(expr->u.zToken, token->z, token->n);
- expr->u.zToken[token->n] = '\0';
- if (dequote) {
- if (expr->u.zToken[0] == '"')
- expr->flags |= EP_DblQuoted;
- if (expr->op == TK_ID || expr->op == TK_COLLATE ||
- expr->op == TK_FUNCTION)
- sqlNormalizeName(expr->u.zToken);
- else
+ if (dequote && expr->u.zToken[0] == '"')
+ expr->flags |= EP_DblQuoted;
+ if (dequote &&
+ (expr->op == TK_ID || expr->op == TK_COLLATE ||
+ expr->op == TK_FUNCTION)) {
+ if (sql_normalize_name(expr->u.zToken, extra_sz,
+ token->z, token->n) < 0)
+ goto tarantool_error;
+ } else {
+ memcpy(expr->u.zToken, token->z, token->n);
+ expr->u.zToken[token->n] = '\0';
+ if (dequote)
sqlDequote(expr->u.zToken);
}
}
return expr;
+tarantool_error:
+ sqlDbFree(parser->db, expr);
+ parser->rc = SQL_TARANTOOL_ERROR;
+ parser->nErr++;
+ return NULL;
}
struct Expr *
@@ -1714,18 +1731,31 @@ sqlExprListSetName(Parse * pParse, /* Parsing context */
)
{
assert(pList != 0 || pParse->db->mallocFailed != 0);
- if (pList) {
- struct ExprList_item *pItem;
- assert(pList->nExpr > 0);
- pItem = &pList->a[pList->nExpr - 1];
- assert(pItem->zName == 0);
- pItem->zName = sqlDbStrNDup(pParse->db, pName->z, pName->n);
- if (dequote)
- sqlNormalizeName(pItem->zName);
- /* n = 0 in case of select * */
- if (pName->n != 0)
- sqlCheckIdentifierName(pParse, pItem->zName);
- }
+ if (pList == NULL || pName->n == 0)
+ return;
+ assert(pList->nExpr > 0);
+ struct ExprList_item *item = &pList->a[pList->nExpr - 1];
+ assert(item->zName == NULL);
+ if (dequote) {
+ int name_len = sql_normalize_name(NULL, 0, pName->z, pName->n);
+ if (name_len < 0)
+ goto tarantool_error;
+ item->zName = sqlDbMallocRawNN(pParse->db, name_len + 1);
+ if (item->zName == NULL)
+ return;
+ if (sql_normalize_name(item->zName, name_len + 1, pName->z,
+ pName->n) < 0)
+ goto tarantool_error;
+ } else {
+ item->zName = sqlDbStrNDup(pParse->db, pName->z, pName->n);
+ if (item->zName == NULL)
+ return;
+ }
+ sqlCheckIdentifierName(pParse, item->zName);
+ return;
+tarantool_error:
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
}
/*
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index d7b721695..631bcb8f7 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -813,7 +813,16 @@ idlist(A) ::= nm(Y).
** that created the expression.
*/
static void spanExpr(ExprSpan *pOut, Parse *pParse, int op, Token t){
- Expr *p = sqlDbMallocRawNN(pParse->db, sizeof(Expr)+t.n+1);
+ int name_len = 0;
+ struct Expr *p = NULL;
+ if (op != TK_VARIABLE) {
+ name_len = sql_normalize_name(NULL, 0, t.z, t.n);
+ if (name_len < 0)
+ goto tarantool_error;
+ } else {
+ name_len = t.n;
+ }
+ p = sqlDbMallocRawNN(pParse->db, sizeof(Expr) + name_len + 1);
if( p ){
memset(p, 0, sizeof(Expr));
switch (op) {
@@ -846,10 +855,12 @@ idlist(A) ::= nm(Y).
p->flags = EP_Leaf;
p->iAgg = -1;
p->u.zToken = (char*)&p[1];
- memcpy(p->u.zToken, t.z, t.n);
- p->u.zToken[t.n] = 0;
- if (op != TK_VARIABLE){
- sqlNormalizeName(p->u.zToken);
+ if (op != TK_VARIABLE) {
+ if (sql_normalize_name(p->u.zToken, name_len + 1, t.z, t.n) < 0)
+ goto tarantool_error;
+ } else {
+ memcpy(p->u.zToken, t.z, t.n);
+ p->u.zToken[t.n] = 0;
}
#if SQL_MAX_EXPR_DEPTH>0
p->nHeight = 1;
@@ -858,6 +869,11 @@ idlist(A) ::= nm(Y).
pOut->pExpr = p;
pOut->zStart = t.z;
pOut->zEnd = &t.z[t.n];
+ return;
+tarantool_error:
+ sqlDbFree(pParse->db, p);
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
}
}
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 5a1bc0c81..e05d082a7 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4172,10 +4172,18 @@ flattenSubquery(Parse * pParse, /* Parsing context */
pList = pParent->pEList;
for (i = 0; i < pList->nExpr; i++) {
if (pList->a[i].zName == 0) {
- char *zName =
- sqlDbStrDup(db, pList->a[i].zSpan);
- sqlNormalizeName(zName);
- pList->a[i].zName = zName;
+ char *str = pList->a[i].zSpan;
+ int len = strlen(str);
+ int name_len =
+ sql_normalize_name(NULL, 0, str, len);
+ if (name_len < 0)
+ goto tarantool_error;
+ char *name = sqlDbMallocRawNN(db, name_len + 1);
+ if (name != NULL &&
+ sql_normalize_name(name, name_len + 1, str,
+ len) < 0)
+ goto tarantool_error;
+ pList->a[i].zName = name;
}
}
if (pSub->pOrderBy) {
@@ -4248,6 +4256,10 @@ flattenSubquery(Parse * pParse, /* Parsing context */
}
#endif
+ return 1;
+tarantool_error:
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
return 1;
}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 048c814e5..380b42d90 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3264,7 +3264,31 @@ void sqlTreeViewWith(TreeView *, const With *);
void sqlSetString(char **, sql *, const char *);
void sqlErrorMsg(Parse *, const char *, ...);
void sqlDequote(char *);
-void sqlNormalizeName(char *z);
+
+/**
+ * Perform SQL name normalization: cast name to the upper-case
+ * (via Unicode Character Folding). Casing is locale-dependent
+ * and context-sensitive. The result may be longer or shorter
+ * than the original. The source string and the destination buffer
+ * must not overlap.
+ * For example, ß is converted to SS.
+ * The result is similar to SQL UPPER function.
+ * @param dst A buffer for the result string. The result will be
+ * NUL-terminated if the buffer is large enough. The
+ * contents is undefined in case of failure.
+ * @param dst_size The size of the buffer (number of bytes).
+ * If it is 0, then dest may be NULL and the
+ * function will only return the length of the
+ * result without writing any of the result
+ * string.
+ * @param src The original string.
+ * @param src_len The length of the original string.
+ * @retval The length of the result string, on success.
+ * @retval < 0 otherwise.
+ */
+int
+sql_normalize_name(char *dst, int dst_size, const char *src, int src_len);
+
void sqlTokenInit(Token *, char *);
int sqlKeywordCode(const unsigned char *, int);
int sqlRunParser(Parse *, const char *, char **);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index a5b6cb4fd..767c2e09f 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -279,16 +279,26 @@ sql_trigger_step_allocate(struct Parse *parser, u8 op,
struct Token *target_name)
{
struct sql *db = parser->db;
- struct TriggerStep *trigger_step =
- sqlDbMallocZero(db, sizeof(TriggerStep) + target_name->n + 1);
+ struct TriggerStep *trigger_step = NULL;
+ int name_len =
+ sql_normalize_name(NULL, 0, target_name->z, target_name->n);
+ if (name_len < 0)
+ goto tarantool_error;
+ trigger_step = sqlDbMallocZero(db, sizeof(TriggerStep) + name_len + 1);
if (trigger_step != NULL) {
char *z = (char *)&trigger_step[1];
- memcpy(z, target_name->z, target_name->n);
- sqlNormalizeName(z);
+ if (sql_normalize_name(z, name_len + 1, target_name->z,
+ target_name->n) < 0)
+ goto tarantool_error;
trigger_step->zTarget = z;
trigger_step->op = op;
}
return trigger_step;
+tarantool_error:
+ sqlDbFree(db, trigger_step);
+ parser->rc = SQL_TARANTOOL_ERROR;
+ parser->nErr++;
+ return NULL;
}
struct TriggerStep *
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index dadae1839..d0a352965 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -41,6 +41,7 @@
#if HAVE_ISNAN || SQL_HAVE_ISNAN
#include <math.h>
#endif
+#include <unicode/ucasemap.h>
/*
* Routine needed to support the testcase() macro.
@@ -289,23 +290,34 @@ sqlDequote(char *z)
z[j] = 0;
}
-
-void
-sqlNormalizeName(char *z)
+int
+sql_normalize_name(char *dst, int dst_size, const char *src, int src_len)
{
- char quote;
- int i=0;
- if (z == 0)
- return;
- quote = z[0];
- if (sqlIsquote(quote)){
- sqlDequote(z);
- return;
- }
- while(z[i]!=0){
- z[i] = (char)sqlToupper(z[i]);
- i++;
+ assert(src != NULL);
+ if (sqlIsquote(src[0])){
+ if (dst_size == 0)
+ return src_len;
+ memcpy(dst, src, src_len);
+ dst[src_len] = '\0';
+ sqlDequote(dst);
+ return src_len;
}
+ UErrorCode status = U_ZERO_ERROR;
+ UCaseMap *case_map = ucasemap_open(NULL, 0, &status);
+ if (case_map == NULL)
+ goto error;
+ int len = ucasemap_utf8ToUpper(case_map, dst, dst_size, src, src_len,
+ &status);
+ ucasemap_close(case_map);
+ if (!U_SUCCESS(status) &&
+ !(dst_size == 0 && status == U_BUFFER_OVERFLOW_ERROR))
+ goto error;
+ return len;
+error:
+ diag_set(CollationError,
+ "string conversion to the uppercase failed: %s",
+ u_errorName(status));
+ return -1;
}
/*
diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua
index f26399eb6..bc4daf0ff 100755
--- a/test/sql-tap/identifier_case.test.lua
+++ b/test/sql-tap/identifier_case.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(71)
+test:plan(73)
local test_prefix = "identifier_case-"
@@ -13,8 +13,10 @@ local data = {
{ 6, [[ "Table1" ]], {0} },
-- non ASCII characters case is not supported
{ 7, [[ русский ]], {0} },
- { 8, [[ Русский ]], {0} },
- { 9, [[ "русский" ]], {"/already exists/"} },
+ { 8, [[ "русский" ]], {0} },
+ { 9, [[ Großschreibweise ]], {0} },
+ { 10, [[ Русский ]], {"/already exists/"} },
+ { 11, [[ Grossschreibweise ]], {"/already exists/"} },
}
for _, row in ipairs(data) do
@@ -35,7 +37,7 @@ data = {
{ 5, [[ "table1" ]], {5}},
{ 6, [[ "Table1" ]], {6}},
{ 7, [[ русский ]], {7}},
- { 8, [[ Русский ]], {8}},
+ { 8, [[ "русский" ]], {8}},
}
for _, row in ipairs(data) do
@@ -66,7 +68,7 @@ test:do_test(
function ()
return test:drop_all_tables()
end,
- 3)
+ 4)
data = {
{ 1, [[ columnn ]], {0} },
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH v1 0/4] sql: store regular identifiers in case-normal form
2019-02-15 13:30 [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
` (3 preceding siblings ...)
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 4/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
@ 2019-02-22 12:20 ` Vladislav Shpilevoy
2019-02-22 12:38 ` Kirill Shcherbatov
4 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy @ 2019-02-22 12:20 UTC (permalink / raw)
To: tarantool-patches, Kirill Shcherbatov
Hi! Thanks for the patch!
On 15/02/2019 16:30, Kirill Shcherbatov wrote:
> Perform SQL name normalization: cast name to the upper-case
> (via Unicode Character Folding). Casing is locale-dependent
> and context-sensitive. The result may be longer or shorter
> than the original. For example, ß is converted to SS.
> The result is similar to SQL UPPER function.
>
> Performed extensive code refactoring to pass parser instance in
> routines that use sql_normalize_name function. This makes
> possible to raise an error in case of normalizing failure.
>
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3991-fix-names-normalization
> Issue: https://github.com/tarantool/tarantool/issues/3991
Issue 3991 is about vshard integration. Please, check your
links next time. The correct issue is 3931, as I understand.
Talking about the patchset - I do not understand a point of
the first 3 patches. I see, that the only reason why you
pass the parser is a wish to set nErr. Why instead not to just
do diag_set, return -1, and check for that in all upper functions,
until parser is already in one of them? This how all box functions
work with SQL right now - they do not take Vdbe, nor Parse. They
just set diag and return -1.
Our global goal is to get rid of parser nErr, not spread it more.
Please, do it.
>
> Kirill Shcherbatov (4):
> sql: patch sql_name_from_token to use Parser
> sql: patch sql_trigger_step_allocate to use Parser
> sql: patch sql_expr_create routine to use Parser
> sql: store regular identifiers in case-normal form
>
> src/box/lua/lua_sql.c | 11 +-
> src/box/sql/alter.c | 2 +-
> src/box/sql/analyze.c | 2 +-
> src/box/sql/build.c | 212 +++++++++------------
> src/box/sql/delete.c | 2 +-
> src/box/sql/expr.c | 256 ++++++++++++--------------
> src/box/sql/fkey.c | 128 +++++++------
> src/box/sql/parse.y | 60 +++---
> src/box/sql/pragma.c | 11 +-
> src/box/sql/resolve.c | 41 ++---
> src/box/sql/select.c | 70 ++++---
> src/box/sql/sqlInt.h | 255 +++++++++++++++++++++++--
> src/box/sql/trigger.c | 201 +++++++++-----------
> src/box/sql/util.c | 42 +++--
> src/box/sql/wherecode.c | 3 +-
> src/box/sql/whereexpr.c | 10 +-
> test/sql-tap/identifier_case.test.lua | 12 +-
> 17 files changed, 753 insertions(+), 565 deletions(-)
>
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH v1 0/4] sql: store regular identifiers in case-normal form
2019-02-22 12:20 ` [tarantool-patches] Re: [PATCH v1 0/4] " Vladislav Shpilevoy
@ 2019-02-22 12:38 ` Kirill Shcherbatov
2019-02-22 12:43 ` Vladislav Shpilevoy
0 siblings, 1 reply; 8+ messages in thread
From: Kirill Shcherbatov @ 2019-02-22 12:38 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
> Talking about the patchset - I do not understand a point of
> the first 3 patches. I see, that the only reason why you
> pass the parser is a wish to set nErr. Why instead not to just
> do diag_set, return -1, and check for that in all upper functions,
> until parser is already in one of them? This how all box functions
> work with SQL right now - they do not take Vdbe, nor Parse. They
> just set diag and return -1.
The problem is that when the family of functions calling sql_normalize_name return NULL,
it is assumed that a memory allocation error has occurred, although this is not the case.
The API for working with expressions is heterogeneous, and in some cases already accepts Parser -
for example sqlPExpr, sqlPExprAddSelect.
Moreover, in the same way, to set a third-party error it is used in sql Expr: see sqlExprCheckHeight
that set
sqlErrorMsg(pParse, "Expression tree is too large (maximum depth %d)", mxHeight);
Thus, I do not introduce new practices to the API, but on the contrary, bring it to a more consistent state.
The alternative that you propose to use the forced setting SQL_TARANTOOL_ERROR (in some cases) three functions.
the stack if the function returned NULL and !db.mallocFailed. This will work, however, for the same sqlPExpr analogy
will be incorrect: it is not true that if the expression construction function returned NULL, a Tarantool error occurred.
This will confuse the already conflicting SQL code.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH v1 0/4] sql: store regular identifiers in case-normal form
2019-02-22 12:38 ` Kirill Shcherbatov
@ 2019-02-22 12:43 ` Vladislav Shpilevoy
0 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy @ 2019-02-22 12:43 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
On 22/02/2019 15:38, Kirill Shcherbatov wrote:
>> Talking about the patchset - I do not understand a point of
>> the first 3 patches. I see, that the only reason why you
>> pass the parser is a wish to set nErr. Why instead not to just
>> do diag_set, return -1, and check for that in all upper functions,
>> until parser is already in one of them? This how all box functions
>> work with SQL right now - they do not take Vdbe, nor Parse. They
>> just set diag and return -1.
>
> The problem is that when the family of functions calling sql_normalize_name return NULL,
> it is assumed that a memory allocation error has occurred, although this is not the case.
Then stop this assumption. Set diag_set(OutOfMemory) inside this function.
> The API for working with expressions is heterogeneous, and in some cases already accepts Parser -
> for example sqlPExpr, sqlPExprAddSelect.> Moreover, in the same way, to set a third-party error it is used in sql Expr: see sqlExprCheckHeight
> that set> sqlErrorMsg(pParse, "Expression tree is too large (maximum depth %d)", mxHeight);
> Thus, I do not introduce new practices to the API, but on the contrary, bring it to a more consistent state.
Again, you rolled back our progress of getting rid of nErr. You should not
insert it deeper into the API. Please, do not.
>
> The alternative that you propose to use the forced setting SQL_TARANTOOL_ERROR (in some cases) three functions.
sql_normalize_name always should mean tarantool error. Even on failed sqlite3StrDup
you can set diag.
> the stack if the function returned NULL and !db.mallocFailed. This will work, however, for the same sqlPExpr analogy
> will be incorrect: it is not true that if the expression construction function returned NULL, a Tarantool error occurred.
>
> This will confuse the already conflicting SQL code.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-22 12:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-15 13:30 [tarantool-patches] [PATCH v1 0/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 1/4] sql: patch sql_name_from_token to use Parser Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 2/4] sql: patch sql_trigger_step_allocate " Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 3/4] sql: patch sql_expr_create routine " Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 4/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
2019-02-22 12:20 ` [tarantool-patches] Re: [PATCH v1 0/4] " Vladislav Shpilevoy
2019-02-22 12:38 ` Kirill Shcherbatov
2019-02-22 12:43 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox