On Fri, 23 Apr 2010 11:09:11 -0400, Jay Pipes <[email protected]> wrote:
> 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.
This means that for properly executing a REPLACE statement the engine
would have to peek at current_session->lex->sql_command to work out if
it should delete+insert on duplicate key.
(currently you can do it by flipping some flag in your engine on
HA_EXTRA_WRITE_CAN_REPLACE - as doing a switch on sql_command means you
have to take care of REPLACE and LOAD DATA (i think))
> 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.
We could just have counters in cursor that were updated for each row
inserted/updated/replaced/deleted.
> 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:
We would need doReplaceRecord as well to make sure the engine knows how
to behave though.
or we could pass in a boolean flag (bool can_replace)?
> Thoughts?
I think there's other bits of the API I'd like to improve first.
Short list:
- DATE gets accessors for dealing with 4 byte ints. 3 byte is annoying
(especially when it's part of a key)
- accessor class for key buffer (index_read et al) instead of each
engine having their own parser (that in turn needs intimate knowledge of
storage of fields... such as DATE)
- fixing so that all places correctly call startTransaction instead of
some implicit txn begins in ::store_lock or worse, rnd_init
- fix of the write set usage in update so that default values are in
it. currently you must:
if (! (**field).isWriteSet() && (**field).is_null())
continue;
in write_row/update_row to get correct (see
write_row_to_innodb_tuple)
- Movement from record buffers to a Tuple object. Everything can (and
possibly should be) still stored in a single buffer, but accessed
through an OO interface would be a big win.
i.e. get rid of Field::move_field_offset which is kind of required
right now (or more of your own specialised row format parsing code)
- Correct auto_increment behaviour not require
innobase_get_int_col_max_value (i.e. the engine to track the max
valid value for each type, as 0 is kinda special in auto_inc)
- The complete removal of ::info() - it smells of ioctl and makes little
to no sense. Individual calls for individual things.
- ERRKEY and the like go from this.
- Something clever to get rid of ::extra
- ON UPDATE NOW() for TIMESTAMP to be done above the engine layer
engines should not have to #include field/timestamp.h and have this is
doUpdateRecord:
if (table->timestamp_field_type & TIMESTAMP_AUTO_SET_ON_UPDATE)
table->timestamp_field->set_time();
- make it possible for an engine to implement DDL transactions (and DDL
only transactions). removing the implicit txn commit.
--
Stewart Smith
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
