Jump to content
MakeWebGames

Is this worth it?


Recommended Posts

Ok on my game that i have re programmed all the file's. I am using a new type of function called "query_m()" what it does is same as mysql but this is how it work's:

 

<?php //added them to colour code.
function query_m($SQL)
{
global $h;

$v = $SQL;

mysql_query($SQL);

if(mysql_error())
{
	mysql_query("INSERT INTO mysql_errors VALUES(NULL,'".$v."','".mysql_error()."')");
}
}
?>

 

It will only insert into mysql error's if there is a mysql error. This is much easier for me to track down bug's etc in my script. Is it worth it?

Link to comment
Share on other sites

Re: Is this worth it?

 

Ok on my game that i have re programmed all the file's. I am using a new type of function called "query_m()" what it does is same as mysql but this is how it work's:

 

<?php //added them to colour code.
function query_m($SQL)
{
global $h;

$v = $SQL;

mysql_query($SQL);

if(mysql_error())
{
	mysql_query("INSERT INTO mysql_errors VALUES(NULL,'".$v."','".mysql_error()."')");
}
}
?>

 

It will only insert into mysql error's if there is a mysql error. This is much easier for me to track down bug's etc in my script. Is it worth it?

 

To answer the question directly, yes it can be worth it.

 

To provide commentary on what you're doing:

#1 You include $h but never use it.

#2 You have the variable $SQL and $v. Why two vars for the same thing?

#3 You insert a query string into a query. Do you see where this sort of thing could be a problem since you aren't using mysql_real_escape_string() on $SQL or $v?

#4 You're logging queries whether you are accessing the page, or someone else is. Why not just display the error if you are loading the page, and log it if it's anyone else?

#5 Why insert NULL into an auto increment field? http://dev.mysql.com/doc/refman/5.0/en/ ... ement.html ---- If you take a look on that page, none of the example queries use NULL as an insert value for the auto increment. I would recommend following the examples from the MySQL site rather than deviating from them.

#6 Your mysql_query() function isn't using a link identifier. You should include $c and plug that in there.

#7 Even better, make a Database class, and do this the right way. ;)

That's about all I can think of lol.

Good luck with it!

Link to comment
Share on other sites

Re: Is this worth it?

lol, did you even read floydian's post? we know what you want to use it for, and yes, it's a good idea. floydian simply posted some feedback for you.

 

#5 Why insert NULL into an auto increment field? http://dev.mysql.com/doc/refman/5.0/en/ ... ement.html ---- If you take a look on that page, none of the example queries use NULL as an insert value for the auto increment. I would recommend following the examples from the MySQL site rather than deviating from them.

Yep, in fact, no values are inserted at all for auto increment columns. So just insert data for specific columns, in your case, the coumns that contain the query and the error.

Link to comment
Share on other sites

Guest Anonymous

Re: Is this worth it?

Only one to add to that Floydian...

Wy use auto-increment fields at all ?

They are the spawn of the devil, they break down at almost every possible opportunity, notably with replication (yes I know that is a bit advanced for the average CE nugget, but just in case), they are non-portable between DB engines .. oo again, a bit too complex...

[me=Nyna]shushes[/me]

Link to comment
Share on other sites

Re: Is this worth it?

 

lol, did you even read floydian's post? we know what you want to use it for, and yes, it's a good idea. floydian simply posted some feedback for you.

[sNIPPED]

lol, I did feel a little *glossed* over there...

No worries though killah, you're doing good there. ;)

And Nyna, Nyna, Nyna.... need I say any more? :P

Nice add Nyna

Link to comment
Share on other sites

Re: Is this worth it?

My quick solution to auto-increment:

$ql = $db->query('SELECT `id` FROM `table` WHERE id > 0 ORDER BY `id` DESC LIMIT 1');
if($db->num_rows($ql))
{
$gql = $db->fetch_row($ql);

$gql['id'] = $gql['id']+1;

$db->query("INSERT INTO `table` VALUES('{$gql['id']}', 'blah')");
}
else
{
$db->query("INSERT INTO `table` VALUES('1', 'blah0')");
}

Would that be a more practical solution?

edited:

so instead of:

CREATE TABLE IF NOT EXISTS `table` (
 `id` int(11) NOT NULL auto_increment,
 `blah` varchar(255) NOT NULL default '',
 PRIMARY KEY  (`id`)
)

it would be:

CREATE TABLE IF NOT EXISTS `table` (
 `id` int(11) NOT NULL default '0',
 `blah` varchar(255) NOT NULL default ''
)

 

Don't use this... for obvious reasons.

Link to comment
Share on other sites

Guest Anonymous

Re: Is this worth it?

Nope.

In a multi-threaded environment - ie. the web - that will FAIL.

Look at your number of users ... you may not have that many, so you may think it's working perfectly, but once you start handling thousands or 100's of thousands queries per hour you are screwed.

Yes, in theory, your script *should* be running in a single thread however, I've seen many cases where this is just blatently not true.

The operation needs to be atomic. That is - incapable of being pre-empted by another thread at any point.

Look at this:

A) SELECT .. FROM table ... WHERE ... -> 1

B) INSERT INTO table VALUES (1+1, ...)

Now what happens under a heavy load, when multiple processes are calling this function. Each one will be running in a separate thread which means that at some stage, the flow of operations to MySQL will be:

[Thread1] A) SELECT .. FROM table ... WHERE ... -> 1

[Thread2] A) SELECT .. FROM table ... WHERE ... -> 1

[Thread1] B) INSERT INTO table VALUES (1+1...)

[Thread2] B) INSERT INTO table VALUES (1+1...) --- *** FAIL ***

This is a common problem, but simple to get around. The solution, is in a fact a one-liner, but I'd leave that open :D

Link to comment
Share on other sites

Re: Is this worth it?

As i see that there are thread hi jacker's amongst us i might aswell join lol.

To see if i am correct.

Should'nt thread2 b be:

INSERT INTO table VALUES (1+2...)

and so forth? If so could just add a easy check num row's add up to that and insert then stop. I may be wrong so don't come with genious post's!

Link to comment
Share on other sites

Guest Anonymous

Re: Is this worth it?

Yep wrong again...

[Thread1] $id = "SELECT id FROM table ..." -> $id is 1

[Thread2] $id = "SELECT id FROM table ..." -> $id is 1

[Thread1] "INSERT INTO table ($id + 1) ..." -> is 1 + 1

[Thread2] "INSERT INTO table ($id + 1) ..." -> is 1 + 1 *** FAIL ***

Link to comment
Share on other sites

Re: Is this worth it?

 

[Thread1] $id = "SELECT id FROM table ..." -> $id is 1

[Thread2] $id = "SELECT id FROM table ..." -> $id is 1

[Thread1] "INSERT INTO table ($id + 1) ..." -> is 1 + 1 => 2

[Thread2] "INSERT INTO table ($id + 2) ..." -> is 1 + 2 => 3 *** FAIL ***

How will it fail?

You have to remember, you are using one query. They're different threads, but performing the same procedures.

You used two different queries above ($id+1 and $id+2). You can't know when to use which, so you can't actually implement that.

 

Would the solution be... transactions? Or locking the table? What's the trade off in performance?

Link to comment
Share on other sites

Guest Anonymous

Re: Is this worth it?

Transactions are not needed here, nor is table locking. The solution is much simpler.

Transactions would need a transaction safe database - and frankly, they can be a bish. Several problems exist with PHP and transactions - not least the fact that there is no real safety net for them, so it's simple to kill the DB server by using poorly thought out code. And thinking about your code when you may have 1...1000000 hits per hour gets *very* confusing.

Locking can be useful, but again, pointless in this case. As with transactions, there can be problems with locks depending on how your environment is configured.

Hand crafting auto-increment systems may seem odd, but on the few occasions you need a unique id -- it's *very* handy.

Link to comment
Share on other sites

Re: Is this worth it?

 

[Thread1] $id = "SELECT id FROM table ..." -> $id is 1

[Thread2] $id = "SELECT id FROM table ..." -> $id is 1

[Thread1] "INSERT INTO table ($id + 1) ..." -> is 1 + 1 => 2

[Thread2] "INSERT INTO table ($id + 1) ..." -> is 1 + 1 => 2 *** FAIL ***

How will it fail?

Nyna is talking about dual queries...

say 2 users hit the same page at the exact same time then the id comes out at 1 then it adds one equaling 2... (with me so far?)

because they did it at the same time they will both get the id 2

then... say it holds important information... you're f'ed

 

i think that's what she's on about anyways

Link to comment
Share on other sites

Re: Is this worth it?

 

public function Query( $string )

{

$this->query = @mysql_query( "{$string}" )or die( $this->Error( 0, mysql_error( ) ) );

return $this;

}

Yes, and in the quote above, from your link, you can add the error handling there.

