List:Internals« Previous MessageNext Message »
From:Michael Widenius Date:January 27 2001 10:07pm
Subject:SEC_TO_TIME bug?
View as plain text  
Hi!

>>>>> "Patrik" == Patrik Wallstrom <pawal@stripped> writes:

Patrik> I am puzzled by a certain SQL statement I make on a data set I have.
Patrik> The first SQL statement delivers the correct result, like this (with and
Patrik> without SEC_TO_TIME):

mysql> select distinct
-> xVCD.VCDID,SEC_TO_TIME(sum(TIME_TO_SEC(sonoxexternal.Song.Length))) as Length
-> from xVCD,sonoxexternal.Song where xVCD.SongID=sonoxexternal.Song.SongID AND
-> xVCD.VCDID="VC39fef2bb7aa90"
-> group by xVCD.VCDID,xVCD.OrderID;
Patrik> +-----------------+----------+
Patrik> | VCDID           | Length   |
Patrik> +-----------------+----------+
Patrik> | VC39fef2bb7aa90 | 01:06:39 |
Patrik> +-----------------+----------+
Patrik> 1 row in set (0.00 sec)

Thanks for uploading the data sets. This helped me find the reason for
this problem.

The problem was caused by the combination of having a group function
inside another function combined with a GROUP BY, where you don't use
all the GROUP BY columns in the result.

This wasn't that easy to fix, as MySQL uses a lot of different methods
to resolve DISTINCT as early as possible.  Here is however a patch
for 3.23.32 that seems to work:


*** sql_select.cc-1.77	Wed Jan 17 03:15:19 2001
--- edited/sql_select.cc	Sat Jan 27 17:05:23 2001
***************
*** 268,274 ****
    join.first_record=join.sort_and_group=0;
    join.select_options=select_options;
    join.result=result;
!   count_field_types(&join.tmp_table_param,all_fields);
    join.const_tables=0;
    join.having=0;
    join.group= group != 0;
--- 268,274 ----
    join.first_record=join.sort_and_group=0;
    join.select_options=select_options;
    join.result=result;
!   count_field_types(&join.tmp_table_param,all_fields,0);
    join.const_tables=0;
    join.having=0;
    join.group= group != 0;
***************
*** 632,640 ****
      /*
      ** If we have different sort & group then we must sort the data by group
      ** and copy it to another tmp table
      */
  
!     if (group && (!test_if_subpart(group,order) || select_distinct))
      {					/* Must copy to another table */
        TABLE *tmp_table2;
        DBUG_PRINT("info",("Creating group table"));
--- 632,645 ----
      /*
      ** If we have different sort & group then we must sort the data by group
      ** and copy it to another tmp table
+     ** This code is also used if we are using distinct something
+     ** we haven't been able to store in the temporary table yet
+     ** like SEC_TO_TIME(SUM(...)).
      */
  
!     if (group && (!test_if_subpart(group,order) || select_distinct) ||
! 	(select_distinct &&
! 	 join.tmp_table_param.using_indirect_summary_function))
      {					/* Must copy to another table */
        TABLE *tmp_table2;
        DBUG_PRINT("info",("Creating group table"));
***************
*** 644,654 ****
        if (make_simple_join(&join,tmp_table))
  	goto err;
        calc_group_buffer(&join,group);
!       count_field_types(&join.tmp_table_param,all_fields);
  
        /* group data to new table */
        if (!(tmp_table2 = create_tmp_table(thd,&join.tmp_table_param,all_fields,
! 					  (ORDER*) 0, 0 , 1, 0,
  					  join.select_options)))
  	goto err;				/* purecov: inspected */
        if (group)
--- 649,664 ----
        if (make_simple_join(&join,tmp_table))
  	goto err;
        calc_group_buffer(&join,group);
!       count_field_types(&join.tmp_table_param,all_fields,
! 			select_distinct && !group);
!       join.tmp_table_param.hidden_field_count=(all_fields.elements-
! 					       fields.elements);
  
        /* group data to new table */
        if (!(tmp_table2 = create_tmp_table(thd,&join.tmp_table_param,all_fields,
! 					  (ORDER*) 0,
! 					  select_distinct && !group,
! 					  1, 0,
  					  join.select_options)))
  	goto err;				/* purecov: inspected */
        if (group)
***************
*** 657,663 ****
  	if (create_sort_index(join.join_tab,group,HA_POS_ERROR) ||
  	    alloc_group_fields(&join,group))
  	{
! 	  free_tmp_table(thd,tmp_table2); /* purecov: inspected */
  	  goto err;				/* purecov: inspected */
  	}
  	group=0;
--- 667,673 ----
  	if (create_sort_index(join.join_tab,group,HA_POS_ERROR) ||
  	    alloc_group_fields(&join,group))
  	{
! 	  free_tmp_table(thd,tmp_table2);	/* purecov: inspected */
  	  goto err;				/* purecov: inspected */
  	}
  	group=0;
***************
*** 696,709 ****
      if (make_simple_join(&join,tmp_table))
        goto err;
      calc_group_buffer(&join,group);
!     count_field_types(&join.tmp_table_param,all_fields);
    }
    if (procedure)
    {
      if (procedure->change_columns(fields) ||
  	result->prepare(fields))
        goto err;
!     count_field_types(&join.tmp_table_param,all_fields);
    }
    if (join.group || join.tmp_table_param.sum_func_count ||
        (procedure && (procedure->flags & PROC_GROUP)))
--- 706,719 ----
      if (make_simple_join(&join,tmp_table))
        goto err;
      calc_group_buffer(&join,group);
!     count_field_types(&join.tmp_table_param,all_fields,0);
    }
    if (procedure)
    {
      if (procedure->change_columns(fields) ||
  	result->prepare(fields))
        goto err;
!     count_field_types(&join.tmp_table_param,all_fields,0);
    }
    if (join.group || join.tmp_table_param.sum_func_count ||
        (procedure && (procedure->flags & PROC_GROUP)))
