Ambiguous column error with groupby & join

[ashleyfae]

If you have a GROUPBY clause, but you're also joining on a table that just so happens to have that same column name, you can end up with an error like this:

WordPress database error Column 'status' in field list is ambiguous for query SELECT status, COUNT(*) as count
FROM wp_edd_orders edd_o
         INNER JOIN wp_edd_order_items edd_oi ON (
        edd_o.id = edd_oi.order_id
        AND edd_oi.product_id = 294
    )
WHERE edd_o.type = 'sale'
GROUP BY edd_o.status

This is because the GROUPBY column is added to the selected fields here, but specifically without the table alias:

https://github.com/berlindb/core/blob/master/query.php#L1392

Note the second parameter for parse_groupby() is false, which means the alias is not added as a prefix. Changing that to true fixed the issue for me, but I wasn't sure if there was a specific reason it was false?

[JJJ]

Hey @ashleyfae. Sorry you ran into this.

Speculating using my fuzzy memory of writing this code, I think it has 2 separate origins.

1. As a simplified approach to avoid forking WP_Meta_Query::find_compatible_table_alias().
2. Came from a time before everything included the table alias (having evolved from WP_Query over the years)

If that's at all accurate, my current thoughts on them are:

1. It would still stink to need to fork that method, but some version of it will need to happen sometime in the future to invent/improve how Column relationships work (which is currently non-existent at best).
2. Table alias coverage is at or near 100% now, and parse_groupby() is private, so the $alias parameter could be removed to simply always include them.

Related: (parse_fields() works similarly, but it isn't ever used without aliases.

---

I recommend we change that false on line 1392 to $alias, so that parse_groupby() matches the results of parse_fields() when called internally. If the fields include table aliases, the groupby definitely should too.

What do you think?

[ashleyfae]

I recommend we change that false on line 1392 to $alias, so that parse_groupby() matches the results of parse_fields() when called internally. If the fields include table aliases, the groupby definitely should too.

What do you think?

Yep that works!