Pulling a group from a database

Question

Alright i have a magic cards site and im trying to pull all of the cards from a certain set if you click on the set on the home page. www(dot)magiccards(dot)me the code on the first page is:

    <?php

require("mysqlconnect.php");

$query = "SELECT COUNT(*) AS `Rows`, `set`,id FROM `magic_cards_copy`  GROUP BY `set` ORDER BY `set`";

$result = mysql_query($query) or die(mysql_error());

// Print out result
while($row = mysql_fetch_array($result)){
    $setlink = $row[1];
    $setlink = str_replace(" ", "", $setlink);
    $setlink = strtolower($setlink);
    $setlink = preg_replace("/[^a-z0-9]./","",$setlink);
    $setlink .= "-c-$row[2]";
    $setlink .= ".html";

    $navigation .= "< href=\"$setlink\">$row[1]</a> <small><i>($row[0])</i></small>";
}

require("template.php");

?>

and the code on the page that comes is:

    <?

require("mysqlconnect.php");

$cat=$_GET['cat'];

echo "Category: $cat<br>";

$query = "SELECT * FROM `magic_cards_copy` WHERE id = $cat ";

$result = mysql_query($query) or die(mysql_error());
$row = mysql_fetch_array($result);
echo "Set: $row[1]<br>";

?>

how would have the code pull up the cards from each set? any help would be great. this is more of a practice site for me.

Answer 1

Not really sure what your asking, but the first thing to look at seeing as this is a practice site is to

$cat = mysql_real_escape_string($_GET['cat']);

at the very least to prevent SQL injection hacks. You should always practice security. PHP.not on SQL Injection

Answer 2

It's not clear from the question what you're trying to do - for example, what is the relationship between the two PHP scripts provided, and are you trying to pull the cards from ALL sets, or the cards from a GIVEN set? I'll try to answer both:

All cards in set $_GET['cat']:

<?php
$dbh = new PDO('mysql:dbname=testdb;host=127.0.0.1', 'dbuser', 'dbpass');
$stmt = $dbh->prepare('SELECT cardname from magic_cards_copy where set = :set');
$stmt->bindParam(':set', $_GET['cat'], PDO::PARAM_STR);
$stmt->execute();

while ($card = $stmt->fetch(PDO::FETCH_ASSOC)) {
  // display card here
}

All cards in all sets:

<?php
$dbh = new PDO('mysql:dbname=testdb;host=127.0.0.1', 'dbuser', 'dbpass');
$query = $dbh->query('SELECT cardname from magic_cards_copy');

while ($card = $query->fetch(PDO::FETCH_ASSOC)) {
  // display card here
}

Note that I strongly recommend you use PDO, as it protects you from all sorts of foolishness.

Answer 3

I would create a table that has two columns. The first column is the set-id (a unique identifier for all sets) and it would be indexed for fast lookups. The second column would be the id's for the cards. You would use this table for a JOIN on the table containing all unique cards

BTW, sanitize all input

$cat=$_GET['cat'];

is open to SQL injection attacks. You should also cast it to an integer.

Answer 4

Read up on Database Normalization. What you ought to do is have one table of "sets" where you list each set with an ID number. Then in your cards database, instead of having the set name, you have an ID number corresponding to the set.

Example sets table:

id    set
------------------
1     Set One Name
2     Set Two Name

Example cards table:

id    setid   card
-------------------------------------
1     1       Card One from First Set
2     1       Card Two from First Set
3     2       A card from Second Set

When you want to list sets, you simply select everything from the sets table. When you want to list cards from a set you select all cards where the set ID is whatever you're looking for.

Answer 5

echo "Category: $cat<br>";

This is subject to a Cross-Site Scripting vulnerability. Since you're using this as a practice site, you should be aware of security flaws and how to avoid them. Start by reading the OWASP site.

As others have noted, you also have an SQL Injection flaw.

Also, here's output from your site:

Category: 143345
Set: Alliances
You have an error in your SQL syntax; check the manual that corresponds to 
your MySQL server version for the right syntax to use near ''magic_cards_copy' 
WHERE category = 'Alliances' LIMIT 40' at line 1

Looks like you're using the wrong kind of delimiter around your table name. Use back-quotes, not single-quotes, around the table name.

You do need to create another table, to record the membership of each card in each set. This is sometimes called a "many-to-many table" or "intersection table." This is especially true because IIRC, some Magic cards can belong to multiple sets.

So here's how I'd do it:

CREATE TABLE CardSets (
  set_id INT PRIMARY KEY AUTO_INCREMENT,
  set_name VARCHAR(40)
);

CREATE TABLE Cards (
  card_id INT PRIMARY KEY AUTO_INCREMENT,
  card_name VARCHAR(40)
  -- other card attributes, color, flavor text, etc.
);

CREATE TABLE CardSetManifest (
  set_id INT NOT NULL,
  card_id INT NOT NULL,
  -- other attributes of card specific to a given set, e.g. rarity
  PRIMARY KEY (set_id, card_id),
  FOREIGN KEY (set_id) REFERENCES CardSets(set_id),
  FOREIGN KEY (card_id) REFERENCES Cards(card_id)
);

So given a set_id you can get the count of cards in that set:

SELECT set_id, COUNT(*) FROM CardSetManifest GROUP BY set_id;

Given a set_id you can get a list of the cards in that set:

SELECT m.set_id, m.card_id, c.card_name
FROM CardSetManifest m JOIN Cards c USING (card_id);