Commit 3038105e authored by bk@mysql.r18.ru's avatar bk@mysql.r18.ru

Merge rkalimullin@bk-internal.mysql.com:/home/bk/mysql-4.1

into mysql.r18.ru:/usr/home/bk/mysql-4.1
parents dad54b7d 492813d9
......@@ -154,8 +154,26 @@ Warning 1258 1 line(s) was(were) cut by group_concat()
show warnings;
Level Code Message
Warning 1258 1 line(s) was(were) cut by group_concat()
set group_concat_max_len = 1024;
drop table if exists T_URL;
Warnings:
Note 1051 Unknown table 'T_URL'
create table T_URL ( URL_ID int(11), URL varchar(80));
drop table if exists T_REQUEST;
Warnings:
Note 1051 Unknown table 'T_REQUEST'
create table T_REQUEST ( REQ_ID int(11), URL_ID int(11));
insert into T_URL values (4,'www.host.com'), (5,'www.google.com'),(5,'www.help.com');
insert into T_REQUEST values (1,4), (5,4), (5,5);
select REQ_ID, Group_Concat(URL) as URL from T_URL, T_REQUEST where
T_REQUEST.URL_ID = T_URL.URL_ID group by REQ_ID;
REQ_ID URL
1 www.host.com
5 www.host.com,www.google.com,www.help.com
drop table T_URL;
drop table T_REQUEST;
select group_concat(sum(a)) from t1 group by grp;
ERROR HY000: Invalid use of group function
select grp,group_concat(c order by 2) from t1 group by grp;
ERROR 42S22: Unknown column '2' in 'group statement'
drop table if exists t1;
drop table t1;
......@@ -249,6 +249,33 @@ INSERT INTO t1 VALUES (1, 'a545f661efdd1fb66fdee3aab79945bf');
SELECT 1 FROM t1 WHERE tmp=AES_DECRYPT(tmp,"password");
1
DROP TABLE t1;
select POSITION(_latin1'B' IN _latin1'abcd');
POSITION(_latin1'B' IN _latin1'abcd')
2
select POSITION(_latin1'B' IN _latin1'abcd' COLLATE latin1_bin);
POSITION(_latin1'B' IN _latin1'abcd' COLLATE latin1_bin)
0
select POSITION(_latin1'B' COLLATE latin1_bin IN _latin1'abcd');
POSITION(_latin1'B' COLLATE latin1_bin IN _latin1'abcd')
0
select POSITION(_latin1'B' COLLATE latin1_general_ci IN _latin1'abcd' COLLATE latin1_bin);
ERROR HY000: Illegal mix of collations (latin1_bin,EXPLICIT) and (latin1_general_ci,EXPLICIT) for operation 'locate'
select POSITION(_latin1'B' IN _latin2'abcd');
ERROR HY000: Illegal mix of collations (latin2_general_ci,COERCIBLE) and (latin1_swedish_ci,COERCIBLE) for operation 'locate'
select FIND_IN_SET(_latin1'B',_latin1'a,b,c,d');
FIND_IN_SET(_latin1'B',_latin1'a,b,c,d')
2
select FIND_IN_SET(_latin1'B' COLLATE latin1_general_ci,_latin1'a,b,c,d' COLLATE latin1_bin);
ERROR HY000: Illegal mix of collations (latin1_general_ci,EXPLICIT) and (latin1_bin,EXPLICIT) for operation 'find_in_set'
select FIND_IN_SET(_latin1'B',_latin2'a,b,c,d');
ERROR HY000: Illegal mix of collations (latin1_swedish_ci,COERCIBLE) and (latin2_general_ci,COERCIBLE) for operation 'find_in_set'
select SUBSTRING_INDEX(_latin1'abcdabcdabcd',_latin1'd',2);
SUBSTRING_INDEX(_latin1'abcdabcdabcd',_latin1'd',2)
abcdabc
select SUBSTRING_INDEX(_latin1'abcdabcdabcd',_latin2'd',2);
ERROR HY000: Illegal mix of collations (latin1_swedish_ci,COERCIBLE) and (latin2_general_ci,COERCIBLE) for operation 'substr_index'
select SUBSTRING_INDEX(_latin1'abcdabcdabcd' COLLATE latin1_general_ci,_latin1'd' COLLATE latin1_bin,2);
ERROR HY000: Illegal mix of collations (latin1_general_ci,EXPLICIT) and (latin1_bin,EXPLICIT) for operation 'substr_index'
select collation(bin(130)), coercibility(bin(130));
collation(bin(130)) coercibility(bin(130))
latin1_swedish_ci 3
......
......@@ -68,6 +68,20 @@ select grp,group_concat(c order by c) from t1 group by grp;
set group_concat_max_len = 5;
select grp,group_concat(c) from t1 group by grp;
show warnings;
set group_concat_max_len = 1024;
# Test variable length
drop table if exists T_URL;
create table T_URL ( URL_ID int(11), URL varchar(80));
drop table if exists T_REQUEST;
create table T_REQUEST ( REQ_ID int(11), URL_ID int(11));
insert into T_URL values (4,'www.host.com'), (5,'www.google.com'),(5,'www.help.com');
insert into T_REQUEST values (1,4), (5,4), (5,5);
select REQ_ID, Group_Concat(URL) as URL from T_URL, T_REQUEST where
T_REQUEST.URL_ID = T_URL.URL_ID group by REQ_ID;
drop table T_URL;
drop table T_REQUEST;
# Test errors
......@@ -76,4 +90,4 @@ select group_concat(sum(a)) from t1 group by grp;
--error 1054
select grp,group_concat(c order by 2) from t1 group by grp;
drop table if exists t1;
drop table t1;
......@@ -136,6 +136,33 @@ DROP TABLE t1;
#
# Test collation and coercibility
#
select POSITION(_latin1'B' IN _latin1'abcd');
select POSITION(_latin1'B' IN _latin1'abcd' COLLATE latin1_bin);
select POSITION(_latin1'B' COLLATE latin1_bin IN _latin1'abcd');
--error 1265
select POSITION(_latin1'B' COLLATE latin1_general_ci IN _latin1'abcd' COLLATE latin1_bin);
--error 1265
select POSITION(_latin1'B' IN _latin2'abcd');
select FIND_IN_SET(_latin1'B',_latin1'a,b,c,d');
--fix this:
--select FIND_IN_SET(_latin1'B',_latin1'a,b,c,d' COLLATE latin1_bin);
--select FIND_IN_SET(_latin1'B' COLLATE latin1_bin,_latin1'a,b,c,d');
--error 1265
select FIND_IN_SET(_latin1'B' COLLATE latin1_general_ci,_latin1'a,b,c,d' COLLATE latin1_bin);
--error 1265
select FIND_IN_SET(_latin1'B',_latin2'a,b,c,d');
select SUBSTRING_INDEX(_latin1'abcdabcdabcd',_latin1'd',2);
--fix this:
--select SUBSTRING_INDEX(_latin1'abcdabcdabcd' COLLATE latin1_bin,_latin1'd',2);
--select SUBSTRING_INDEX(_latin1'abcdabcdabcd',_latin1'd' COLLATE latin1_bin,2);
--error 1265
select SUBSTRING_INDEX(_latin1'abcdabcdabcd',_latin2'd',2);
--error 1265
select SUBSTRING_INDEX(_latin1'abcdabcdabcd' COLLATE latin1_general_ci,_latin1'd' COLLATE latin1_bin,2);
select collation(bin(130)), coercibility(bin(130));
select collation(oct(130)), coercibility(oct(130));
select collation(conv(130,16,10)), coercibility(conv(130,16,10));
......
......@@ -183,7 +183,7 @@ Field::Field(char *ptr_arg,uint32 length_arg,uchar *null_ptr_arg,
field_name(field_name_arg),
query_id(0),key_start(0),part_of_key(0),part_of_sortkey(0),
unireg_check(unireg_check_arg),
field_length(length_arg),null_bit(null_bit_arg)
field_length(length_arg),null_bit(null_bit_arg),abs_offset(0)
{
flags=null_ptr ? 0: NOT_NULL_FLAG;
comment.str= (char*) "";
......
......@@ -66,6 +66,7 @@ public:
uint32 field_length; // Length of field
uint16 flags;
uchar null_bit; // Bit used to test null bit
uint abs_offset; // use only in group_concat
Field(char *ptr_arg,uint32 length_arg,uchar *null_ptr_arg,uchar null_bit_arg,
utype unireg_check_arg, const char *field_name_arg,
......
......@@ -1045,10 +1045,18 @@ longlong Item_func_coercibility::val_int()
return (longlong) args[0]->derivation();
}
void Item_func_locate::fix_length_and_dec()
{
maybe_null=0; max_length=11;
if (cmp_collation.set(args[0]->collation, args[1]->collation))
my_coll_agg_error(args[0]->collation, args[1]->collation, func_name());
}
longlong Item_func_locate::val_int()
{
String *a=args[0]->val_str(&value1);
String *b=args[1]->val_str(&value2);
bool binary_cmp= (cmp_collation.collation->state & MY_CS_BINSORT) ? 1 : 0;
if (!a || !b)
{
null_value=1;
......@@ -1063,7 +1071,7 @@ longlong Item_func_locate::val_int()
{
start=(uint) args[2]->val_int()-1;
#ifdef USE_MB
if (use_mb(a->charset()))
if (use_mb(cmp_collation.collation))
{
start0=start;
if (!binary_cmp)
......@@ -1076,7 +1084,7 @@ longlong Item_func_locate::val_int()
if (!b->length()) // Found empty string at start
return (longlong) (start+1);
#ifdef USE_MB
if (use_mb(a->charset()) && !binary_cmp)
if (use_mb(cmp_collation.collation) && !binary_cmp)
{
const char *ptr=a->ptr()+start;
const char *search=b->ptr();
......@@ -1095,7 +1103,7 @@ longlong Item_func_locate::val_int()
return (longlong) start0+1;
}
skipp:
if ((l=my_ismbchar(a->charset(),ptr,strend)))
if ((l=my_ismbchar(cmp_collation.collation,ptr,strend)))
ptr+=l;
else ++ptr;
++start0;
......@@ -1201,6 +1209,8 @@ void Item_func_find_in_set::fix_length_and_dec()
}
}
}
if (cmp_collation.set(args[0]->collation, args[1]->collation))
my_coll_agg_error(args[0]->collation, args[1]->collation, func_name());
}
static const char separator=',';
......@@ -1228,7 +1238,6 @@ longlong Item_func_find_in_set::val_int()
null_value=0;
int diff;
CHARSET_INFO *charset= find->charset();
if ((diff=buffer->length() - find->length()) >= 0)
{
const char *f_pos=find->ptr();
......@@ -1242,7 +1251,8 @@ longlong Item_func_find_in_set::val_int()
const char *pos= f_pos;
while (pos != f_end)
{
if (my_toupper(charset,*str) != my_toupper(charset,*pos))
if (my_toupper(cmp_collation.collation,*str) !=
my_toupper(cmp_collation.collation,*pos))
goto not_found;
str++;
pos++;
......@@ -2013,7 +2023,7 @@ void Item_func_set_user_var::update_hash(void *ptr, uint length,
my_free(entry->value,MYF(0));
entry->value=0;
entry->length=0;
collation.set(cs, dv);
entry->collation.set(cs, dv);
}
else
{
......
......@@ -610,17 +610,13 @@ public:
class Item_func_locate :public Item_int_func
{
String value1,value2;
bool binary_cmp;
DTCollation cmp_collation;
public:
Item_func_locate(Item *a,Item *b) :Item_int_func(a,b) {}
Item_func_locate(Item *a,Item *b,Item *c) :Item_int_func(a,b,c) {}
const char *func_name() const { return "locate"; }
longlong val_int();
void fix_length_and_dec()
{
maybe_null=0; max_length=11;
binary_cmp = args[0]->binary() || args[1]->binary();
}
void fix_length_and_dec();
};
......@@ -684,6 +680,7 @@ class Item_func_find_in_set :public Item_int_func
String value,value2;
uint enum_value;
ulonglong enum_bit;
DTCollation cmp_collation;
public:
Item_func_find_in_set(Item *a,Item *b) :Item_int_func(a,b),enum_value(0) {}
longlong val_int();
......
......@@ -1037,6 +1037,15 @@ void Item_func_substr::fix_length_and_dec()
}
void Item_func_substr_index::fix_length_and_dec()
{
max_length= args[0]->max_length;
if (collation.set(args[0]->collation, args[1]->collation) ||
(collation.derivation == DERIVATION_NONE))
my_coll_agg_error(args[0]->collation, args[1]->collation, func_name());
}
String *Item_func_substr_index::val_str(String *str)
{
String *res =args[0]->val_str(str);
......@@ -1054,6 +1063,8 @@ String *Item_func_substr_index::val_str(String *str)
if (!res->length() || !delimeter_length || !count)
return &empty_string; // Wrong parameters
res->set_charset(collation.collation);
#ifdef USE_MB
if (use_mb(res->charset()))
{
......
......@@ -215,7 +215,7 @@ class Item_func_substr_index :public Item_str_func
public:
Item_func_substr_index(Item *a,Item *b,Item *c) :Item_str_func(a,b,c) {}
String *val_str(String *);
void fix_length_and_dec() { max_length= args[0]->max_length; }
void fix_length_and_dec();
const char *func_name() const { return "substr_index"; }
};
......
......@@ -1453,17 +1453,18 @@ String *Item_sum_udf_str::val_str(String *str)
GROUP_CONCAT(DISTINCT expr,...)
*/
static int group_concat_key_cmp_with_distinct(void* arg, byte* key1,
int group_concat_key_cmp_with_distinct(void* arg, byte* key1,
byte* key2)
{
Item_func_group_concat* item= (Item_func_group_concat*)arg;
for (uint i= 0; i < item->arg_count_field; i++)
{
Item *field_item= item->expr[i];
Item *field_item= item->args[i];
Field *field= field_item->tmp_table_field();
if (field)
{
uint offset= field->offset();
uint offset= field->abs_offset;
int res= field->key_cmp(key1 + offset, key2 + offset);
/*
......@@ -1483,9 +1484,10 @@ static int group_concat_key_cmp_with_distinct(void* arg, byte* key1,
GROUP_CONCAT(expr,... ORDER BY col,... )
*/
static int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2)
int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2)
{
Item_func_group_concat* item= (Item_func_group_concat*)arg;
for (uint i=0; i < item->arg_count_order; i++)
{
ORDER *order_item= item->order[i];
......@@ -1493,7 +1495,7 @@ static int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2)
Field *field= item->tmp_table_field();
if (field)
{
uint offset= field->offset();
uint offset= field->abs_offset;
bool dir= order_item->asc;
int res= field->key_cmp(key1 + offset, key2 + offset);
......@@ -1513,8 +1515,7 @@ static int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2)
GROUP_CONCAT(DISTINCT expr,... ORDER BY col,... )
*/
static int group_concat_key_cmp_with_distinct_and_order(void* arg,
byte* key1,
int group_concat_key_cmp_with_distinct_and_order(void* arg,byte* key1,
byte* key2)
{
if (!group_concat_key_cmp_with_distinct(arg,key1,key2))
......@@ -1528,7 +1529,7 @@ static int group_concat_key_cmp_with_distinct_and_order(void* arg,
item is pointer to Item_func_group_concat
*/
static int dump_leaf_key(byte* key, uint32 count __attribute__((unused)),
int dump_leaf_key(byte* key, uint32 count __attribute__((unused)),
Item_func_group_concat *group_concat_item)
{
char buff[MAX_FIELD_WIDTH];
......@@ -1539,13 +1540,12 @@ static int dump_leaf_key(byte* key, uint32 count __attribute__((unused)),
for (uint i= 0; i < group_concat_item->arg_show_fields; i++)
{
Item *show_item= group_concat_item->expr[i];
Item *show_item= group_concat_item->args[i];
if (!show_item->const_item())
{
Field *f= show_item->tmp_table_field();
uint offset= f->offset();
char *sv= f->ptr;
f->ptr= (char *)key + offset;
f->ptr= (char *)key + f->abs_offset;
String *res= f->val_str(&tmp,&tmp2);
group_concat_item->result.append(*res);
f->ptr= sv;
......@@ -1595,9 +1595,14 @@ Item_func_group_concat::Item_func_group_concat(bool is_distinct,
List<Item> *is_select,
SQL_LIST *is_order,
String *is_separator)
:Item_sum(), tmp_table_param(0), warning_available(false),
:Item_sum(), tmp_table_param(0), max_elements_in_tree(0), warning(0),
warning_available(0), key_length(0), rec_offset(0),
tree_mode(0), distinct(is_distinct), warning_for_row(0),
separator(is_separator), tree(&tree_base), table(0),
count_cut_values(0), tree_mode(0), distinct(is_distinct)
order(0), tables_list(0), group_concat_max_len(0),
show_elements(0), arg_count_order(0), arg_count_field(0),
arg_show_fields(0), count_cut_values(0)
{
original= 0;
quick_group= 0;
......@@ -1613,16 +1618,12 @@ Item_func_group_concat::Item_func_group_concat(bool is_distinct,
We need to allocate:
args - arg_count+arg_count_order (for possible order items in temporare
tables)
expr - arg_count_field
order - arg_count_order
*/
args= (Item**) sql_alloc(sizeof(Item*)*(arg_count+arg_count_order+
arg_count_field)+
args= (Item**) sql_alloc(sizeof(Item*)*(arg_count+arg_count_order)+
sizeof(ORDER*)*arg_count_order);
if (!args)
return; // thd->fatal is set
expr= args;
expr+= arg_count+arg_count_order;
return;
/* fill args items of show and sort */
int i= 0;
......@@ -1630,12 +1631,12 @@ Item_func_group_concat::Item_func_group_concat(bool is_distinct,
Item *item_select;
for ( ; (item_select= li++) ; i++)
args[i]= expr[i]= item_select;
args[i]= item_select;
if (arg_count_order)
{
i= 0;
order= (ORDER**)(expr + arg_count_field);
order= (ORDER**)(args + arg_count + arg_count_order);
for (ORDER *order_item= (ORDER*) is_order->first;
order_item != NULL;
order_item= order_item->next)
......@@ -1696,13 +1697,15 @@ bool Item_func_group_concat::reset()
bool Item_func_group_concat::add()
{
if (always_null)
return 0;
copy_fields(tmp_table_param);
copy_funcs(tmp_table_param->items_to_copy);
bool record_is_null= TRUE;
for (uint i= 0; i < arg_show_fields; i++)
{
Item *show_item= expr[i];
Item *show_item= args[i];
if (!show_item->const_item())
{
Field *f= show_item->tmp_table_field();
......@@ -1718,13 +1721,13 @@ bool Item_func_group_concat::add()
null_value= FALSE;
if (tree_mode)
{
if (!tree_insert(tree, table->record[0], 0,tree->custom_arg))
if (!tree_insert(tree, table->record[0] + rec_offset, 0, tree->custom_arg))
return 1;
}
else
{
if (result.length() <= group_concat_max_len && !warning_for_row)
dump_leaf_key(table->record[0],1,
dump_leaf_key(table->record[0] + rec_offset, 1,
(Item_func_group_concat*)this);
}
return 0;
......@@ -1757,12 +1760,6 @@ Item_func_group_concat::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
return 1;
maybe_null |= args[i]->maybe_null;
}
for (i= 0 ; i < arg_count_field ; i++)
{
if (expr[i]->fix_fields(thd, tables, expr + i) || expr[i]->check_cols(1))
return 1;
maybe_null |= expr[i]->maybe_null;
}
/*
Fix fields for order clause in function:
GROUP_CONCAT(expr,... ORDER BY col,... )
......@@ -1796,6 +1793,7 @@ bool Item_func_group_concat::setup(THD *thd)
/*
all not constant fields are push to list and create temp table
*/
always_null= 0;
for (uint i= 0; i < arg_count; i++)
{
Item *item= args[i];
......@@ -1808,6 +1806,8 @@ bool Item_func_group_concat::setup(THD *thd)
always_null= 1;
}
}
if (always_null)
return 0;
List<Item> all_fields(list);
if (arg_count_order)
......@@ -1827,12 +1827,25 @@ bool Item_func_group_concat::setup(THD *thd)
return 1;
table->file->extra(HA_EXTRA_NO_ROWS);
table->no_rows= 1;
qsort_cmp2 compare_key;
tree_mode= distinct || arg_count_order;
Field** field, **field_end;
field_end = (field = table->field) + table->fields;
uint offset = 0;
for (key_length = 0; field < field_end; ++field)
{
uint32 length= (*field)->pack_length();
(*field)->abs_offset= offset;
offset+= length;
key_length += length;
}
rec_offset = table->reclength - key_length;
/*
choise function of sort
*/
tree_mode= distinct || arg_count_order;
qsort_cmp2 compare_key;
if (tree_mode)
{
if (arg_count_order)
......@@ -1856,9 +1869,9 @@ bool Item_func_group_concat::setup(THD *thd)
*/
init_tree(tree, min(thd->variables.max_heap_table_size,
thd->variables.sortbuff_size/16), 0,
table->reclength, compare_key, 0, NULL, (void*) this);
max_elements_in_tree= ((table->reclength) ?
thd->variables.max_heap_table_size/table->reclength : 1);
key_length, compare_key, 0, NULL, (void*) this);
max_elements_in_tree= ((key_length) ?
thd->variables.max_heap_table_size/key_length : 1);
};
item_thd= thd;
......@@ -1909,3 +1922,6 @@ String* Item_func_group_concat::val_str(String* str)
}
return &result;
}
......@@ -644,13 +644,28 @@ class Item_func_group_concat : public Item_sum
uint max_elements_in_tree;
MYSQL_ERROR *warning;
bool warning_available;
uint key_length;
int rec_offset;
bool tree_mode;
bool distinct;
bool warning_for_row;
bool always_null;
friend int group_concat_key_cmp_with_distinct(void* arg, byte* key1,
byte* key2);
friend int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2);
friend int group_concat_key_cmp_with_distinct_and_order(void* arg,
byte* key1,
byte* key2);
friend int dump_leaf_key(byte* key, uint32 count __attribute__((unused)),
Item_func_group_concat *group_concat_item);
public:
String result;
String *separator;
TREE tree_base;
TREE *tree;
TABLE *table;
Item **expr;
ORDER **order;
TABLE_LIST *tables_list;
ulong group_concat_max_len;
......@@ -659,9 +674,6 @@ class Item_func_group_concat : public Item_sum
uint arg_count_field;
uint arg_show_fields;
uint count_cut_values;
bool tree_mode, distinct;
bool warning_for_row;
bool always_null;
/*
Following is 0 normal object and pointer to original one for copy
(to correctly free resources)
......@@ -677,10 +689,14 @@ class Item_func_group_concat : public Item_sum
max_elements_in_tree(item.max_elements_in_tree),
warning(item.warning),
warning_available(item.warning_available),
key_length(item.key_length),
rec_offset(item.rec_offset),
tree_mode(0),
distinct(item.distinct),
warning_for_row(item.warning_for_row),
separator(item.separator),
tree(item.tree),
table(item.table),
expr(item.expr),
order(item.order),
tables_list(item.tables_list),
group_concat_max_len(item.group_concat_max_len),
......@@ -689,9 +705,6 @@ class Item_func_group_concat : public Item_sum
arg_count_field(item.arg_count_field),
arg_show_fields(item.arg_show_fields),
count_cut_values(item.count_cut_values),
tree_mode(0),
distinct(item.distinct),
warning_for_row(item.warning_for_row),
original(&item)
{
quick_group = 0;
......
......@@ -1227,7 +1227,7 @@ JOIN::exec()
{
// Some tables may have been const
curr_join->tmp_having->update_used_tables();
JOIN_TAB *table= &curr_join->join_tab[const_tables];
JOIN_TAB *table= &curr_join->join_tab[curr_join->const_tables];
table_map used_tables= curr_join->const_table_map | table->table->map;
Item* sort_table_cond= make_cond_for_table(curr_join->tmp_having,
......@@ -1263,7 +1263,7 @@ JOIN::exec()
We can abort sorting after thd->select_limit rows if we there is no
WHERE clause for any tables after the sorted one.
*/
JOIN_TAB *table= &curr_join->join_tab[const_tables+1];
JOIN_TAB *table= &curr_join->join_tab[curr_join->const_tables+1];
JOIN_TAB *end_table= &curr_join->join_tab[tables];
for (; table < end_table ; table++)
{
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment