Comments inline. Looks ok, but as always, more explanation will be
useful in future.
Jonas Oreland wrote:
> #At file:///home/jonas/src/telco-7.0/ based on
> revid:jonas@stripped
>
> 4144 Jonas Oreland 2011-01-27
> ndb - bug#59756 - Don't put table into global cache until schema trans has been
> successfully committed. Add interface to get ObjectId back from createTable()
>
> modified:
> mysql-test/suite/ndb/r/ndb_basic.result
> mysql-test/suite/ndb/t/ndb_basic.test
> sql/ha_ndbcluster.cc
> storage/ndb/include/ndbapi/NdbDictionary.hpp
> storage/ndb/src/ndbapi/NdbDictionary.cpp
> storage/ndb/src/ndbapi/NdbDictionaryImpl.cpp
> storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp
> === modified file 'mysql-test/suite/ndb/r/ndb_basic.result'
> --- a/mysql-test/suite/ndb/r/ndb_basic.result 2011-01-27 10:36:47 +0000
> +++ b/mysql-test/suite/ndb/r/ndb_basic.result 2011-01-27 13:23:07 +0000
> @@ -873,4 +873,16 @@ id parent_id
> 2 2
> 3 3
> DROP TABLE child, parent;
> +CREATE TABLE t1 (a INT PRIMARY KEY, b TEXT)
> +ENGINE=ndb PARTITION BY KEY(a) PARTITIONS 24;
> +ERROR HY000: Can't create table 'test.t1' (errno: 140)
> +show warnings;
> +Level Code Message
> +Error 1296 Got error 1224 'Too many fragments' from NDB
> +Error 1005 Can't create table 'test.t1' (errno: 140)
> +CREATE TABLE t1 (a INT PRIMARY KEY, b TEXT)
> +ENGINE=ndb;
> +show warnings;
> +Level Code Message
> +drop table t1;
> End of 5.1 tests
>
> === modified file 'mysql-test/suite/ndb/t/ndb_basic.test'
> --- a/mysql-test/suite/ndb/t/ndb_basic.test 2011-01-27 10:36:47 +0000
> +++ b/mysql-test/suite/ndb/t/ndb_basic.test 2011-01-27 13:23:07 +0000
> @@ -785,4 +785,16 @@ SELECT * FROM child ORDER BY id;
>
> DROP TABLE child, parent;
>
> +#
> +# bug#59756
> +#
> +--error 1005
> +CREATE TABLE t1 (a INT PRIMARY KEY, b TEXT)
> +ENGINE=ndb PARTITION BY KEY(a) PARTITIONS 24;
> +show warnings;
> +CREATE TABLE t1 (a INT PRIMARY KEY, b TEXT)
> +ENGINE=ndb;
> +show warnings;
> +drop table t1;
> +
> --echo End of 5.1 tests
>
> === modified file 'sql/ha_ndbcluster.cc'
> --- a/sql/ha_ndbcluster.cc 2011-01-27 10:36:47 +0000
> +++ b/sql/ha_ndbcluster.cc 2011-01-27 13:23:07 +0000
> @@ -7674,6 +7674,7 @@ int ha_ndbcluster::create(const char *na
> bool ndb_sys_table= FALSE;
> partition_info *part_info;
> int result= 0;
> + NdbDictionary::ObjectId objId;
>
> DBUG_ENTER("ha_ndbcluster::create");
> DBUG_PRINT("enter", ("name: %s", name));
> @@ -7700,7 +7701,6 @@ int ha_ndbcluster::create(const char *na
>
> Ndb *ndb= get_ndb(thd);
> NDBDICT *dict= ndb->getDictionary();
> - Ndb_table_guard ndbtab_g(dict);
>
> #ifndef NDB_WITHOUT_TABLESPACE_IN_FRM
> DBUG_PRINT("info", ("Tablespace %s,%s", form->s->tablespace,
> create_info->tablespace));
> @@ -7754,6 +7754,7 @@ int ha_ndbcluster::create(const char *na
>
> if (is_truncate)
> {
> + Ndb_table_guard ndbtab_g(dict);
> ndbtab_g.init(m_tabname);
> if (!(m_table= ndbtab_g.get_table()))
> ERR_RETURN(dict->getNdbError());
> @@ -8055,7 +8056,7 @@ int ha_ndbcluster::create(const char *na
> }
>
> // Create the table in NDB
> - if (dict->createTable(tab) != 0)
> + if (dict->createTable(tab, &objId) != 0)
> {
> const NdbError err= dict->getNdbError();
> set_ndb_err(thd, err);
> @@ -8063,26 +8064,14 @@ int ha_ndbcluster::create(const char *na
> goto abort;
> }
>
> - ndbtab_g.init(m_tabname);
> - // temporary set m_table during create
> - // reset at return
> - m_table= ndbtab_g.get_table();
> - // TODO check also that we have the same frm...
> - if (!m_table)
> - {
> - /* purecov: begin deadcode */
> - const NdbError err= dict->getNdbError();
> - set_ndb_err(thd, err);
> - my_errno= ndb_to_mysql_error(&err);
> - goto abort;
> - /* purecov: end */
> - }
> -
> DBUG_PRINT("info", ("Table %s/%s created successfully",
> m_dbname, m_tabname));
>
> // Create secondary indexes
Add comment :
// Assigning table's created objectid for index creation to reference
> + tab.assignObjId(objId);
> + m_table= &tab;
> my_errno= create_indexes(thd, ndb, form);
> + m_table= 0;
>
> if (!my_errno)
> {
> @@ -8118,6 +8107,13 @@ err_return:
> m_table= 0;
> ERR_RETURN(dict->getNdbError());
> }
> +
> + /**
> + * createTable/index schema transaction OK
> + */
Add comment :
// Load table definition back from kernel
> + Ndb_table_guard ndbtab_g(dict, m_tabname);
> + m_table= ndbtab_g.get_table();
> +
> if (my_errno)
> {
> /*
>
> === modified file 'storage/ndb/include/ndbapi/NdbDictionary.hpp'
> --- a/storage/ndb/include/ndbapi/NdbDictionary.hpp 2010-10-13 09:33:02 +0000
> +++ b/storage/ndb/include/ndbapi/NdbDictionary.hpp 2011-01-27 13:23:07 +0000
> @@ -1041,6 +1041,14 @@ public:
> * passing NULL pointer will equal to bitmap with all columns set
> */
> int checkColumns(const Uint32* bitmap, unsigned len_in_bytes) const;
> +
> + /**
> + * Set tableId,tableVersion on a table...
> + * this is a "work-around" since createIndex can't (currently)
> + * accept an ObjectId instead of table-object in createIndex
> + * this as way way too much stuff is pushed into NdbDictInterface
> + */
Remove comment about 'way way too much stuff is pushed...'
Mention instead that tableid, tableversion are required for createIndex,
so this simplifies the createTable(), createIndex() call sequence.
> + void assignObjId(const ObjectId &);
> #endif
>
> // these 2 are not de-doxygenated
> @@ -2211,6 +2219,14 @@ public:
> int createTable(const Table &table);
>
> /**
> + * Create defined table given defined Table instance
> + * return ObjectId
> + * @param table Table to create
> + * @return 0 if successful otherwise -1.
> + */
> + int createTable(const Table &table, ObjectId * objid);
> +
> + /**
> * Start table optimization given defined table object
> * @param t Object of table to optimize
> * @param Pre-allocated OptimizeTableHandle
>
> === modified file 'storage/ndb/src/ndbapi/NdbDictionary.cpp'
> --- a/storage/ndb/src/ndbapi/NdbDictionary.cpp 2010-09-30 14:27:18 +0000
> +++ b/storage/ndb/src/ndbapi/NdbDictionary.cpp 2011-01-27 13:23:07 +0000
> @@ -532,6 +532,7 @@ NdbDictionary::Table::addColumn(const Co
> {
> return -1;
> }
> + col->m_column_no = m_impl.m_columns.size() - 1;
Is this a separate bug?
> return 0;
> }
>
> @@ -976,6 +977,14 @@ NdbDictionary::Table::getPartitionId(Uin
> }
> }
>
> +void
> +NdbDictionary::Table::assignObjId(const NdbDictionary::ObjectId& _objId)
> +{
> + const NdbDictObjectImpl& objId = NdbDictObjectImpl::getImpl(_objId);
> + m_impl.m_id = objId.m_id;
> + m_impl.m_version = objId.m_version;
> +}
> +
Quite dangerous if it's misused!
> /*****************************************************************
> * Index facade
> */
> @@ -2064,12 +2073,23 @@ NdbDictionary::Dictionary::~Dictionary()
> int
> NdbDictionary::Dictionary::createTable(const Table & t)
> {
> + return createTable(t, 0);
> +}
> +
> +int
> +NdbDictionary::Dictionary::createTable(const Table & t, ObjectId * objId)
> +{
> int ret;
> + ObjectId tmp;
> + if (objId == 0)
> + objId = &tmp;
> +
> if (likely(! is_ndb_blob_table(t.getName())))
> {
> DO_TRANS(
> ret,
> - m_impl.createTable(NdbTableImpl::getImpl(t))
> + m_impl.createTable(NdbTableImpl::getImpl(t),
> + NdbDictObjectImpl::getImpl( *objId))
> );
> }
> else
>
> === modified file 'storage/ndb/src/ndbapi/NdbDictionaryImpl.cpp'
> --- a/storage/ndb/src/ndbapi/NdbDictionaryImpl.cpp 2011-01-12 08:04:39 +0000
> +++ b/storage/ndb/src/ndbapi/NdbDictionaryImpl.cpp 2011-01-27 13:23:07 +0000
> @@ -2879,7 +2879,7 @@ NdbDictInterface::parseTableInfo(NdbTabl
> * Create table and alter table
> */
> int
> -NdbDictionaryImpl::createTable(NdbTableImpl &t)
> +NdbDictionaryImpl::createTable(NdbTableImpl &t, NdbDictObjectImpl & objid)
> {
> DBUG_ENTER("NdbDictionaryImpl::createTable");
>
> @@ -2910,6 +2910,8 @@ NdbDictionaryImpl::createTable(NdbTableI
> Uint32* data = (Uint32*)m_receiver.m_buffer.get_data();
> t.m_id = data[0];
> t.m_version = data[1];
> + objid.m_id = data[0];
> + objid.m_version = data[1];
>
> // update table def from DICT - by-pass cache
> NdbTableImpl* t2 =
> @@ -3014,7 +3016,8 @@ NdbDictionaryImpl::createBlobTables(cons
> assert(bc != NULL);
> bc->setStorageType(d);
> }
> - if (createTable(bt) != 0) {
> + NdbDictionary::ObjectId objId; // ignore objid
> + if (createTable(bt, NdbDictObjectImpl::getImpl(objId)) != 0) {
> DBUG_RETURN(-1);
> }
> }
>
> === modified file 'storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp'
> --- a/storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp 2010-11-09 20:40:03 +0000
> +++ b/storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp 2011-01-27 13:23:07 +0000
> @@ -817,7 +817,7 @@ public:
> bool setTransporter(class Ndb * ndb, class TransporterFacade * tf);
> bool setTransporter(class TransporterFacade * tf);
>
> - int createTable(NdbTableImpl &t);
> + int createTable(NdbTableImpl &t, NdbDictObjectImpl &);
> int optimizeTable(const NdbTableImpl &t,
> NdbOptimizeTableHandleImpl &h);
> int optimizeIndex(const NdbIndexImpl &index,
>
>
>
> ------------------------------------------------------------------------
>
>
--
Frazer Clement, Senior Software Engineer,
MySQL Cluster / Oracle - www.mysql.com
Office: Edinburgh, UK
Are you MySQL certified? www.mysql.com/certification
