DBI update function 1

[jcarrott]

I am looping through a batch of data. I am creating a output header and detail record for each record that has a status of 112584.

After I creat the header and detail output records I want to change the status field from 112584 to 112586.

I do not want to effect the loop I am in. I think I have the code correct but I was wondering if somebody that has done this could review this and see if I have it correct.

$dbh=DBI->connect($dsn, $user, $pass) or die;

    $sth = $dbh->prepare("SELECT * FROM CVIS.LAW_INV_ITEM_TRX")
            or die "Couldn't prepare statement: " . $dbh->errstr;
    @data;
    $rv = $sth->execute or die "Couldn't execute: " . $dbh->errstr;

    open(OUT, ">>$vPath1$vName1") or die "Couldn't open: " . $dbh->errstr;

## Loop through each line of the table

    while (@data = $sth->fetchrow_array()) {

... other code ...

        if ($data[8] eq 112584) {

## map and load the header line

            $sLin2  = $vComp . $vICLoc . "40" . "IS" . $vDesc
                   .= $vSeq . $vSeq . $vCRD1 . $sNStri20 . $vICLoc
                   .= $sNStri313 . "\n";

            print OUT $sLin2;

## map and load the detail line

            $sLin3  = $vComp . $vICLoc . "40" . "IS" . $vDesc
                   .= $vLine . $vSeq . $vItem . $vQty . $vQtySign
                   .= $sNStri36 . $vSUOM . $sNStri260 . "\n";

            print OUT $sLin3;

            $sLin4 = $data[8] . "\n";

            print $sLin4;

## update the TRX_STATUS for this record to 112586 export completed

	$st2 = $dbh->prepare("UPDATE CVIS.LAW_INV_ITEM_TRX 
				SET $data[8] = 112586
				WHERE $data[10] = $vSysTrxId
	$st2->execute or die "Could't update data: " . $DBH->errstr;

        }
    }

$sth->finish;
$dbh->disconnect();

 

[feherke]

Hi

This looks really bad :

    $st2 = $dbh->prepare("UPDATE CVIS.LAW_INV_ITEM_TRX
                SET $data[8] = 112586
                WHERE $data[10] = $vSysTrxId

Syntactically :
[ul]
[li]Unclosed string.[/li]
[li]Unclosed parenthesis.[/li]
[li]Unclosed statement.[/li]
[/ul]
Logically :
[ul]
[li]$data[8] is supposed to be a value and due to the [tt]if[/tt] condition it will always be 112584. So the [tt]SET 112584 = 112586[/tt] clause will fail.[/li]
[li]Probably the same thing applies to $data[10].[/li]
[li]Prepared statements have two big benefits : by preparing once and using multiple times improves the speed; by binding values to placeholders ensures correct formatting of the values. There you just trashed both benefits.[/li]
[/ul]
Minor ugliness :
[ul]
[li]If you will have to touch that code after a couple of months you will not know what $data[8] is. By using [tt]fetchall_hashref()[/tt] instead of [tt]fetchrow_array()[/tt] you could use $data->{'field_name'} instead of $data[8].[/li]
[/ul]
Theoretically the above quoted code should look like this :

[gray]# only once, before the while loop[/gray]
    [navy]$st2[/navy] [teal]=[/teal] [navy]$dbh[/navy][teal]->[/teal][COLOR=darkgoldenrod]prepare[/color][teal]([/teal][green][i]"UPDATE CVIS.LAW_INV_ITEM_TRX
                SET field_name = 112586
                WHERE other_field = ?"[/i][/green][teal]);[/teal]

[gray]# every time its needed, inside the while loop[/gray]
    [navy]$st2[/navy][teal]->[/teal][COLOR=darkgoldenrod]bind_param[/color][teal]([/teal][purple]1[/purple][teal],[/teal] [navy]$data[/navy][teal][[/teal][purple]10[/purple][teal]]);[/teal]
    [navy]$st2[/navy][teal]->[/teal]execute or [b]die[/b] [green][i]"Could't update data: "[/i][/green] [teal].[/teal] [navy]$DBH[/navy][teal]->[/teal]errstr[teal];[/teal]

Note that I not really understood your goal, so the above code is an example, not a solution.


Feherke.

http://feherke.github.com/