maria-developers team mailing list archive

[Michael Widenius <monty@xxxxxxxxxxxx>]

Hi!

>>>>> "Jeremy" == Jeremy Thomerson <jeremy@xxxxxxxxxxxxxxxxxxx> writes:

Jeremy> Monty, et al...
Jeremy> On Mon, Jun 13, 2011 at 9:36 AM, Michael Widenius <monty@xxxxxxxxxxxx>wrote:

>> 
>> Hi!
>> 
>> >>>>> "Sergei" == Sergei Golubchik <serg@xxxxxxxxxxxx> writes:
>> 
Sergei> Hi, Jeremy!
Sergei> On Jun 10, Jeremy Thomerson wrote:
>> >> I just submitted a patch to a MySQL bug that I am also experiencing.  I
>> >> would like to see it accepted into the wider community (MySQL, MariaDB,
>> >> Percona, etc).  Please let me know what I need to do so that this patch
>> >> could also be accepted into your project.
>> >>
>> >> The bug is here: http://bugs.mysql.com/bug.php?id=59449
>> >> The "[11 Jun 3:16] Jeremy Thomerson" comment will be the most useful.
>> 


Sergei> But if Oracle will refuse to fix this bug, we will look at your
>> patch,
Sergei> certainly.
>> 
>> To ensure we don't forget about this, if you are not sure that Oracle
>> has fixed the bug within one month, ping us and we will look at
>> adding this into MariaDB at once.
>> 

Jeremy> Thanks a bunch!  I was away a lot in July and August and didn't get to
Jeremy> follow up on this.  Looking at the original bug report, I have seen
Jeremy> absolutely no activity.  I'm not even sure why they have not touched /
Jeremy> accepted it.  If anyone here knows a way to get it into one (or both)
Jeremy> projects, I'd appreciate any help you can offer.

The patch you suggested was:

=== modified file 'sql/sql_base.cc'
--- sql/sql_base.cc	2011-04-12 10:59:59 +0000
+++ sql/sql_base.cc	2011-06-10 20:11:52 +0000
@@ -6975,6 +6975,28 @@
         *counter= i;
         *resolution= RESOLVED_IGNORING_ALIAS;
         break;
+      } 
+      else if (find->type() == Item::FUNC_ITEM && ((Item_func*) find)->functype() == Item_func::COLLATE_FUNC && ((Item_func*) find)->arg_count == 2) 
+      {
+        /*
+          If someone uses "ORDER BY column_name COLLATE utf8_spanish_ci", it is a function
+          that we are trying to find, and not a real column.  Bug #59449 exposes that this
+          failed when column_name is actually referring to the name of an alias, whether 
+          that alias is for a real column or for a function result (such as coalesce). This
+          block of code finds the real item, which is the "column" referred to by the 
+          collation function for this scenario.
+        */
+        Item *collated_column = ((Item_func*) find)->arguments()[0];
+        int is_alias = item->type() == Item::FUNC_ITEM ||
+                       ((item->type() == Item::FIELD_ITEM || item->type() == Item::REF_ITEM) &&
+                        my_strcasecmp(system_charset_info, ((Item_ident*) item)->field_name, item->name));
+        if (is_alias && collated_column->name && item->name && !my_strcasecmp(system_charset_info, item->name, collated_column->name))
+        {
+          found = &collated_column;
+          *counter = i;
+          *resolution = RESOLVED_BEHIND_ALIAS;
+          break;
+        }
       }
     }
     else if (table_name && item->type() == Item::REF_ITEM &&

What I don't like with the patch is that we make COLLATE a special
case. I think that all usage of alias names in ORDER BY should be
treated the same.

In other words:

ORDER BY LanguageName
ORDER BY LanguageName COLLATE utf8_spanish_ci
ORDER BY concat(LanguageName,"aa")

should all use either the column or the alias in the same manner.

As we normally prefer alias for a column name in the ORDER BY,
I think we should fix this so that we always prefer the alias name in
all ORDER BY context.

The other option would be to give an error in this case as it's not
clear what the user wants.  This is hower likely to break many old
applictions.

After talking with Sergei, he concluded:

"yes, I believe now that according to the SQL:2003 (ISO/IEC
9075-2:2003, part 2 (foundations), 14.1 <declare cursor>, Syntax
Rules, item 18) aliases from the select list take precedence over
column names from the table"

The standard is very vague in the area, but it seams to support the
idea that alias'es should resolve first).

This means that we should fix things to always first prefer alias.
The fix in this case needs to be done in item::fix_fields() and not in
sql_base.cc

I will ask Timour to see if he can provide a fix for this.
(As he knows the fix_fields code quite well and he needs something to
bite into now when he is back from his vacation).

Timour, can you handle this?

Regards,
Monty