Proper syntax for sql query

Discussion in 'PHP' started by egdcltd, Jun 29, 2007.

  1. #1
    I am having problems wording the following query to do what I want

    				$sql = "SELECT *
    					FROM " . ADR_LIBRARY_TABLE ."
    					WHERE book_difficulty <= $book_difficulty 
    					AND book_zone = $area_id
    					AND book_false = '0'";
    PHP:
    book_zone can be '0', any other number, eg '5' or several numbers, eg 1,3,4,52

    $area_id is only ever a whole integer with a value of 1 or greater

    I want to word the book_zone bit so that it will select the results where book_zone = '0' or book_zone = $area_id (when book_zone has only one number stored in it) or $area_id is one of the series of numbers stored in book_zone, eg if $area_id = '1' and book_zone = 1,4,5,6,11 butmaking sure it only takes whole numbers in the last case, so if $area_id is '1' it won't select where book_zone has '11' stored in it.
     
    egdcltd, Jun 29, 2007 IP
  2. jestep

    jestep Prominent Member

    Messages:
    3,659
    Likes Received:
    215
    Best Answers:
    19
    Trophy Points:
    330
    #2
    What are you getting right now that is wrong. It looks like your query should work for what you are trying to do.

    The only think i would change is putting ' around the other values:
    
    WHERE book_difficulty <= '$book_difficulty' 
                        AND book_zone = '$area_id'
                        AND book_false = '0'";
    
    PHP:
     
    jestep, Jun 29, 2007 IP
  3. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #3
    The query works, just not the way I want. It never gets the results if book_zone is '0', whilst it should always get them. Plus, when book_zone has more than 1 value, it only checks the first one against $area_id
     
    egdcltd, Jun 29, 2007 IP
  4. jestep

    jestep Prominent Member

    Messages:
    3,659
    Likes Received:
    215
    Best Answers:
    19
    Trophy Points:
    330
    #4
    I see what you saying. Basically the book_zone can have a csv list of numbers, and you're having problems when there is more than 1 number in there.
     
    jestep, Jun 29, 2007 IP
  5. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #5
    Yes, or when it equals '0'. I know explode can break the list into seperate values, but then I can't get it to work with the next part of the script.

    Basically, the first sql selects a number of results where all values are true, including book_zone, then one of these results is picked randomly, and information on that result is got. Although I think the second sql could possibly be removed, I can't see how to both get a random result and explode book_zone

    Here's the code segment
    				$sql = "SELECT *
    					FROM " . ADR_LIBRARY_TABLE ."
    					WHERE book_difficulty <= $book_difficulty 
    					AND book_zone = $area_id
    					AND book_false = '0'";
    				$result = $db->sql_query($sql);
    				if( !$result )
    				{
    					message_die(GENERAL_ERROR, 'Could not obtain book information', "", __LINE__, __FILE__, $sql);
    				}
    				$books = mysql_num_rows($result);
    				$library = $db->sql_fetchrowset($result);
    			}
    
    			if ( !$library )
    			{
    				adr_previous ( Adr_library_failure , adr_research , '' );
    			}
    			else
    			{
    				// Now roll for research
    				$rnd_research = mt_rand ( 0 , (($books) - '1'));
    
    				$sql = "SELECT *
    					FROM " . ADR_LIBRARY_TABLE ."
    					WHERE book_id = '".$library[$rnd_research]['book_id']."' ";
    				$result = $db->sql_query($sql);
    				if( !$result )
    				{
    					message_die(GENERAL_ERROR, 'Could not obtain book information', "", __LINE__, __FILE__, $sql);
    				}
    				$research_result = $db->sql_fetchrow($result);
    PHP:
     
    egdcltd, Jun 29, 2007 IP
  6. TwistMyArm

    TwistMyArm Peon

    Messages:
    931
    Likes Received:
    44
    Best Answers:
    0
    Trophy Points:
    0
    #6
    Having multiple values in a single value is bad DB management. You should, instead, look at reorganising your database so that each field only ever represents a single piece of information.

    Not knowing your DB schema doesn't help, but I daresay all you would need to do is create a 'book_zone' table that has two columns: book ID and book zone, with the combination of columns being unique, but allowing more than one row to have the same book ID.

    The way it is now, you could treat it like a string, but that would be BAD.
     
    TwistMyArm, Jun 29, 2007 IP
  7. jestep

    jestep Prominent Member

    Messages:
    3,659
    Likes Received:
    215
    Best Answers:
    19
    Trophy Points:
    330
    #7
    By not changing the database you pretty much need to not use the book_zone in your select statement. Then you would explode it and use php to fliter out the results you don't need.
     
    jestep, Jun 29, 2007 IP
  8. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #8
    I have no idea how to do it by modifying the database. I wouldn't have the first clue about how to structure things.

    Regarding using explode, I can select all the instances where WHERE book_difficulty <= $book_difficulty AND book_false = '0' then explode the book_zone into it's seperate values. I just don't know how to get all the instances from that where book_zone = $area_id, then randomly select one of them.

    I get so far, then can't get any further

                   $sql = "SELECT *
                        FROM " . ADR_LIBRARY_TABLE ."
                        WHERE book_difficulty <= $book_difficulty 
                        AND book_false = '0'";
                    $result = $db->sql_query($sql);
                    if( !$result )
                    {
                        message_die(GENERAL_ERROR, 'Could not obtain book information', "", __LINE__, __FILE__, $sql);
                    }
                    $books = mysql_num_rows($result);
                    $library = $db->sql_fetchrowset($result);
    
    for ( $i = 0 ; $i < count( $library ) ; $i++ )
    {
    	$book_zone_array = explode( ',' , $library[$i]['book_zone'] );
    
    	if( in_array( $area_id , $book_zone_array ) || $book_zone_array[0] == '0' )
    	{
    
    	}
    }
    PHP:
     
    egdcltd, Jun 29, 2007 IP
  9. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #9
    Could you explain why this is bad please? I'm not doubting you, but as the alternative methods would, I think, hugely increase the number of sql queries (eg, if book_zone has thirty values, total number of queries to do this insert would be 31, not 1), why is this preferable to storing all the values in a string?
     
    egdcltd, Jun 30, 2007 IP
  10. TwistMyArm

    TwistMyArm Peon

    Messages:
    931
    Likes Received:
    44
    Best Answers:
    0
    Trophy Points:
    0
    #10
    Well for starters, yes, you will have multiple inserts, but they can be processed by a single 'query', so it's not a massive problem. You're not going to be saving any space (in fact, it's possible that you would save more space by using the numbers singly) and essentially, it's the 'right' way to do it.

    Believe it or not, but most times you will find that if you do it the right way, you don't end up with (for example) the problems that you are having right now. If you STORE the data the way it should be stored, you can READ the data the way you will want to read it. In the future, you might want to work out how many places have a book... if you store it properly, it's a single count, if you don't, it's another hack, right?

    The biggest problem is that, like I said, you are using a single field to store multiple pieces of information. It will come back to bite you, as it already has...
     
    TwistMyArm, Jun 30, 2007 IP
  11. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #11
    Do you have any examples of scripts that use the SQL in the way you suggested (preferably phpBB mods)? Basically, I find a script, or bit of code, that does something similar to what I want and adapt it. That's why I was using that method, all the scripts I looked at were using it.

    Here's the structure of the table at present

    CREATE TABLE `phpbb_adr_library` (
      `book_id` int(8) NOT NULL auto_increment,
      `book_name` varchar(255) NOT NULL default '',
      `book_details` longtext NOT NULL,
      `book_zone` text NOT NULL,
      `book_difficulty` tinyint(2) NOT NULL default '2',
      `book_false` int(1) NOT NULL default '0',
      PRIMARY KEY  (`book_id`)
    ) ENGINE=MyISAM AUTO_INCREMENT=6 DEFAULT CHARSET=latin1 AUTO_INCREMENT=6 ;
    PHP:
    I would assume that I would need to remove the book_zone column, then create a new table with two columns, book_id and book_zone, with book_id being the same as the one from the original table (using the same id looks like it will be fairly easy)

    The existing admin-side code uses a multiple selection box to get the book_zone, then uses the following code to make the value for db insertion

    			if ( in_array( '0' , $HTTP_POST_VARS['book_zone'] ) )
    			    $book_zone = '0';
    			else
    				$book_zone = implode(',' , $HTTP_POST_VARS['book_zone']);
    PHP:
    I'm not sure how to replace that. Also, don't know how to do the user-side with two tables.
     
    egdcltd, Jun 30, 2007 IP
  12. TwistMyArm

    TwistMyArm Peon

    Messages:
    931
    Likes Received:
    44
    Best Answers:
    0
    Trophy Points:
    0
    #12
    Do I have any examples? I would say that most scripts do it. It's a hard thing to point to and say 'see, there' because even if you see it, well, it probably doesn't help much with your situation anyway.

    So you're right about the table structures. The rest of the stuff is difficult to help you with (realistically) without more code, but essentially for your insert you would just loop over the $HTTP_POST_VARS['book_zone'] array and do the inserts one at a time. Without more code, it's hard to offer much more assistance I'm afraid... if you can show us all of the code that is used in creating and running your insert and all of the code that is used to do your query for selecting then we can help more easily.

    I won't get into the fact that your code does not seem to be very secure from SQL poisoning (just yet).
     
    TwistMyArm, Jun 30, 2007 IP
  13. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #13
    egdcltd, Jun 30, 2007 IP
  14. TwistMyArm

    TwistMyArm Peon

    Messages:
    931
    Likes Received:
    44
    Best Answers:
    0
    Trophy Points:
    0
    #14
    OK, not exactly what I asked for, but anyway....

    With regards to the save function, I would use code something like the following:
    
    case "save":
    
    	$book_id = ( !empty($HTTP_POST_VARS['book_id']) ) ? intval($HTTP_POST_VARS['book_id']) : intval($HTTP_GET_VARS['book_id']);
    	$book_name = ( isset($HTTP_POST_VARS['book_name']) ) ? trim($HTTP_POST_VARS['book_name']) : trim($HTTP_GET_VARS['book_name']);
    
    	// CHANGE: make $book_zone an array, or just an array of the '0' value
    	$book_zone = $HTTP_POST_VARS['book_zone'];
    	if ( in_array( '0' , $book_zone ) ) {
    		$book_zone = array( 0 );
    	}
    	// END OF CHANGE
    
    	$book_details = ( isset($HTTP_POST_VARS['book_details']) ) ? trim($HTTP_POST_VARS['book_details']) : trim($HTTP_GET_VARS['book_details']);
    	$book_difficulty = intval($HTTP_POST_VARS['book_difficulty']);
    	$book_false = intval($HTTP_POST_VARS['book_false']);
    
    	if (($book_name == '' ) || ($book_details == '' ))
    	{
    		message_die(MESSAGE, $lang['Fields_empty']);
    	}
    
    	// CHANGE: Remove the book_zone from this table
    	$sql = "UPDATE " . ADR_LIBRARY_TABLE . "
    		SET book_name = '" . str_replace("\'", "''", $book_name) . "', 	
    			book_details = '" . str_replace("\'", "''", $book_details) . "',
    			book_difficulty = $book_difficulty,
    			book_false = '$book_false'
    		WHERE book_id = " . $book_id;
    	if( !($result = $db->sql_query($sql)) )
    	{
    		message_die(GENERAL_ERROR, "Couldn't update book", "", __LINE__, __FILE__, $sql);
    	}
    
    	// CHANGE: Add the values of the book_zone array to the book_zone table, but drop the current ones first
    	$sql = 'DELETE FROM ' . ADR_BOOK_ZONE_TABLE . ' WHERE `book_id`=' . $book_id;
    	if( !($result = $db->sql_query($sql)) )
    	{
    		message_die(GENERAL_ERROR, "Couldn't delete book zones", "", __LINE__, __FILE__, $sql);
    	}
    
    	// this code builds up the multiple values for a single insert first.
    	$insert_values_array = array();
    	foreach( $book_zone as $zone_id ) {
    		$insert_value_array[] = '(' . $book_id . ', ' . $zone_id . ')';
    	}
    	$insert_values_string = implode(', ', $insert_values_array);
    	$sql = 'INSERT INTO ' . ADR_BOOK_ZONE_TABLE . '(`book_id`, `zone_id`) VALUES ' . $insert_values_string;
    	if( !($result = $db->sql_query($sql)) )
    	{
    		message_die(GENERAL_ERROR, "Couldn't insert book zones", "", __LINE__, __FILE__, $sql);
    	}
    
    	adr_previous( Adr_library_successful_edited , admin_adr_library , '' );
    	break;
    
    PHP:
    The 'savenew' code would be similar (you just wouldn't have to drop the values from the book zones table in the first place).

    Now, regardng the select statements... my SQL is rusty and I don't have a server here to test it on, but you would replace, for example:
    
    $sql = "SELECT *
    	FROM " . ADR_LIBRARY_TABLE ."
    	WHERE book_false = $book_false 
    	AND book_zone = $area_id";
    
    PHP:
    with something like:
    
    $sql = "SELECT " . ADR_LIBRARY_TABLE . ".*
    	FROM " . ADR_LIBRARY_TABLE ."
    	INNER JOIN " . ADR_BOOK_ZONE_TABLE . " ON " . ADR_LIBRARY_TABLE . ".book_id=" . ADR_BOOK_ZONE_TABLE . ".book_id 
    	WHERE book_false = $book_false";
    
    PHP:
    Something similar to that, anyway. Now, the PHP and the SQL are probably both syntactically incorrect, but hopefully you get the idea. If not, read up on SQL JOINs.
     
    TwistMyArm, Jun 30, 2007 IP
  15. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #15
    Okay, I'll take a look through, thanks.
     
    egdcltd, Jun 30, 2007 IP
  16. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #16
    Keep getting a MySQL from this bit, specifically the insert

        // this code builds up the multiple values for a single insert first.
        $insert_values_array = array();
        foreach( $book_zone as $zone_id ) {
            $insert_value_array[] = '(' . $book_id . ', ' . $zone_id . ')';
        }
        $insert_values_string = implode(', ', $insert_values_array);
        $sql = 'INSERT INTO ' . ADR_BOOK_ZONE_TABLE . '(`book_id`, `zone_id`) VALUES ' . $insert_values_string;
        if( !($result = $db->sql_query($sql)) )
        {
            message_die(GENERAL_ERROR, "Couldn't insert book zones", "", __LINE__, __FILE__, $sql);
        }
    PHP:
     
    egdcltd, Jun 30, 2007 IP
  17. TwistMyArm

    TwistMyArm Peon

    Messages:
    931
    Likes Received:
    44
    Best Answers:
    0
    Trophy Points:
    0
    #17
    "Keep getting a MySQL from this bit"... a MySQL what? I'll assume you mean a MySQL error. If so, how about you actually tell us what the error is?

    Help us to help you... I already mentioned that I don't have a server to test on... you have one. You're even getting an error and you don't tell us what it is. What do you expect us to do, then?

    When you tell us, too, be sure to tell us what you tried to fix it: the hard part is done, so surely you've had a few attempts at fixing it first.
     
    TwistMyArm, Jun 30, 2007 IP
  18. egdcltd

    egdcltd Peon

    Messages:
    691
    Likes Received:
    14
    Best Answers:
    0
    Trophy Points:
    0
    #18
    Sorry, hadn't realized I'd missed the error out. Bit tricky debugging an error if you don't know what it is.

    I've tried to echo the variables to see if data is being passed correctly; unfortunately, I'm unfamiliar with the method for doing the single insert, so the echos didn't really give anything useful out.
     
    egdcltd, Jul 1, 2007 IP
  19. ansi

    ansi Well-Known Member

    Messages:
    1,483
    Likes Received:
    65
    Best Answers:
    0
    Trophy Points:
    100
  20. TwistMyArm

    TwistMyArm Peon

    Messages:
    931
    Likes Received:
    44
    Best Answers:
    0
    Trophy Points:
    0
    #20
    @egdcltd: I'd have thought that your message_die function would have been called. At least that way we could tell what the complete query was, as that's what we really need (MySQL isn't as helpful as it could be with its error messages: the whole query helps).

    @ansi: thanks for the input. It's refreshing to see someone who pulls out the most minute detail and takes two seconds to link to something about that, as opposed to actually helping out with the problem at hand. It sometimes seems that on this forum people are less interested in actually helping than in getting a post (every second post about SQL these days seems to be simply "oh, your code is so hackable" without even looking at the question).
     
    TwistMyArm, Jul 1, 2007 IP