Instead of dieing on error, handle it gracefully.

The good thing about this is that since you have it all tucked into a class, you can do a lot more, and do it efficiently.

 

Do you combine the two queries?

INSERT INTO table ((SELECT id FROM table...)+1)...

Or does that make no difference?

Zeggy, please forget about that stuff you read. It's the wrong way to do it entirely. It's not like it's a little wrong, it's just completely wrong. No offense. Just don't want anyone to implement something that isn't going to work for them.

It is a good thought though. I've thought of doing that myself back in the day as well.

Auto increments handle themselves. Simply don't insert to the auto increment column. That's it.

insert into table (blah, foo, bar)

values ('eh', 'oh', 'ah')

 

where your table has columns:

id

blah

foo

bar

 

And the id is an auto increment. You HAVE to explicitly label the columns you're adding. I know lots of people skip that. Get into the habbit of adding them in. You have to do it, if you want to do things properly.

Link to comment
Share on other sites

Guest Anonymous

Re: Is this worth it?

 

How will it fail?

The reason I prefixed each line with a thread # is to show you *why* this method of auto-increment generation does not work.

Lets make things simple:

Assume a simple table:

MyTable
  id INT(10) UNSIGNED NOT NULL PRIMARY

 

Now lets insert some initial data...

INSERT INT MyTable (id) VALUES (0);

 

Now... we use this for our "effective" auto-incs..

$last = $db->fetchOne("SELECT id FROM MyTable ORDER BY id DESC LIMIT 1");
$last = $last + 1;
$db->execute("INSERT INTO MyTable (id) VALUES ($last)");

 

Okay, this will work to an extent, but it suffers from a number of problems.

The most notable being it will not *always* work in a heavily used multi-threaded environment:

### THREAD #1
$last = $db->fetchOne("SELECT id FROM MyTable ORDER BY id DESC LIMIT 1");
### Thread 1 sees $last as 0

### THREAD #2
$last = $db->fetchOne("SELECT id FROM MyTable ORDER BY id DESC LIMIT 1");
### Thread 2 sees $last as 0

### THREAD #1
$last = $last + 1;
### Thread 1 now has $last = 1

### THREAD #2
$last = $last + 1;
### Thread 2 now has $last = 1

### THREAD #1
$db->execute("INSERT INTO MyTable (id) VALUES ($last)");
### Thread 1 updates the table, by inserting 1 -- SUCCESS

### THREAD #2
$db->execute("INSERT INTO MyTable (id) VALUES ($last)");
### Thread 2 attempts to update the table, by inserting 1 -- FAILURE

 

And that is only looking at 2 threads... We often have to deal with several hundred thousand queries per hour.

Link to comment
Share on other sites

Re: Is this worth it?

with that mysql class i posted a link to there is a problem with a loop. Using the FetchArray it will just look forever, i tried it on a normal working loop so it is the code.

It looks to me like it runs a new query every repetition.. any ideas?

while($r = $mysql->Connect()->Query($SQL)->FetchArray($result)->Close()) {

Link to comment
Share on other sites

Re: Is this worth it?

 

Zeggy, please forget about that stuff you read. It's the wrong way to do it entirely. It's not like it's a little wrong, it's just completely wrong. No offense. Just don't want anyone to implement something that isn't going to work for them.

It is a good thought though. I've thought of doing that myself back in the day as well.

Auto increments handle themselves. Simply don't insert to the auto increment column. That's it.

insert into table (blah, foo, bar)

values ('eh', 'oh', 'ah')

 

where your table has columns:

id

blah

foo

bar

 

And the id is an auto increment. You HAVE to explicitly label the columns you're adding. I know lots of people skip that. Get into the habbit of adding them in. You have to do it, if you want to do things properly.

Umm, I know how to do an insert properly. What you wrote above is exactly what I suggested in my first post.

My previous post was responding to the problem nyna posed (about concurrency issues when using your own auto increment implementation).

Link to comment
Share on other sites

Re: Is this worth it?

 

with that mysql class i posted a link to there is a problem with a loop. Using the FetchArray it will just look forever, i tried it on a normal working loop so it is the code.

It looks to me like it runs a new query every repetition.. any ideas?

while($r = $mysql->Connect()->Query($SQL)->FetchArray($result)->Close()) {

With that code, it looks like you'd be makin a new connection to the database, doing a query, fetching the first row, closing the db connection, on every iteration.

If that's the case, it's definitely wrong.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...