5
\$\begingroup\$

I'm using Microsoft SQL Server 2014 Express.

I found myself wanting to add the Columns CompanyNameChangeDate (1 through 10) and CompanyNameChangePreviousName (1 through 10) to a table in my database.

I wrote the following query to do it:

declare @i int
set @i = 0

Declare @nameChangeDatePrefix as varchar(255) = 'CompanyNameChangeDate'

Declare @nameChangePreviousNamePrefix as varchar(255) = 'CompanyNameChangePreviousName'

declare @sql as varchar(255)

while (@i < 10)
begin
    set @i = @i + 1

    set @sql = 'ALTER TABLE company_registry_data ADD ' + @nameChangeDatePrefix + Cast(@i as varchar(255)) + ' DATE'
    execute(@sql)

    set @sql = 'ALTER TABLE company_registry_data ADD ' + @nameChangePreviousNamePrefix + Cast(@i as varchar(255)) + ' VARCHAR(255)'
    execute(@sql)
end
  1. How could I make it better?
  2. What naming conventions have I contravened?
  3. What habits should I start building right now?
  4. What are the code smells?
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

I would consider your whole proposal for this schema to be a code smell. In SQL, if there are many data to be treated alike, they should be stored in one column, rather than in multiple columns. If you store your data using this proposed schema, any code that tries to use the schema is likely to be nasty too. (What's the average time interval between name changes? Good luck writing that query!) You would also have an arbitrary limit of 10 name changes per company, and accommodating more names would necessitate another schema change!

Therefore, it would make more sense to have a CompanyNameChange table, with (companyId, changeDate) as the primary key, and additional attributes previousName as an additional attribute. If, for some reason, you do need the data to be presented in that proposed tabular format, you could always perform some joins.

\$\endgroup\$
3
\$\begingroup\$

Using dynamic queries to alter the database schema is typically a pretty bad idea.

If you are changing "real" tables, it would be better to pull the alter statement in to a completely separate session and run it from there. Sure, use the SQL query to build the alter statement, but then copy/paste the alter statement in to a new session. That way you get the chance to inspect the alter before running it.

Additionally, it would be much better to have a concatenating process inside your loop, than to actually run the dynamic statements.

Consider something like:

set @sql = 'ALTER TABLE company_registry_data ADD '

while (@i < 10)
begin
    if @i > 0
        set @sql = @sql + ', '

    set @i = @i + 1

    set @sql = @sql + @nameChangeDatePrefix + Cast(@i as varchar(255)) + ' DATE, '
            + @nameChangePreviousNamePrefix + Cast(@i as varchar(255)) + ' VARCHAR(255)'
end

Now you have a single SQL alter statement to run, instead of 20. This will have less impact and reorganization requirement on the table, and will likely run significantly faster, especially if the table is populated.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.