[tarantool-patches] Re: [PATCH v2 3/4] sql: use space_by_name in SQL

  • From: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
  • To: Kirill Yukhin <kyukhin@xxxxxxxxxxxxx>
  • Date: Sat, 27 Oct 2018 00:00:59 +0300

On Thu, Oct 25, 2018 at 11:17:11AM +0300, Kirill Yukhin wrote:

diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 3d72e31..7c28437 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -47,18 +47,17 @@ sql_alter_table_rename(struct Parse *parse, struct 
SrcList *src_tab,
      if (new_name == NULL)
              goto exit_rename_table;
      /* Check that new name isn't occupied by another table. */
-     uint32_t space_id = box_space_id_by_name(new_name, strlen(new_name));
-     if (space_id != BOX_ID_NIL) {
+     struct space *space = space_by_name(new_name);
+     if (space != NULL) {
              diag_set(ClientError, ER_SPACE_EXISTS, new_name);
              goto tnt_error;
      }
      const char *tbl_name = src_tab->a[0].zName;
-     space_id = box_space_id_by_name(tbl_name, strlen(tbl_name));
-     if (space_id == BOX_ID_NIL) {
+     space = space_by_name(tbl_name);
+     if (space == NULL) {
              diag_set(ClientError, ER_NO_SUCH_SPACE, tbl_name);
              goto tnt_error;
      }
-     struct space *space = space_by_id(space_id);
      assert(space != NULL);

Pointless assertion.

      if (space->def->opts.is_view) {
              diag_set(ClientError, ER_ALTER_SPACE, tbl_name,
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 674d53d..ad068a54 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1114,16 +1114,15 @@ sqlite3Analyze(Parse * pParse, Token * pName)
              /* Form 2:  Analyze table named */
              char *z = sqlite3NameFromToken(db, pName);
              if (z != NULL) {
-                     uint32_t space_id = box_space_id_by_name(z, strlen(z));
-                     if (space_id != BOX_ID_NIL) {
-                             struct space *sp = space_by_id(space_id);
-                             assert(sp != NULL);
-                             if (sp->def->opts.is_view) {
+                     struct space *space = space_by_name(z);
+                     if (space != NULL) {
+                             assert(space != NULL);

Another one.

+                             if (space->def->opts.is_view) {
                                      sqlite3ErrorMsg(pParse, "VIEW isn't "\
                                                      "allowed to be "\
                                                      "analyzed");
                              } else {
-                                     vdbe_emit_analyze_table(pParse, sp);
+                                     vdbe_emit_analyze_table(pParse, space);
                              }
                      } else {
                              diag_set(ClientError, ER_NO_SUCH_SPACE, z);
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 8f6c76f..72e0575 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -36,15 +36,14 @@
 #include "tarantoolInt.h"
 
 struct Table *
-sql_lookup_table(struct Parse *parse, struct SrcList_item *tbl_name)
+sql_lookup_table(struct Parse *parse, struct SrcList_item *src_name)
 {
-     assert(tbl_name != NULL);
-     assert(tbl_name->pTab == NULL);
-     uint32_t space_id = box_space_id_by_name(tbl_name->zName,
-                                              strlen(tbl_name->zName));
-     struct space *space = space_by_id(space_id);
-     if (space_id == BOX_ID_NIL || space == NULL) {
-             sqlite3ErrorMsg(parse, "no such table: %s", tbl_name->zName);
+     assert(src_name != NULL);
+     assert(src_name->pTab == NULL);
+     const char *name = src_name->zName;
+     struct space *space = space_by_name(name);
+     if (space == NULL) {
+             sqlite3ErrorMsg(parse, "no such table: %s", name);

I hate it when one tries to squeeze some unrelated "cleanup", like
this variable renaming, in a patch that doesn't really need it.
This complicates review and clutters the history. I reverted this
part, removed useless assertions, and pushed the patch to 2.1.

              return NULL;
      }
      assert(space != NULL);

Other related posts:

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