CodeSOD: Finding the Right Size

Zeke sends us a C# snippet from an extract-transform-load process his company uses. It's… special.

private void ResizeColumn(string table, string column, int minSize)
{
    if(null == _connection) return;

    string sqlReadSize = "SELECT DATA_LENGTH,DATA_TYPE,DATA_PRECISION,DATA_SCALE FROM USER_TAB_COLS WHERE TABLE_NAME = '" + table.ToUpper() + "' AND COLUMN_NAME = '" + column.ToUpper() + "'";
    string data_length = "";
    string data_type = "";
    string data_precision = "";
    string data_scale = "";
    string sizeInfo = minSize.ToString();
    
    IDataReader r = null;

    try
    {
        r = _connection.DbAccessor.ExecuteSqlText.ExecuteReader(sqlReadSize);
        if(null != r && r.Read())
        {
            if(!r.IsDBNull(0)) data_length = Convert.ToString(r[0]);
            if(!r.IsDBNull(1)) data_type = Convert.ToString(r[1]);
            if(!r.IsDBNull(2)) data_precision = Convert.ToString(r[2]);
            if(!r.IsDBNull(3)) data_scale = Convert.ToString(r[3]);

            r.Close();
            r = null;

        }
    }
    catch(Exception ex)
    {
        System.Diagnostics.Debug.WriteLine(ex.Message);
        return;
    }
    finally
    {
        if(null != r)
        {
            r.Close();
            r = null;
        }
    }
    if(data_type == "NUMBER")
    {
        return;
    }

    if(data_type == "DATE")
    {
        return;
    }

    if(data_type == "CLOB")
    {
        return;
    }

    if(data_type == "BLOB")
    {
        return;
    }
    
    if(minSize <= Convert.ToInt32(data_length))
    {
        return;
    }

    string sqlAlterSize = "ALTER TABLE " + table + " modify " 
        + column.ToUpper() + " " + data_type + "(" + sizeInfo + ")";


    try
    {
        _connection.DbAccessor.ExecuteSqlText.ExecuteScalar(sqlAlterSize);
    }
    catch(Exception ex)
    {
        System.Diagnostics.Debug.WriteLine(ex.Message);
        return;
    }
}

Let's trace through this. We start by failing silently if the database connection hasn't been initialized, which is sure to make debugging a real treat. Then we prepare a string-concatenated query into the databases's data dictionary, opening up a lovely SQL injection attack (which, in their defense, I don't expect the inputs to this function to be user driven, but still).

We run the query and populate local variables with the results- local string variables. The source data in the database is numeric (this is, by the by, an Oracle database)- so we just discard that and convert it to strings.

There's a lovely bonus of a double-close- they close the reader in the try portion and the finally portion, instead of just using the finally. More fun, they also do the useless null assignment that people who don't understand garbage collection often do.

Then, we check the type of the column in question. If it's a number, a date, a CLOB or a BLOB, we do nothing. This is really fun because there are plenty of built-in datatypes that don't have a size field, but aren't included here- TIMESTAMP for example, expects fractional seconds precision, not a size. We won't change the size of a NUMBER (which takes precision and scale), but will change the precision of a FLOAT.

What they really wanted was to just change the sizes of CHAR, NCHAR, VARCHAR2, and NVARCHAR2s. But instead of saying that, they opted to try and come at it the back way around, and created a method that's almost certain to create explosions at some point.

Finally, they execute an ALTER TABLE command to do the update. Any failures are treated as a debugging issue, which again, is going to be great when you're trying to decide why this failed in production.

Not only is the method horrible, but it's also unnecessary. Once upon a time, someone recognized that they needed to hold text fields of arbitrary lengths and configured all the text columns to be 4000 bytes (the default maximum size in Oracle), which was much larger than any of the input data.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

This post originally appeared on The Daily WTF.