Feeling insecure about my code

Here at WTF Code, they give examples of good and bad code.  I want to know how my code is.  This code was written to run daily to ETL the CPU information fact table in a data warehouse.  It’s not “professional” code and was written to be functional before anything else since I’m the only one who’ll be maintaining it.

This code is a CLI PHP script.  It does work and has worked fine for two days now.

#!/usr/bin/php
<?php

 $server = "";
 $schema = "00CLICKSTR";
 $user   = "";
 $passwd = "";

 $dbcnx = mysql_connect($server, $user, $passwd)
     or die (mysql_error());
 $dbcxt = mysql_select_db($schema) or die (mysql_error());

 // find the appropriae date keys for this row
 $dbcxt_oltp = mysql_select_db("CLICKSTREAM_OLTP")
     or die (mysql_error());
 $sql = "SELECT `TIMESTAMP`, 1MINLOAD, 5MINLOAD, 15MINLOAD, SWAP,
     FREE, BUFFER, CACHE, SWAPIN, SWAPOUT, IOIN, IOOUT, USER, SYS,
     IDLE, IOWAIT, INFOCON FROM CPU_MEMORY";

 mysql_query("LOCK TABLES CLICKSTREAM_OLTP.CPU_MEMORY READ,
      CLICKSTREAM_OLTP.CPU_MEMORY C1 WRITE, LOCK TABLES 01CPUFACT
      WRITE, 02LCLDTEDM READ, 02LCLTMEDM READ, 02GMTDTEDM READ,
      02GMTTMEDM READ, mysql.time_zone_name READ, mysql.time_zone
      READ, mysql.time_zone_transition_type READ,
      mysql.time_zone_transition READ");
 $dbqry = mysql_query($sql) or die (mysql_error());

 #echo "Rows fetched:  " . mysql_num_rows($dbqry) . "\n";

 $dbcxt = mysql_select_db($schema) or die (mysql_error());

 while ($dbres = mysql_fetch_assoc($dbqry) or die (mysql_error()))
 {
 mysql_select_db("00CLICKSTR") or die (mysql_error());
 $sql = "SELECT id02LCLDTEDM FROM 02LCLDTEDM WHERE 03SQLDATE =
      DATE('" . $dbres['TIMESTAMP'] . "') LIMIT 1";
 $dbqry2       = mysql_query($sql) or die (mysql_error());
 $row          = mysql_fetch_assoc($dbqry2) or die (mysql_error());
 $localdatekey = $row['id02LCLDTEDM'];

 $sql = "SELECT id02LCLTMEDM FROM 02LCLTMEDM WHERE 03HOURS24 =
     HOUR('" . $dbres['TIMESTAMP'] . "') AND 03MINUTES =
     MINUTE('" . $dbres['TIMESTAMP'] . "') AND 03SECONDS =
     SECOND('" . $dbres['TIMESTAMP'] . "') LIMIT 1";
 #echo $sql . "\n";
 $dbqry2       = mysql_query($sql) or die (mysql_error());
 $row          = mysql_fetch_assoc($dbqry2) or die (mysql_error());
 $localtimekey = $row['id02LCLTMEDM'];

 // determine current local time zone
 $year = date('Y', strtotime($dbres['TIMESTAMP']));

 switch ($year) {
 case 2009:
 case 2015:
 case 2020:
 $dst_start = strtotime('March 8');
 $dst_end   = strtotime('November 1');
 break;

 case 2008:
 case 2014:
 case 2025:
 $dst_start = strtotime('March 9');
 $dst_end   = strtotime('November 2');
 break;

 case 2013:
 case 2019:
 case 2024:
 $dst_start = strtotime('March 10');
 $dst_end   = strtotime('November 3');
 break;

 case 2007:
 case 2012:
 case 2018:
 $dst_start = strtotime('March 11');
 $dst_end   = strtotime('November 4');
 break;

 case 2017:
 case 2023:
 $dst_start = strtotime('March 12');
 $dst_end   = strtotime('November 5');
 break;

 case 2011:
 case 2016:
 case 2022:
 $dst_start = strtotime('March 13');
 $dst_end   = strtotime('November 6');
 break;

 case 2010:
 case 2021:
 $dst_start = strtotime('March 14');
 $dst_end   = strtotime('November 7');
 break;

 }

 if (strtotime(date('F j', strtotime($dbres['TIMESTAMP']))) >
      strtotime(date('F j', strtotime($dst_start))) &&
      strtotime(date('F j', strtotime($dbres['TIMESTAMP'])))
      < strtotime(date('F j', strtotime($dst_end)))) {
 $current_local_timezone = 'CDT';
 }

 else {
 $current_local_timezone = 'CST';
 }

 // since we now know the timezone of our local time key, we can
 // convert the timestamp to GMT so we can look up the GMT key

 $gmt_timestamp_sql = "SELECT CONVERT_TZ('" . $dbres['TIMESTAMP'] .
      "', '$current_local_timezone', 'GMT')";
 $ts_qry        = mysql_query($gmt_timestamp_sql)
      or die (mysql_error());
 $row           = mysql_fetch_array($ts_qry) or die (mysql_error());
 $gmt_timestamp = $row[0];

 // do what we did above, but this time to the GMT date table
 $sql = "SELECT id02GMTDTEDM FROM 02GMTDTEDM WHERE 03SQLDATE =
      DATE('" . $dbres['TIMESTAMP'] . "') LIMIT 1";
 $dbqry2      = mysql_query($sql) or die (mysql_error());
 $row         = mysql_fetch_assoc($dbqry2) or die (mysql_error());
 $gmt_datekey = $row['id02GMTDTEDM'];

 $sql = "SELECT id02GMTTMEDM FROM 02GMTTMEDM WHERE 03HOURS24 =
     HOUR('" . $dbres['TIMESTAMP'] . "') AND 03MINUTES =
     MINUTE('" . $dbres['TIMESTAMP'] . "') AND 03SECONDS =
     SECOND('" . $dbres['TIMESTAMP'] . "') LIMIT 1";
 $dbqry2     = mysql_query($sql) or die (mysql_error());
 $row         = mysql_fetch_assoc($dbqry2) or die (mysql_error());
 $gmt_timekey = $row['id02GMTTMEDM'];

 // build the insert query
 $sql = "INSERT INTO 01CPUFACT(04GMTDTKEY, 04GMTTMKEY, 04LCLDTKEY,
      04LCLTMKEY, 051MINLOAD, 055MINLOAD, 0515MINLOAD, 05SWAP,
      05FREE, 05BUFFER, 05CACHE, 05SWAPIN, 05SWAPOUT, 05IOIN,
      05IOOUT, 05USER, 05SYS, 05IDLE, 05IOWAIT, 04INFOCON) VALUES
      ($gmt_datekey, $gmt_timekey, $localdatekey, $localtimekey, "
      . $dbres['1MINLOAD'] . ", " . $dbres['5MINLOAD'] . ", " .
      $dbres['15MINLOAD'] . ", " . $dbres['SWAP'] . ", " .
      $dbres['FREE'] . ", " . $dbres['BUFFER'] . ", " .
      $dbres['CACHE'] . ", " . $dbres['SWAPIN'] . ", " .
      $dbres['SWAPOUT'] . ", " . $dbres['IOIN'] . ", " .
      $dbres['IOOUT'] . ", " . $dbres['USER'] . ", " .
      $dbres['SYS'] . ", " . $dbres['IDLE'] . ", " .
      $dbres['IOWAIT'] . ", '" . $dbres['INFOCON'] . "')";

 #echo $sql . "\n";

 // execute the insert query
 mysql_query($sql) or die (mysql_error());

 // build the delete query
 $sql = "DELETE FROM CPU_MEMORY WHERE TIMESTAMP = '" .
      $dbres['TIMESTAMP'] . "' LIMIT 1";

 // execute the delete query
 mysql_select_db('CLICKSTREAM_OLTP') or die (mysql_error());
 mysql_query($sql) or die (mysql_error());

 }

 // don't forget to unlock the tables
 mysql_query("UNLOCK TABLES");

 // The problem is that we can't truncate the table while it's locked
 // and once we unlock it, the pending transactions will be executed
 // and we'll have new rows that won't have been inserted into the
 // fact table yet.  If we truncate now, we'll lose those rows.

 // Fixed with an explicit LOCK CPU_MEMORY AS C1 WRITE.

 // Don't call an explicit flush - flush happens automatically after
 // inserts waiting on the LOCK have been processed.

UPDATE:  I’ve added linebreaks so WordPress doesn’t render the code unreadable.  WordPress has already killed my indentation.

For proof that this code actually populates the table properly, here’s a sampling of columns from the CPU fact table.  These are the first ten rows inserted for yesterday’s date:

mysql> SELECT 04LCLDTKEY, 04LCLTMKEY, 0515MINLOAD, 05FREE, 05IOIN, 05IDLE, 05IOWAIT, 04INFOCON
 FROM 01CPUFACT
 ORDER BY 04LCLDTKEY DESC
 LIMIT 10;
+------------+------------+-------------+--------+--------+--------+----------+-----------+
| 04LCLDTKEY | 04LCLTMKEY | 0515MINLOAD | 05FREE | 05IOIN | 05IDLE | 05IOWAIT | 04INFOCON |
+------------+------------+-------------+--------+--------+--------+----------+-----------+
|      14189 |      86343 |        0.03 |  41168 |    555 |     85 |        7 | green     |
|      14189 |      86282 |        0.04 |  40672 |    555 |     85 |        7 | green     |
|      14189 |      86223 |        0.05 |  41168 |    555 |     85 |        7 | green     |
|      14189 |      86162 |        0.05 |  45756 |    555 |     85 |        7 | green     |
|      14189 |      86102 |        0.06 |  42532 |    555 |     85 |        7 | green     |
|      14189 |      86043 |        0.06 |  42160 |    555 |     85 |        7 | green     |
|      14189 |      85983 |        0.07 |  42780 |    555 |     85 |        7 | green     |
|      14189 |      85922 |        0.06 |  45872 |    555 |     85 |        7 | green     |
|      14189 |      85863 |        0.04 |  43640 |    555 |     85 |        7 | green     |
|      14189 |      85804 |        0.04 |  31732 |    555 |     85 |        7 | green     |
+------------+------------+-------------+--------+--------+--------+----------+-----------+
10 rows in set (0.00 sec)

Advertisements
  1. #1 by Joshua on November 6, 2009 - 9:14 AM

    It queried so fast ’cause my ORDER BY clause used an indexed column – 04LCLDTKEY. Okay, I’m lying – my server isn’t THAT good. It took 0.07 sec the first time – this is the third time executing the same query so it’s in the query cache. That’s why it took 0.00 sec to fetch.

  2. #2 by Joshua on November 8, 2009 - 10:55 AM

    I have good reason to feel insecure about this code. It’s in PHP. PHP as a CLI scripting language is teh suck. Perl is where it’s at.

    To that end, I’ve begun rewriting this in perl.

  1. #!/usr/bin/perl « Around Teh Table

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: