MySQL coding advice please.

#1 I don't know anything about the torque implementation of mySQL, but I am pretty experienced in its use from PHP though rusty as heck as I haven't used it in a while. However, looking at what you have up there you are missing your table name.

SELECT * FROM users WHERE userid=%userid

#2 John: Thanks.

But the point is, the table should not be defined like that. It should depend on what %userid has been set to. So if you type in JohnV in the username field, it will forward that to %userid, and I want SQL to use %userid to set the table name :)

I tried the change though, and it gave my syntax error. :/

I KNOW this can be done easily, I'm just not that great with scripts nor C++. ;)

#3 Eh? You have a table per user? That doesn't make any sense at all. Why would you do that? I see what you mean now its just weird :) Not standard DB design thats for sure lol.

Like I said, I don't know the specifics of the torque implementation so i'm sorry I can't be of more help. If I was doing what you want in PHP it would be like:

$query = "SELECT * FROM ".$userid;

Try this in Torque (my string handling is rusty in Torque)

%query = "SELECT * FROM " @ %userid;
%login.Query(%query);

#4 John's approach looks like it should work.

#5 Thanks alot John!

Now I have a few questions for you gurus out there ;)
Why would my approach not work? Size issue or performance issue or just not practical?

#6 Its basic database design to be honest.

Think of it like coding - you group similar objects in a class. Translated to db - you group similar data in a table. Users are similar, and have a user table.

No hocus pokus. It can be done the other way around theoretically, but it is extremely ugly and not practical.

E.g. what happens if you have an highscore table - and suddenly a user decides he should be called "highscore".......

#7 Yeah I guess I can't really say WHY you shouldn't do it that way beyond what Thomas has said. It just isn't the way you do it :p

For one, it is extremely inefficient to have all those tables. Space wise. Every table has overhead. To store a single record in a table is shear lunacy :) A huge waste. Heaven forbid you ever had to start doing any coress correlation between users. Then not only are you wasting space, but you are wasting time. Those lookups will be a lot slower than they could be.

Just make one table, "users" or whatever, and store all the users in it. Each user gets one entry.

#8 Thomas and John: Cheers guys :) Thanks for the heads up, I'll rebuild the DB using this example, I think it makes sense.

#9 Alright, with the new way of using the database a new "problem" arised.

As you see down there in the code, I try to compare the database password with the one written in the password field. If they are the same, I want it to grant access. If not, then I do not want it to grant access.

Basically I'm simulating this with the two echos.
But it does not work, it always give back access no matter what I type in the box.

Also, I would like to place this in the server code instead (just trying it from the client). Is it possible to send such information between the client and server?

Say, only information that would not be critical, that is.

Any information is helpful!

%login.ValidateSettings();
	
	%login.Connect();
    %userid = ($pref::Username);
    %userpw = ($pref::Password);

	%login.Query ("SELECT * FROM users");

	%result= %login.StoreResult();

	for (%i= 0; %i< %login.NumRows (%result); %i++)
	{
		%login.FetchRow (%result);

		%password= %login.GetRowCell (%result, %userid);
	}
 
    if (%userpw == %password)
       echo("Access Granted");
    else
        echo("Access Denied");
        
	%login.FreeResult(%result);
	%login.Close();

}

#10 You should simplify your logic.

first of your sql should be more like what john said
and with the complexity of the password check as well
(INNER JOIN?)
I am Far from a sql expert.
but I am confident this should be done with the query

then login.Query(..) can run the query.
then you can have:

%result = %login.Query("my Query");

 if(%result)
 {

 }

Edit:
but your code above.. is the %password variable getting scoped?
you should declare it above with the others if you wish to use it beyond the loop as the code shows.

As well the for loop looks wierd.

// just what is happening here? %login.NumRows (%result); 
// I would assume that it is getting the rowcount from the result

for (%i= 0; %i< %login.NumRows (%result); %i++)   
{
   %login.FetchRow (%result);  // not passing iterator?
  // this function iterates itself? .. 
  // how does it know which row i want?

   %password= %login.GetRowCell (%result, %userid);   
  // doesn't password get scoped here? 
}  
// is this the same password var? does this script language not scope properly?
if (%userpw == %password)
   echo("Access Granted");
else 
   echo("Access Denied");

%login.FreeResult(%result); 
%login.Close();
}

#11 I was not clear enough, sorry. The IF statement is where I have problems, it always returns the same, which in this case is the ELSE statement, it returns "echo("Access Denied"); no matter what I type in the field.

That's why I think that's the problem and not the rest of the code, as it gets the values correctly from the DB and shows them, but can't compare to the already existing one.

Come on :) Someone? It's just a simple IF statement.

#12

[b]if (%userpw == %password)[/b]
       echo("Access Granted");
else        
       echo("Access Denied");

Again, this is where the problems are.. I'm not a programmer.

#13 You need the string comparison operator..

$=

not ==

#14 %login.Query ("SELECT * FROM users where userid = '" @ %userid @ "' and userpw = '" @ %userpw @ "'");

%result = %login.StoreResult();
if (%login.NumRows (%result) > 0)
echo("Access Granted");
else
echo("Access Denied");

#15 Matthew's way is the better way to do it.

#16 Although I feel I need to point out something VERY important regarding database security...

Never ever take a users input for granted and patch it directly into a query statement. If you do you will end up with users who "hijack" the query to get whatever they want.

#17 Hehe

I *just* watched my boss do that with a search form in our accounting app. He was able to hijack the query and get the usernames and passwords for everyone in the database.

The method I posted is a low security option. Sending an unencrypted username and password across the wire is also never a good idea (especially all of them ;). However, this will get you started =)

#18 Doh someones in trouble :)

The "technical" term for it is in "SQL Injection". Basically it allows someone to inject an arbitrary query into it.

For this reason it is important to properly validate and format variables BEFORE putting them into the query.

Here are a couple examples in PHP:

Bad code:

// $userID is a numeric value
$query = "SELECT * FROM users WHERE userid=$userID";

Good Code:

// $userID is a numeric value
$userID = intval($userID) // force $userID to be numeric
$query = "SELECT * FROM users WHERE userid=$userID";

Bad Code:

// $userName is a string value - these are the really bad ones
$query = "SELECT * FROM users WHERE username='$username'";

In the above, many people think putting the single quote "wraps" the string and protects it. It doesn't.
$userName = "' injection code here --"
Will unwrap it and inject arbitrary code. Validate and format:

Good Code:

// $userName is a string value - these are the really bad ones
$userName = str_replace("'", "''", $userName);
$query = "SELECT * FROM users WHERE username='$username'";

In the above, if a user tries to unwrap:
$userName = "' inject here --";
$query = "SELECT * FROM users WHERE username=' ' inject here --";
It turns into:
$userName = "'' inject here --";
$query = "SELECT * FROM users WHERE username=' '' inject here --";

#19 Thanks for the input everyone

This script will be placed on a server.

Honestly though, what made it work now was Phil's suggestion of putting a $ string comparision operator infront of the = mark.
Matthew's suggestion looks very difficult to me, as I don't understand what he was trying to do :) And if works the way I think, it looks innesecary to do the comparision that way.

REALLY great thanks to all you guys though, I've got a great insight of how this works now ;)

Edit;Vanderback: Lol, seems like you posted right before I was done. Are you telling this from a client perspective? I mean, if the client never sees any password or username, they can't spoof it either way right?

At least, that's what I thought.

/Stefan Lundmark

#20 And booh on me, sorry for my bad english at times. It's just not my native language. :/