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),
®_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
