Merge branch 'wl10787-drop-cleanup'

Author: rsomla1

cdk/parser/expr_parser.cc

@@ -468,13 +468,18 @@ bool Expr_parser_base::parse_schema_ident(Token::Type (*types)[2])
 
 void Expr_parser_base::parse_column_ident(Path_prc *prc)
 {
-  parse_schema_ident();
+  if (!parse_schema_ident())
+    parse_error(L"Expected a column identifier");
   parse_column_ident1(prc);
 }
 
 
 void Expr_parser_base::parse_column_ident1(Path_prc *prc)
 {
+  /*
+    Note: at this point we assume that an (possibly schema qualified) identifier
+    has been already seen and is stored in m_col_ref.table()
+  */
   if (consume_token(Token::DOT))
   {
     string name;
@@ -488,11 +493,13 @@ void Expr_parser_base::parse_column_ident1(Path_prc *prc)
   {
     // Re-interpret table name parsed by parse_schema_ident() as a
     // column name of the form [<table>.]<column>
+    auto table = m_col_ref.table();
+    assert(table);
 
-    if (m_col_ref.table()->schema())
-      m_col_ref.set(m_col_ref.table()->name(), m_col_ref.table()->schema()->name());
+    if (table->schema())
+      m_col_ref.set(table->name(), table->schema()->name());
     else
-      m_col_ref.set(m_col_ref.table()->name());
+      m_col_ref.set(table->name());
   }
 
   if (consume_token(Token::ARROW) || consume_token(Token::ARROW2))

devapi/session.cc

@@ -1056,44 +1056,63 @@ void check_reply_skip_error_throw(cdk::Reply&& r, int skip_server_error)
 void Session::dropSchema(const string &name)
 {
   try{
+    prepare_for_cmd();
     std::stringstream qry;
     qry << "Drop Schema `" << name << "`";
     //skip server error 1008 = schema doesn't exist
-    check_reply_skip_error_throw(get_cdk_session().sql(qry.str()),
-                                 1008);
+    check_reply_skip_error_throw(get_cdk_session().sql(qry.str()), 1008);
   }
   CATCH_AND_WRAP
 }
 
 
-/*
-  TODO: better implementation: check if drop_collection can drop views also
-*/
+void Schema::dropTable(const string& table)
+{
+  /*
+    Note: We simply forward to dropCollection() because xplugin does not
+    have a dedicated command for dropping tables and it does not distinguish
+    tables from collections.
+  */
+  dropCollection(table);
+}
 
-void Session::dropTable(const mysqlx::string& schema, const string& table)
+
+void Schema::dropCollection(const mysqlx::string& collection)
 {
   try{
-    Args args(schema, table);
-    // Doesn't throw if table doesn't exit (server error 1051)
-    check_reply_skip_error_throw(get_cdk_session().admin("drop_collection", args),
-                                 1051);
+    m_sess->prepare_for_cmd();
+    Args args(m_name, collection);
+    // Doesn't throw if collection doesn't exit (server error 1051)
+    check_reply_skip_error_throw(
+          m_sess->get_cdk_session().admin("drop_collection", args),
+          1051
+          );
   }
   CATCH_AND_WRAP
 }
 
 
-void Session::dropCollection(const mysqlx::string& schema,
-                              const mysqlx::string& collection)
+void Schema::dropView(const mysqlx::string& name)
 {
-  try{
-    Args args(schema, collection);
-    // Doesn't throw if collection doesn't exit (server error 1051)
-    check_reply_skip_error_throw(get_cdk_session().admin("drop_collection", args),
-                                 1051);
+  Table_ref view(getName(),name);
+
+  try {
+    m_sess->prepare_for_cmd();
+    /*
+      Note: false argument to view_drop() means that we do not check for
+      the existence of the view being dropped.
+      We do not throw error 1347 (object is not a view) treating it the same
+      as the case when the view does not exist.
+    */
+    check_reply_skip_error_throw(
+          m_sess->get_cdk_session().view_drop(view, false),
+          1347
+    );
   }
   CATCH_AND_WRAP
 }
 
+
 Schema Session::getDefaultSchema()
 {
   if (m_impl->m_default_db.empty())

devapi/table_crud.cc

@@ -676,53 +676,4 @@ ViewAlter::ViewAlter(Schema &sch, const string &name)
 } // namespace mysqlx
 
 
-/*
-   ViewDrop
-   ========
-*/
-
-namespace mysqlx {
-namespace internal{
-
-class Op_ViewDrop
-    : public Op_base<View_drop_impl>
-    ,  public Table_ref
-{
-  bool m_checkExistence = true;
-
-public:
-
-  Op_ViewDrop(Schema &sch, const string &name)
-    : Op_base<View_drop_impl>(sch.getSession())
-    , Table_ref(sch.getName(), name)
-  {}
-
-  void if_exists() override
-  {
-    m_checkExistence = false;
-  }
-
-  Executable_impl* clone() const override
-  {
-    return new Op_ViewDrop(*this);
-  }
-
-  cdk::Reply* send_command() override
-  {
-    return new cdk::Reply(get_cdk_session().view_drop(*this,m_checkExistence));
-  }
-};
-
-} // namespace internal
-
-
-ViewDrop::ViewDrop(Schema &sch, const string &name)
-{
-  m_impl.reset(new internal::Op_ViewDrop(sch, name));
-}
-
-} // namespace mysqlx
-
-
-
 // ---------------------------------------------------------------------

devapi/tests/bugs-t.cc

@@ -92,3 +92,20 @@ TEST_F(Bugs, bug25505482)
 
   cout << "Done!" << endl;
 }
+
+TEST_F(Bugs, bug26130226_crash_update)
+{
+  SKIP_IF_NO_XPLUGIN;
+
+  get_sess().dropSchema("crash_update");
+  get_sess().createSchema("crash_update");
+  Schema sch = get_sess().getSchema("crash_update");
+  Collection coll = sch.createCollection("c1", true);
+
+  coll.add("{ \"name\": \"abc\", \"age\": 1 , \"misc\": 1.2}").execute();
+  Table tabNew = sch.getCollectionAsTable("c1");
+
+  EXPECT_THROW(
+    tabNew.update().set((char *)0, expr("")).execute(), // SegFault
+    Error);
+}

devapi/tests/crud-t.cc

@@ -1330,9 +1330,9 @@ TEST_F(Crud, doc_path)
   cout << "Creating session..." << endl;
 
   Session sess(this);
-  sess.dropCollection("test", "coll");
 
   Schema sch = sess.getSchema("test");
+  sch.dropCollection("coll");
   Collection coll = sch.createCollection("coll",false);
 
   coll.add( "{\"date\": {\"monthName\":\"December\", \"days\":[1,2,3]}}").execute();
@@ -1915,8 +1915,6 @@ TEST_F(Crud, group_by_having)
   Session sess(this);
 
   Schema test = sess.createSchema("test", true);
-  sess.dropCollection("test", "coll");
-
   Collection coll = test.createCollection("coll", true);
   Table tbl = test.getCollectionAsTable("coll", true);
 
@@ -2301,7 +2299,6 @@ TEST_F(Crud, single_document)
   cout << "Session accepted, creating collection..." << endl;
 
   Schema sch = sess.getSchema("test");
-  sess.dropCollection("test", "c1");
   Collection coll = sch.createCollection("c1", true);
 
   coll.add("{\"_id\":\"id1\", \"name\":\"foo\" }" )
@@ -2350,7 +2347,6 @@ TEST_F(Crud, add_or_replace)
   cout << "Session accepted, creating collection..." << endl;
 
   Schema sch = sess.getSchema("test");
-  sess.dropCollection("test", "c1");
   Collection coll = sch.createCollection("c1", true);
 
   coll.add("{\"_id\":\"id1\", \"name\":\"foo\" }")

devapi/tests/ddl-t.cc

@@ -101,16 +101,31 @@ TEST_F(Ddl, create_drop)
 
     //Drop Tables/Views
 
-    std::list<string> names_list = schema.getTableNames();
+    /*
+      Note: currently passing view to dropTable() does nothing. Changing this
+      requires changes in xplugin. Similar with passing table to dropView().
+    */
+    //EXPECT_THROW(schema.dropTable("view1"), mysqlx::Error);
+    EXPECT_NO_THROW(schema.dropTable("view1"));
+    EXPECT_NO_THROW(schema.getTable("view1", true));
 
-    for (auto name : names_list)
+    EXPECT_NO_THROW(schema.dropView("tb1"));
+    EXPECT_NO_THROW(schema.getTable("tb1", true));
+
+    std::list<Table> table_list = schema.getTables();
+
+    for (auto t : table_list)
     {
       //View is not dropped, but still no error thrown
-      get_sess().dropTable(schema.getName(), name);
+      if (t.isView())
+        schema.dropView(t.getName());
+      else
+        schema.dropTable(t.getName());
     }
 
     EXPECT_THROW(schema.getTable("tb1", true), mysqlx::Error);
     EXPECT_THROW(schema.getTable("tb2", true), mysqlx::Error);
+    EXPECT_THROW(schema.getTable("view1", true), mysqlx::Error);
   }
 
   //Collection tests
@@ -137,16 +152,24 @@ TEST_F(Ddl, create_drop)
 
     // Drop Collections
 
+    /*
+      Note: currently passing collection to dropView() does nothing. Changing this
+      requires changes in xplugin.
+    */
+
+    EXPECT_NO_THROW(schema.dropView(collection_name_1));
+    EXPECT_NO_THROW(schema.getCollection(collection_name_1, true));
+
     std::list<string> list_coll_name = schema.getCollectionNames();
     for (auto name : list_coll_name)
     {
-      get_sess().dropCollection(schema.getName(), name);
+      schema.dropCollection(name);
     }
 
     //Doesn't throw even if don't exist
     for (auto name : list_coll_name)
     {
-      get_sess().dropCollection(schema.getName(), name);
+      schema.dropCollection(name);
     }
 
     //Test Drop Collection
@@ -183,3 +206,42 @@ TEST_F(Ddl, create_drop)
 
   cout << "Done!" << endl;
 }
+
+TEST_F(Ddl, bugs)
+{
+
+  SKIP_IF_NO_XPLUGIN;
+
+  {
+    /*
+      Having Result before dropView() triggered error, because cursor was closed
+      without informing Result, so that Result coud cache and then close Cursor
+      and Reply
+    */
+
+    get_sess().dropSchema("bugs");
+    get_sess().createSchema("bugs", false);
+
+    Schema schema = get_sess().getSchema("bugs");
+    sql(
+          "CREATE TABLE bugs.bugs_table("
+          "c0 JSON,"
+          "c1 INT"
+          ")");
+
+    Table tbl = schema.getTable("bugs_table");
+
+    schema.createView("newView").algorithm(Algorithm::TEMPTABLE)
+        .security(SQLSecurity ::INVOKER).definedAs(tbl.select()).execute();
+
+    RowResult result = get_sess().sql("show create view bugs.newView").execute();
+    Row r = result.fetchOne();
+
+    schema.dropView("newView");
+
+    schema.createView("newView").algorithm(Algorithm::TEMPTABLE)
+        .security(SQLSecurity ::INVOKER).definedAs(tbl.select()).execute();
+
+    r = result.fetchOne();
+  }
+}