***************
*** 3265,3270 ****
--- 3275,3281 ----
  {
    TABLE *table;
    uint	i,field_count,reclength,null_count,null_pack_length,
+         hidden_null_count, hidden_null_pack_length, hidden_field_count,
  	blob_count,group_null_items;
    bool	using_unique_constraint=0;
    char	*tmpname,path[FN_REFLEN];
***************
*** 3292,3300 ****
--- 3303,3314 ----
        (*tmp->item)->marker=4;			// Store null in key
      if (param->group_length >= MAX_BLOB_WIDTH)
        using_unique_constraint=1;
+     if (group)
+       distinct=0;				// Can't use distinct
    }
  
    field_count=param->field_count+param->func_count+param->sum_func_count;
+   hidden_field_count=param->hidden_field_count;
    if (!my_multi_malloc(MYF(MY_WME),
  		       &table,sizeof(*table),
  		       &reg_field,sizeof(Field*)*(field_count+1),
***************
*** 3334,3342 ****
    table->tmp_table=1;
    table->db_low_byte_first=1;			// True for HEAP and MyISAM
  
!   /* Calculate with type of fields we will need in heap table */
  
!   reclength=blob_count=null_count=group_null_items=0;
  
    List_iterator<Item> li(fields);
    Item *item;
--- 3348,3357 ----
    table->tmp_table=1;
    table->db_low_byte_first=1;			// True for HEAP and MyISAM
  
!   /* Calculate which type of fields we will store in the temporary table */
  
!   reclength=blob_count=null_count=hidden_null_count=group_null_items=0;
!   param->using_indirect_summary_function=0;
  
    List_iterator<Item> li(fields);
    Item *item;
***************
*** 3344,3351 ****
    while ((item=li++))
    {
      Item::Type type=item->type();
!     if (item->with_sum_func && type != Item::SUM_FUNC_ITEM ||
! 	item->const_item())
        continue;
      if (type == Item::SUM_FUNC_ITEM && !group && !save_sum_fields)
      {						/* Can't calc group yet */
--- 3359,3375 ----
    while ((item=li++))
    {
      Item::Type type=item->type();
!     if (item->with_sum_func && type != Item::SUM_FUNC_ITEM)
!     {
!       /*
! 	Mark that the we have ignored an item that refers to a summary
! 	function. We need to know this if someone is going to use
! 	DISTINCT on the result.
!       */
!       param->using_indirect_summary_function=1;
!       continue;
!     }
!     if (item->const_item())			// We don't have to store this
        continue;
      if (type == Item::SUM_FUNC_ITEM && !group && !save_sum_fields)
      {						/* Can't calc group yet */
***************
*** 3396,3401 ****
--- 3420,3427 ----
        }
        *(reg_field++) =new_field;
      }
+     if (!--hidden_field_count)
+       hidden_null_count=null_count;
    }
    field_count= (uint) (reg_field - table->field);
  
***************
*** 3420,3427 ****
  
    table->blob_fields=blob_count;
    if (blob_count == 0)
!     null_count++;				// For delete link
!   reclength+=(null_pack_length=(null_count+7)/8);
    if (!reclength)
      reclength=1;				// Dummy select
  
--- 3446,3461 ----
  
    table->blob_fields=blob_count;
    if (blob_count == 0)
!   {
!     /* We need to ensure that first byte is not 0 for the delete link */
!     if (hidden_null_count)
!       hidden_null_count++;
!     else
!       null_count++;
!   }
!   hidden_null_pack_length=(hidden_null_count+7)/8;
!   null_pack_length=hidden_null_count+(null_count+7)/8;
!   reclength+=null_pack_length;
    if (!reclength)
      reclength=1;				// Dummy select
  
***************
*** 3449,3454 ****
--- 3483,3489 ----
      bfill(null_flags,null_pack_length,255);	// Set null fields
    }
    null_count= (blob_count == 0) ? 1 : 0;
+   hidden_field_count=param->hidden_field_count;
    for (i=0,reg_field=table->field; i < field_count; i++,reg_field++,recinfo++)
    {
      Field *field= *reg_field;
***************
*** 3496,3501 ****
--- 3531,3538 ----
        recinfo->type=FIELD_SKIPP_ENDSPACE;
      else
        recinfo->type=FIELD_NORMAL;
+     if (!--hidden_field_count)
+       null_count=(null_count+7) & ~7;		// move to next byte
    }
  
    param->copy_field_count=(uint) (copy - param->copy_field);
***************
*** 3559,3569 ****
      }
    }
  
