Hi Jay and fellow Drizzlers! First thing that pondered in my mind was the overhead in dealing with the return status object but you've made it clear that we don't have to worry about this. So this looks pretty cool.
One thing that I'd like to expand on this topic is the future interface for write_row()/insertRecord(). Currently we are given a pointer to a byte array that contains the raw MySQL formatted row. As I mentioned in our tutorial at the conference, this information alone is not developer friendly (e.g. most folks would likely first stumble on finding the length of the byte array). So given that claim, Here's one idea that's in my mind atm. We could design a class that contains minimal amount of information on a particular column (like the current Field class) and give an array of that as an argument to insertRecord(). So, something like this: int Cursor::insertRecord(ColumnInfo **columns, const uint32_t num_of_columns); There is obviously more to a row than this (like the first n-bytes in a row that represents which column is NULL). But I think if we design this right, we can absorb all that in a minimal fashion. Not easy but doable :) Cheers, - Toru On Sat, Apr 24, 2010 at 12:09 AM, Jay Pipes <[email protected]> wrote: > Hi all! > > So, I've been doing some cleanups around the Cursor API, as has Brian. > > I've modified the old Cursor::ha_write_row() and Cursor::write_row() > calls to follow Drizzle coding standards: > > Cursor::ha_write_row() is now Cursor::insertRecord() > Cursor::write_row() is now Cursor::doInsertRecord() > > Anyway, I'm proposing to change the API for the above calls, and > remove the HA_EXTRA_WRITE_CAN_REPLACE flag. > > The existing API call is like so: > > virtual int Cursor::doInsertRecord(unsigned char *new_row); > > The call returns an integer that corresponds to an engine error code > (with 0 being "success"). The problem with this API, as briefly > outlined by Kostja in this MySQL internals mailing list post: > > > > is that the return code does not indicate whether the call to > write_row() (doInsertRecord() in Drizzle) inserted a new row or > whether it replaced an existing row. > > I propose changing the above Cursor::doInsertRecord() in the following way: > > class Cursor > { > public: > ... > typedef std::pair<uint64_t, uint64_t> RecordChanges; > typedef std::pair<int, RecordChanges> InsertRecordResult; > /** public wrapper */ > InsertRecordResult > private: > /** virtual private method */ > virtual InsertRecordResult doInsertRecord(uint8_t *new_record); > ... > }; > > The HA_EXTRA_WRITE_CAN_REPLACE flag can then be removed. The engine > will construct a return tuple (an InsertRecordResult) containing the > error code, if any, the number of records inserted, and the number of > records replaced. Usage in the kernel would be like so: > > uint64_t inserted_rows= 0; > uint64_t replaced_rows= 0; > > Cursor::InsertRecordResult result= cursor->insertRecord(table[0]); > > if (result.first == 0) /* successful result */ > { > inserted_rows= result.second.first; > replaced_rows= result.second.second; > } > > Thoughts? > > -jay > > _______________________________________________ > Mailing list: https://launchpad.net/~drizzle-discuss > Post to : [email protected] > Unsubscribe : https://launchpad.net/~drizzle-discuss > More help : https://help.launchpad.net/ListHelp > -- Toru Maesaka <[email protected]> _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
