List:Commits« Previous MessageNext Message »
From:Sergei Golubchik Date:March 7 2008 7:12pm
Subject:Re: bk commit into 5.1 tree (mattiasj:1.2518) BUG#33479
View as plain text  
Hi!

On Mar 03, Mattias Jonsson wrote:

> >>diff -Nrup a/sql/ha_partition.cc b/sql/ha_partition.cc
> >>--- a/sql/ha_partition.cc	2007-12-20 19:16:51 +01:00
> >>+++ b/sql/ha_partition.cc	2008-02-21 10:09:42 +01:00
> >>@@ -5579,19 +5636,29 @@ int ha_partition::cmp_ref(const uchar *r
> >>                 MODULE auto increment
> >>
> ****************************************************************************/
> >> 
> >>-void ha_partition::restore_auto_increment(ulonglong)
> >>-{
> >> 
> >>+int ha_partition::ha_reset_auto_increment(ulonglong value)
> >
> >Never. You can only override reset_auto_increment(), and never ha_xxx
> >methods. By the way, ha_reset_auto_increment() (and all other ha_xxx
> >methods) are not even virtual.
> >
> >Something must be wrong with your test, if you didn't notice that you
> >"override" a non-virtual method, didn't notice that your
> >ha_reset_auto_increment() is never called.
> 
> Thank you very much for pointing this out to me. I will change it and 
> test again. (I must have done some kind of typo, and you are correct 
> that I have not done any test for this function, I will add a test with 
> truncate table)
> 
> When testing, I found that I had to use reset_auto_increment in 
> delete_all_rows if SQLCOM_TRUNCATE. Is it OK to create a bug that MyISAM 
> does not support this function, and is not working properly with 
> TRUNCATE TABLE if it is partitioned? (Or could you guide me how to 
> implement reset_auto_increment properly in ha_myisam.cc?)

feel free to submit a bugreport.
If you'd like you can assign it to yourself and fix right away (but in a
separate changeset) - the fix should probably be in mi_delete_all_rows()
function, you'll see it resets various members in state (MI_STATE_INFO),
but apparently state->auto_increment is forgotten. Should be a
one-liner.
 
> Could I also report a bug about 'ALTER TABLE t1 AUTO_INCREMENT = n' is 
> actually rewriting the whole table, instead of only set its storage 
> engines internal AUTO_INCREMENT value? (and the lack of a API to set 
> this without inserting a row). Or is this intentional?

I don't think it's intentional. Please report it, so that it won't be
forgottent, but it'll be a feature request, not a bug.
 
> Are there any documentation or notes about the naming convention of 
> functions, define, variables and such?

Not that I know of. That is, there is documentation -
http://forge.mysql.com/wiki/MySQL_Internals_Coding_Guidelines
but the ha_ prefix of handler methods isn't covered there.  It would be
more appropriate to explain the logic (ha_ for handler non-virtual
public functions, which are basically wrappers for the corresponding
"real" handler methods, the latter being virtual and private, so that SQL
code can only access them via ha_ wrappers) here:
http://forge.mysql.com/wiki/MySQL_Internals_Custom_Engine
but it's not explained there either :(

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer/Server Architect
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140
Thread
bk commit into 5.1 tree (mattiasj:1.2518) BUG#33479mattiasj21 Feb
  • Re: bk commit into 5.1 tree (mattiasj:1.2518) BUG#33479Sergei Golubchik29 Feb
    • Re: bk commit into 5.1 tree (mattiasj:1.2518) BUG#33479Mattias Jonsson3 Mar
      • Re: bk commit into 5.1 tree (mattiasj:1.2518) BUG#33479Sergei Golubchik7 Mar