Skip to main content
2 of 5
fixed second query; improved processing of second query results
200_success
  • 144.2k
  • 22
  • 188
  • 473

Preface

For this review, we shall need to refer to the database schema for the WordPress Simmer plugin:

if ( $items_table_name != $wpdb->get_var( "SHOW TABLES LIKE '$items_table_name'" ) ) {
        // The recipe items table.
        $query .= "CREATE TABLE $items_table_name (
               recipe_item_id bigint(20) NOT NULL auto_increment,
               recipe_item_type varchar(200) NOT NULL DEFAULT '',
               recipe_id bigint(20) NOT NULL,
               recipe_item_order int(11) NOT NULL DEFAULT '0',
               PRIMARY KEY  (recipe_item_id),
               KEY recipe_id (recipe_id)
               ) $charset_collate;";
}

if ( $item_meta_table_name != $wpdb->get_var( "SHOW TABLES LIKE '$item_meta_table_name'" ) ) {

        // The recipe item meta table.
        $query .= "CREATE TABLE $item_meta_table_name (
               meta_id bigint(20) NOT NULL auto_increment,
               recipe_item_id bigint(20) NOT NULL,
               meta_key varchar(255) NULL,
               meta_value longtext NULL,
               PRIMARY KEY  (meta_id),
               KEY recipe_item_id (recipe_item_id),
               KEY meta_key (meta_key)
               ) $charset_collate;";
}

Querying

To start, let's format your first query for readability:

query = """
    SELECT a.recipe_id, a.recipe_item_type
         , b.meta_key, b.meta_value, b.recipe_item_id
        FROM wp_simmer_recipe_items a, wp_simmer_recipe_itemmeta b
        WHERE a.recipe_item_id = b.recipe_item_id
        GROUP BY a.recipe_item_id
"""

Your query has a non-sensical GROUP BY clause. The only way that this query could be legal is if the schema and query are formulated so as to guarantee that each recipe_item_id only appears at most once in the query result. While the wp_simmer_recipe_items table does have a PRIMARY KEY (recipe_item_id) constraint, there is no such uniqueness guarantee in the wp_simmer_recipe_itemmeta table. MySQL < 5.7.5 somehow executes the query despite the fact that it makes no sense; in MySQL ≥ 5.7.5, it should fail with an error, as it should in any sane SQL implementation:

ERROR 1055 (42000): Expression #3 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'somedb.b.meta_key' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

Based on the query results, you build the ingredients and instructions lists by appending entries. However, you fail to ORDER BY recipe_item_order, so the ingredients and instructions may appear in any order. (Try baking the bread before mixing the dough!)

The worst sin in this code is executing query1 for each recipe found by query. This is a bad idea for two reasons:

  • For good performance, you should never execute queries in a loop, especially when the number of secondary queries depends on the length of the results of the primary query. Each additional query necessitates a round trip to the server, and takes time to interpret and execute.

  • The information returned by query1 is entirely redundant — all of the information you need from it (namely, the meta_key and meta_value for each recipe_item_id) is already contained within the results of query!

Furthermore, you don't need to make two connections to the same database. All you need is two cursors, which can be created from just one connection.

Suggested solution

Take advantage of itertools.groupby() to help construct the inner lists and dictionaries.

from itertools import groupby
import json
from operator import itemgetter
import mysql.connector

cnx = mysql.connector.connect(
    database='somedb', user='root', password='root', host='localhost'
)
cursor = cnx.cursor()
data = {}

cursor.execute("""
    SELECT item.recipe_id
         , meta.meta_value AS instruction_text
        FROM wp_simmer_recipe_items item
            INNER JOIN wp_simmer_recipe_itemmeta meta
                ON item.recipe_item_id = meta.recipe_item_id
        WHERE
            item.recipe_item_type = 'instruction'
            AND meta.meta_key <> 'is_heading'
        ORDER BY item.recipe_id, item.recipe_item_order
""")
for recipe_id, instructions in groupby(cursor.fetchall(), itemgetter(0)):
    data[recipe_id] = {
        'instructions': [row[1] for row in instructions]
    }

cursor.execute("""
    SELECT item.recipe_id
         , meta.recipe_item_id
         , meta.meta_key
         , meta.meta_value
        FROM wp_simmer_recipe_items item
            INNER JOIN wp_simmer_recipe_itemmeta meta
                ON item.recipe_item_id = meta.recipe_item_id
        WHERE
            item.recipe_item_type <> 'instruction'
        ORDER BY item.recipe_id, item.recipe_item_order, meta.meta_key
""")
for recipe_id, item in groupby(cursor.fetchall(), itemgetter(0)):
    data[recipe_id]['ingredients'] = [
        {row[2]: row[3] for row in item_attributes}
        for _, item_attributes in groupby(item, itemgetter(0, 1))
    ]

cursor.close()
cnx.close()

with open('data.json', 'w') as outfile:
    json.dump(data, outfile, sort_keys=True, indent=4)

Further discussion

MySQL, since version 5.7.22, has JSON support. It would be nice to rewrite the SQL such that the entire JSON result could be returned from just a single query. Unfortunately, the JSON_ARRAYAGG() function does not let you specify the order of its results:

JSON_ARRAYAGG(col_or_expr)

Aggregates a result set as a single JSON array whose elements consist of the rows. The order of elements in this array is undefined.

That, to me, would be a deal-breaker for exporting a recipe.

200_success
  • 144.2k
  • 22
  • 188
  • 473