0

I have the following sql statement written in PHP.

$sql='INSERT INTO pictures (`picture`) VALUES ("'.$imgs[$i]['name'].'",)';
$db->query($sql);
$imgs[$i]['sqlID']      = $this->id=mysql_insert_id();
$imgs[$i]['newImgName'] = $imgs[$i]['sqlID'].'_'.$imgs[$i]['name'];
$sql='UPDATE pictures SET picture="'.$imgs[$i]['newImgName'].'" WHERE id='.$imgs[$i]['sqlID'];
$db->query($sql);

Now this writes the image name to database table pictures. After that is done I get the mysql_insert_id() and than I'll update the picture name with the last id in front of the name with underscore.

I'll do this to make sure no picture name can be the same. Cause all those pictures get saved in the same folder. Is there another way to save that ID already the first time I set the sql query? Or are there other better ways to achieve this result?

Thanks for all advices

caramba
  • 21,963
  • 19
  • 86
  • 127
  • 5
    [Please, stop using mysql_* functions](http://stackoverflow.com/q/12859942/1238019) in new code, they are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Instead of, have a look on [prepared statements](http://dev.mysql.com/doc/refman/5.0/en/sql-syntax-prepared-statements.html), and use [Mysqli](http://php.net/manual/en/book.mysqli.php) or [PDO](http://php.net/manual/en/book.pdo.php). – zessx Nov 18 '13 at 15:58
  • No. This is the best way to get the id. `select max(id)+1` is utterly unreliable and subject to race conditions - you WILL get collisions. – Marc B Nov 18 '13 at 15:59
  • 2
    The real question is "do you really need to copy the primary key of a table in another field of that same table". – Vatev Nov 18 '13 at 16:02
  • @Vatev yes I have to do that. So the real question is: "Is it possible to do that in another way?" – caramba Nov 18 '13 at 16:05
  • @caramba Using the native auto_increment - there is no other way. You need to do it in the 3 steps you described. – Vatev Nov 18 '13 at 16:07
  • My advice is to transfer all of your sql to a single stored procedure that does everything you need doing to the database. – Dan Bracuk Nov 18 '13 at 16:09
  • 1
    @DanBracuk: pointless, since OP's using the record's ID as part of the filename of the on-disk file preresenting the image he's recording. – Marc B Nov 18 '13 at 16:13
  • @vatev thanks for pointing at all those things. If you would like to use your comments as an answer I would appreciate and mark it as the right answer. – caramba Nov 18 '13 at 16:24

2 Answers2

1

I don't think in this particular case it's reasonable to use MySQL insert Id in the file name. It might be required in some cases but you provided no information why it would be in this one.

Consider something like:

while( file_exists( $folder . $filename ) {
  $filename = rand(1,999) . '_' . $filename;
}

$imgs[$i]['newImgName'] = $filename;

Of course you can use a larger range for rand or a loop counter if you wanted tot systematically increment the number used to prefix the original file name.

marekful
  • 14,986
  • 6
  • 37
  • 59
  • "Cause all those pictures get saved in the same folder." that's why. so its part of the filename at the end, and the filenames have to be unique. – caramba Nov 18 '13 at 16:08
  • The code in the answer does just that - leaves you with unique file names w/o using inser Id. – marekful Nov 18 '13 at 16:10
  • If you're going to implement this for real use `mt_rand` instead of `rand` - it might save you some headaches. – Vatev Nov 18 '13 at 16:12
1

Using the native auto_increment - there is no other way. You need to do the 3 steps you described.

As Dan Bracuk mentioned, you can create a stored proc to do the 3 queries (you can still get the insert id after executing it).

Other possible options are:

  • not storing the id in the filename - you can concatenate it later if you want (when selecting)
  • using an ad-hoc auto increment instead of the native one - I would not recommend it in this case, but it's possible
  • using some sort of UUID instead of auto increment
  • generating unique file names using the file system (Marcell Fülöp's answer)
Vatev
  • 7,493
  • 1
  • 32
  • 39