How would i find out exactly from which functions i have to remove
unused params.And moreover i could i say which function is completely
unused.
And also after removing the unused params i ran the "python
tests/dbqp.py" and it didnt return anything wrong.So in case of
virtual functions should i have to go through the entire list of class
functions to check whether that param is being used or not.
Please suggest some better method of doing this.
Akash Sinha
On Fri, Mar 11, 2011 at 10:12 AM, Stewart Smith
<email address hidden> wrote:
> On Thu, 10 Mar 2011 21:57:07 -0000, Akash Sinha <email address hidden> wrote:
>> --- drizzled/cursor.h 2011-03-07 12:31:41 +0000
>> +++ drizzled/cursor.h 2011-03-10 21:44:21 +0000
>> @@ -319,12 +319,12 @@
>> virtual ha_rows estimate_rows_upper_bound()
>> { return stats.records+EXTRA_RECORDS; }
>>
>> - virtual const char *index_type(uint32_t)
>> + virtual const char *index_type()
>> { assert(0); return "";}
>
> It looks like index_type() is completely unused (apart from being implemented
> in every storage engine) - so instead of just removing the parameter
> (which shouldn't always be unused anyway) we should remove all the
> implementations of it too.
>
>> @@ -349,15 +349,15 @@
>> const unsigned char * key,
>> key_part_map keypart_map,
>> enum ha_rkey_function find_flag);
>> - virtual int index_next(unsigned char *) __attribute__ ((warn_unused_result))
>> - { return HA_ERR_WRONG_COMMAND; }
>> - virtual int index_prev(unsigned char *)
>> - { return HA_ERR_WRONG_COMMAND; }
>> - virtual int index_first(unsigned char *)
>> - { return HA_ERR_WRONG_COMMAND; }
>> - virtual int index_last(unsigned char *)
>> - { return HA_ERR_WRONG_COMMAND; }
>> - virtual int index_next_same(unsigned char *, const unsigned char *, uint32_t);
>> + virtual int index_next() __attribute__ ((warn_unused_result))
>> + { return HA_ERR_WRONG_COMMAND; }
>> + virtual int index_prev()
>> + { return HA_ERR_WRONG_COMMAND; }
>> + virtual int index_first()
>> + { return HA_ERR_WRONG_COMMAND; }
>> + virtual int index_last()
>> + { return HA_ERR_WRONG_COMMAND; }
>> + virtual int index_next_same();
>
> This is incorrect, the buffer being passed in is important and at least
> *should* be used by classes implementing them.
>
>> private:
>> uint32_t calculate_key_len(uint32_t key_position, key_part_map keypart_map_arg);
>> @@ -379,20 +379,20 @@
>> bool eq_range, bool sorted);
>> virtual int read_range_next();
>> int compare_key(key_range *range);
>> - virtual int rnd_next(unsigned char *)=0;
>> - virtual int rnd_pos(unsigned char *, unsigned char *)=0;
>> + virtual int rnd_next()=0;
>> + virtual int rnd_pos()=0;
>
> the rnd_pos() method is used and does use the parameters (just not in
> the default implementation that does nothing), same with rnd_next() -
> that is used.
>
>> virtual int read_first_row(unsigned char *buf, uint32_t primary_key);
>> - virtual int rnd_same(unsigned char *, uint32_t)
>> + virtual int rnd_same()
>> { return HA_ERR_WRONG_COMMAND; }
>> - virtual ha_rows records_in_range(uint32_t, key_range *, key_range *)
>> + virtual ha_rows records_in_range()
>> { return (ha_rows) 10; }
>
> also wrong, these parameters are used by all classes that implement it.
>
>
>
> So most of this patch isn't quite right (especially around
> Cursor). However, there are some unused bit sthat can work. I'd
> recommend starting from scratch on a tree and doing these bits
>
> I'd also be sure to when checking out any virtual function to check
> everywhere that calls it as well as everywhere that implements it.
>
> --
> Stewart Smith
>
How would i find out exactly from which functions i have to remove
unused params.And moreover i could i say which function is completely
unused.
And also after removing the unused params i ran the "python
tests/dbqp.py" and it didnt return anything wrong.So in case of
virtual functions should i have to go through the entire list of class
functions to check whether that param is being used or not.
Please suggest some better method of doing this.
Akash Sinha
On Fri, Mar 11, 2011 at 10:12 AM, Stewart Smith rows_upper_ bound() EXTRA_RECORDS; } type(uint32_ t) unused_ result) ) WRONG_COMMAND; } WRONG_COMMAND; } unsigned char *) WRONG_COMMAND; } WRONG_COMMAND; } same(unsigned char *, const unsigned char *, uint32_t); unused_ result) ) WRONG_COMMAND; } WRONG_COMMAND; } WRONG_COMMAND; } WRONG_COMMAND; } key_len( uint32_ t key_position, key_part_map keypart_map_arg); key(key_ range *range); row(unsigned char *buf, uint32_t primary_key); WRONG_COMMAND; } in_range( uint32_ t, key_range *, key_range *)
<email address hidden> wrote:
> On Thu, 10 Mar 2011 21:57:07 -0000, Akash Sinha <email address hidden> wrote:
>> --- drizzled/cursor.h 2011-03-07 12:31:41 +0000
>> +++ drizzled/cursor.h 2011-03-10 21:44:21 +0000
>> @@ -319,12 +319,12 @@
>> virtual ha_rows estimate_
>> { return stats.records+
>>
>> - virtual const char *index_
>> + virtual const char *index_type()
>> { assert(0); return "";}
>
> It looks like index_type() is completely unused (apart from being implemented
> in every storage engine) - so instead of just removing the parameter
> (which shouldn't always be unused anyway) we should remove all the
> implementations of it too.
>
>> @@ -349,15 +349,15 @@
>> const unsigned char * key,
>> key_part_map keypart_map,
>> enum ha_rkey_function find_flag);
>> - virtual int index_next(unsigned char *) __attribute__ ((warn_
>> - { return HA_ERR_
>> - virtual int index_prev(unsigned char *)
>> - { return HA_ERR_
>> - virtual int index_first(
>> - { return HA_ERR_
>> - virtual int index_last(unsigned char *)
>> - { return HA_ERR_
>> - virtual int index_next_
>> + virtual int index_next() __attribute__ ((warn_
>> + { return HA_ERR_
>> + virtual int index_prev()
>> + { return HA_ERR_
>> + virtual int index_first()
>> + { return HA_ERR_
>> + virtual int index_last()
>> + { return HA_ERR_
>> + virtual int index_next_same();
>
> This is incorrect, the buffer being passed in is important and at least
> *should* be used by classes implementing them.
>
>> private:
>> uint32_t calculate_
>> @@ -379,20 +379,20 @@
>> bool eq_range, bool sorted);
>> virtual int read_range_next();
>> int compare_
>> - virtual int rnd_next(unsigned char *)=0;
>> - virtual int rnd_pos(unsigned char *, unsigned char *)=0;
>> + virtual int rnd_next()=0;
>> + virtual int rnd_pos()=0;
>
> the rnd_pos() method is used and does use the parameters (just not in
> the default implementation that does nothing), same with rnd_next() -
> that is used.
>
>> virtual int read_first_
>> - virtual int rnd_same(unsigned char *, uint32_t)
>> + virtual int rnd_same()
>> { return HA_ERR_
>> - virtual ha_rows records_
>> + virtual ha_rows records_in_range()
>> { return (ha_rows) 10; }
>
> also wrong, these parameters are used by all classes that implement it.
>
>
>
> So most of this patch isn't quite right (especially around
> Cursor). However, there are some unused bit sthat can work. I'd
> recommend starting from scratch on a tree and doing these bits
>
> I'd also be sure to when checking out any virtual function to check
> everywhere that calls it as well as everywhere that implements it.
>
> --
> Stewart Smith
>