!   if (distinct && !group)
    {
!     /* Create an unique key or an unique constraint over all columns */
!     keyinfo->key_parts=field_count+ test(null_count);
!     if (distinct && allow_distinct_limit)
      {
        set_if_smaller(table->max_rows,thd->select_limit);
        param->end_write_records=thd->select_limit;
--- 3596,3612 ----
      }
    }
  
!   if (distinct)
    {
!     /*
!       Create an unique key or an unique constraint over all columns
!       that should be in the result.  In the temporary table, there are
!       'param->hidden_field_count' extra columns, whose null bits are stored
!       in the first 'hidden_null_pack_length' bytes of the row.
!     */
!     null_pack_length-=hidden_null_pack_length;
!     keyinfo->key_parts=field_count+ test(null_pack_length);
!     if (allow_distinct_limit)
      {
        set_if_smaller(table->max_rows,thd->select_limit);
        param->end_write_records=thd->select_limit;
***************
*** 3585,3595 ****
      keyinfo->flags=HA_NOSAME;
      keyinfo->key_length=(uint16) reclength;
      keyinfo->name=(char*) "tmp";
!     if (null_count)
      {
        key_part_info->null_bit=0;
!       key_part_info->offset=0;
!       key_part_info->length=(null_count+7)/8;
        key_part_info->field=new Field_string((char*) table->record[0],
  					    (uint32) key_part_info->length,
  					    (uchar*) 0,
--- 3628,3638 ----
      keyinfo->flags=HA_NOSAME;
      keyinfo->key_length=(uint16) reclength;
      keyinfo->name=(char*) "tmp";
!     if (null_pack_length)
      {
        key_part_info->null_bit=0;
!       key_part_info->offset=hidden_null_pack_length;
!       key_part_info->length=null_pack_length;
        key_part_info->field=new Field_string((char*) table->record[0],
  					    (uint32) key_part_info->length,
  					    (uchar*) 0,
***************
*** 3600,3606 ****
        key_part_info->type=    HA_KEYTYPE_BINARY;
        key_part_info++;
      }
!     for (i=0,reg_field=table->field; i < field_count;
  	 i++, reg_field++, key_part_info++)
      {
        key_part_info->null_bit=0;
--- 3643,3650 ----
        key_part_info->type=    HA_KEYTYPE_BINARY;
        key_part_info++;
      }
!     for (i=param->hidden_field_count, reg_field=table->field + i ;
! 	 i < field_count;
  	 i++, reg_field++, key_part_info++)
      {
        key_part_info->null_bit=0;
***************
*** 5917,5929 ****
  *****************************************************************************/
  
  void
! count_field_types(TMP_TABLE_PARAM *param, List<Item> &fields)
  {
    List_iterator<Item> li(fields);
    Item *field;
  
!   param->field_count=param->sum_func_count=
!     param->func_count=0;
    param->quick_group=1;
    while ((field=li++))
    {
--- 5961,5974 ----
  *****************************************************************************/
  
  void
! count_field_types(TMP_TABLE_PARAM *param, List<Item> &fields,
! 		  bool reset_with_sum_func)
  {
    List_iterator<Item> li(fields);
    Item *field;
  
!   param->field_count=param->sum_func_count=param->func_count= 
!     param->hidden_field_count=0;
    param->quick_group=1;
    while ((field=li++))
    {
***************
*** 5949,5955 ****
--- 5994,6004 ----
        }
      }
      else
+     {
        param->func_count++;
+       if (reset_with_sum_func)
+ 	field->with_sum_func=0;
+     }
    }
  }
  
===== item_sum.cc 1.6 vs edited =====
*** item_sum.cc-1.6	Wed Aug 30 22:42:22 2000
--- edited/item_sum.cc	Sat Jan 27 15:39:04 2001
***************
*** 811,817 ****
    for (uint i=0; i < arg_count ; i++)
      if (list.push_back(args[i]))
        return 1;
!   count_field_types(tmp_table_param,list);
    if (table)
    {
      free_tmp_table(thd, table);
--- 811,817 ----
    for (uint i=0; i < arg_count ; i++)
      if (list.push_back(args[i]))
        return 1;
!   count_field_types(tmp_table_param,list,0);
    if (table)
    {
      free_tmp_table(thd, table);
===== sql_select.h 1.11 vs edited =====
*** sql_select.h-1.11	Sat Oct 14 03:16:34 2000
--- edited/sql_select.h	Sat Jan 27 17:03:48 2001
***************
*** 124,133 ****
    KEY *keyinfo;
    ha_rows end_write_records;
    uint	copy_field_count,field_count,sum_func_count,func_count;
    uint	group_parts,group_length;
    uint	quick_group;
  
!   TMP_TABLE_PARAM() :copy_field(0), group_parts(0), group_length(0) {}
    ~TMP_TABLE_PARAM()
    {
      cleanup();
--- 124,136 ----
    KEY *keyinfo;
    ha_rows end_write_records;
    uint	copy_field_count,field_count,sum_func_count,func_count;
+   uint  hidden_field_count;
    uint	group_parts,group_length;
    uint	quick_group;
+   bool  using_indirect_summary_function;
  
!   TMP_TABLE_PARAM() :copy_field(0), group_parts(0), group_length(0)
!   {}
    ~TMP_TABLE_PARAM()
    {
      cleanup();
***************
*** 178,184 ****
  			ORDER *group, bool distinct, bool save_sum_fields,
  			bool allow_distinct_limit, uint select_options);
  void free_tmp_table(THD *thd, TABLE *entry);
! void count_field_types(TMP_TABLE_PARAM *param, List<Item> &fields);
  bool setup_copy_fields(TMP_TABLE_PARAM *param,List<Item> &fields);
  void copy_fields(TMP_TABLE_PARAM *param);
  void copy_funcs(Item_result_field **func_ptr);
--- 181,188 ----
  			ORDER *group, bool distinct, bool save_sum_fields,
  			bool allow_distinct_limit, uint select_options);
  void free_tmp_table(THD *thd, TABLE *entry);
! void count_field_types(TMP_TABLE_PARAM *param, List<Item> &fields,
! 		       bool reset_with_sum_func);
  bool setup_copy_fields(TMP_TABLE_PARAM *param,List<Item> &fields);
  void copy_fields(TMP_TABLE_PARAM *param);
  void copy_funcs(Item_result_field **func_ptr);


Regards,
Monty


Thread
SEC_TO_TIME bug?Patrik Wallstrom26 Jan
  • SEC_TO_TIME bug?Michael Widenius26 Jan
  • SEC_TO_TIME bug?Michael Widenius27 Jan