11
\$\begingroup\$

This is my first windows batch file. The script basically goes through this basic flow of tasks, in order to update some specific SQL Server DBs to a new DB version:

  1. Backup all SQL server DBs that will be upgraded.
  2. Upgrade to the latest version and apply any last-minute patches to those DBs.
  3. Error handling is done when the process fails at any point, rolling back all DBs to their initial version (before the update).

I would appreciate your feedback on:

  • What parts of the code could be improved in general (fix some bad practices, improve brittle code, etc).
  • How to better organize (comments, separation of sub-sections, etc) the code and make it clearer (for easy maintenance).
  • Any other aspect you think needs to be improved, specially regarding batch coding style and standarization.

Here is the full code:

:: MAIN SCRIPT OPTIONS
@echo off
setlocal EnableDelayedExpansion

:: ------------------------------------------------------------------------------------------------
:: PARAMETERS (ADJUST TO SPECIFIC ENVIRONMENT)

:: SQL Server: instance holding the DBs
set serverName=SERVER\SQLEXPRESS
:: SQL Server: user name and user password for performing the upgrade
set sqlUser=%1
set sqlPwd=%2
:: SQL Server: comma separated list of DBs in this server instance that should be upgraded
set sqlDBs=DB1,DB2

:: DB version: current version to be upgraded
set currentVersion=4.3.0
:: DB version: target version to upgrade to
set targetVersion=4.4.0
:: DB version: scripts (if multiple, quote and separate with commas) to run before applying the upgrade (leave blank if none is needed)
set prePatchScripts=
:: DB version: scripts (if multiple, quote and separate with commas) to run after applying the upgrade (leave blank if none is needed)
set postPatchScripts="Script1.sql,Script2.sql"

:: ------------------------------------------------------------------------------------------------
:: CALCULATED VARIABLES (DO NOT MODIFY)

:: Folders: scripts working directory
set workingDir=%~dp0
:: Folders: path to store the DB backups
set backupDir=%workingDir%Backups\
:: Folders: path where upgrade and patch scripts will be found
set scriptsDir=%workingDir%Scripts\

:: File names: name of the upgrade script to be applied
set updateScript=Update_SCM_%currentVersion%_a_%targetVersion%.sql
:: File names: name of the log file where messages will be written
set logFile=log.txt

:: ------------------------------------------------------------------------------------------------
:: BACK UP EVERY DB IN THE SERVER
call :echo_header "Backing up all DBs in the server"
for /d %%a in (%sqlDBs%) do (
    echo Backing up database: %%a to %backupDir%%%a.bak
    sqlcmd -b -S !serverName! -U !sqlUser! -P !sqlPwd! -Q "backup database [%%a] to Disk='!backupDir!%%a.bak' with init"
    echo.
) 

:: ------------------------------------------------------------------------------------------------
:: UPDATE AND PATCH EACH DB IN THE SERVER
call :echo_header "Starting DB update process"
for /d %%a in (%sqlDBs%) do (
    if not [!prePatchScripts!]==[] call :run_script Pre-patching %%a !prePatchScripts!
    if not errorlevel 1 call :run_script Updating %%a !updateScript!
    if not errorlevel 1 if not [!postPatchScripts!]==[] call :run_script Post-patching %%a !postPatchScripts!
    if errorlevel 1 goto :eof
    echo.   
) 

:: ------------------------------------------------------------------------------------------------
:: END THE PROCESS SUCCESSFULLY
call :echo_header "SUCCESS: Update to %targetVersion% was successful!"
goto :eof

:: ------------------------------------------------------------------------------------------------
:: FUNCTION TO EXECUTE ONE SCRIPT ON ONE SPECIFIC DB
:: %1 the action to be performed on the DB (Updating, Pre-patching or Post-patching)
:: %2 the name of the database to execute the script on
:: %3 the name of the script(s) to be executed
:run_script
for /d %%b in (%~3) do (
    echo %1 database %2 with script %%b
    sqlcmd -b -S !serverName! -U !sqlUser! -P !sqlPwd! -d %2 -i !scriptsDir!%%b -o %logFile%
    if errorlevel 1 (
        echo Script failed on DB %2. 
        echo.
        call :restore 
        exit /b 1
    )
    echo Script executed successfully.
) 
exit /b 0

:: ------------------------------------------------------------------------------------------------
:: FUNCTION TO DISPLAY UPDATE STEP MESSAGES
:: %1 the text to be displayed as a header
:echo_header
echo ##########################################################################################
echo %~1 
echo ##########################################################################################
echo.
goto :eof

:: ------------------------------------------------------------------------------------------------
:: PERFORM DB RESTORE PROCESS
:restore
call :echo_header "Starting DB restore process"
:: Restore DB backups
for /d %%a in (%sqlDBs%) do (
    echo Restoring database: %%a from %backupDir%%%a.bak
    sqlcmd -b -S !serverName! -U !sqlUser! -P !sqlPwd! -Q "alter database [%%a] set single_user with rollback immediate"
    sqlcmd -b -S !serverName! -U !sqlUser! -P !sqlPwd! -Q "restore database [%%a] from Disk='!backupDir!%%a.bak' with replace"
    echo.
) 
call :echo_header "ERROR: Update to %targetVersion% failed. See log for details."
\$\endgroup\$
4
+200
\$\begingroup\$

Function style

  • Indent the function's commands so the reader can recognize easily which command belong to the function.

  • Add localization of environment variables. This can be done by issuing a SETLOCAL command at the beginning of the function and ENDLOCAL at its end. Otherwise, all the variables inside the function will be global and therefore visible and modifiable by other scripts. While this can be sometimes convenient - It is a source for nasty bugs.

  • Name the positional parameters at the beginning of the function. First, It will make your code much clearer. Second, when you need to add parameters or reorder them , You will not need to change all the code, only the set commands at the beginning of the function.

For Example:

:echo_header
    ::
    :: Function used to display section titles inside the batch process. 
    ::
    :: %1 the text to be displayed as a header
    ::
    :: @Return errorlevel 0 always. 
    ::
    setlocal
    set header=%~1

    echo.
    echo /////////////////////////////////////////////////////////////
    echo %header%
    echo /////////////////////////////////////////////////////////////
    echo.

    endlocal
    exit /b 0

You should use function whenever possible. The code inside the loop can be refactored as a function as it do a certain task - restore the database from a backup file. First, it will allow you to test and debug your code easily. Second, It is much easier to reuse this code later.

for /f "delims=" %%a in ('dir %backupsDir% /b') do (
    call :restore_db [%%a] [%backupsDir%] [%serverName%] [%sqlUser%] [%sqlPwd%]
)    
...    
:restore_db
   setlocal
   set file=%1
   set backupsDir=%2
   set serverName=%3
   set sqlUser=%4

   :: Use above variables to restore the db 
   ...
   endlocal
   exit /b 0
\$\endgroup\$
5
\$\begingroup\$

Well, in case anyone is interested, here is what I've come up with:

BASICS

I like using the :: type of comments over REM comments, because I think they are easier to read (less clutter).

Also, I use lower case text for everything inside the file (except for section headers, as you will see below). I understand that many people prefer to use upper case text for function names, labels, etc., but I think lower case text is easier to read, so that is what I am adopting for my files.

COMMENTS

Taking this into consideration, I use a header on each file like this:

::
:: SCRIPTNAME.BAT
:: 
:: Author: Author's Name (email@domain.com)
:: vx.y - Month dd, yyyy
::
:: Summary of what the script is supposed to do, special remarks,
:: acknowledgments, and other important general notes. 
::

Then I divide my file in sections, delimited by separators like these:

:: --------------------------------------------------------------
:: FILE PARAMETERS (FOR ADJUSTING SCRIPT TO A NEW ENVIRONMENT)
:: --------------------------------------------------------------

Some sections are optional - more details regarding the sections I use below in the organization part.

Blocks of code inside each section can be commented with something like this, in order to create a clear separation of the different parts of code, and also to provide useful comments for business logic, batch file techniques used to "make something work", etc:

:: 
:: Perform some specific sub-task (block title)
:: 
:: This is a comment to explain what will happen in the next code block.
:: You can document here important aspects of the lines that follow. 
::

Finally, functions are documented in a similar way than blocks of code, but I decided to include parameter and return value definitions (inspired to some degree in Javadoc comments). Here is an example (take notice of the tag at the top of the comment):

::
:function_name
::
:: Function used to perform some task on some elements (function brief description). 
::
:: (Optional) comments can be added if you need to provide further details about the function.
:: These comments can span multiple lines. 
::
:: %1 this is a description of what the first parameter is.
:: %2 this is a description of what the second parameter is.
::
:: @Return errorlevel 0 if the action is valid, errorlevel 2 if error A
:: happens, and return errorlevel 1 otherwise. 
::

FILE ORGANIZATION

The basic organization of my files was thought in order to put everything that makes sense together, in the most 'findable' way, trying also to follow the basic flow of my scripts. In this regard, the main sections that my scripts have now are:

  • Main Script Options: here I set @echo off, setlocal EnableDelayedExpansion, etc. Every option that needs to be turned on for the whole script.
  • File Parameters: these are at the top of the file for easy modification, as they are the parameters that are 'hard coded' in the script, but can be changed to adjust the script to a new environment. Examples are set serverName=MYSERVER\MYINSTANCE or set workingDir=D:\Path\To\Files. These aren't supposed to change during each execution of the script.
  • Prompt Parameters: this section is for defining parameters that are set by prompting the user for specific input. For example, asking the user for a user name and/or password, or any other parameter that you don't want to store directly in the script (because it's sensible, or it changes every time the script is executed). Examples can be set /p user=Enter user name: or set /p sqlDBs=Comma separated list of DBs:.
  • Calculated Variables: this section holds the definition of variables that will be used in several places during the script execution, so it's better (imho) to have them centralized. These variables' values are derived from the values of the (file or prompt) parameters, usually using simple calculations/transformations. Examples would be: set scriptsDir=%workingDir%Scripts\ or set userFolder=%workingDir%\users\%user%. Although these examples only show folder "calculations", it can be extended to calculated patterns, dates, file names, etc. i.e. everything that's derived from the parameters, but not set directly. You get the idea.
  • Script Parameter Validation & Cleaning: all parameter validation happens here, before "doing any damage". If any validation fails, the whole script halts and a (hopefully helpful) message error is presented to the user. Any parameter cleaning is also done here (for instance, removing strange/unexpected characters).
  • Main Script Flow: these is where the main logic of the script is defined. Sub-sections of the script's flow can be commented using the convention shown above. You should be able to read the main logic of your script by reading this section.
  • Script Functions: this is where "reusable" code is located, properly documented with the style explained above. Code you need to use more than once, or code that you can use in several batch files is included here. This helps hiding the low-level logic of some operations from the 'Main Script Flow'.

With all that said, this is how one of my scripts look after all these conventions are applied (I used a simpler script than the one shown above to focus on the conventions and not the script itself):

::
:: DBRESTORE.BAT
:: 
:: Author: Carlos Sierra
:: v1.0 - August 24, 2016
::
:: This batch file goes through all the backup files in the Backups 
:: subfolder, and restores them. 
::

:: --------------------------------------------------------------
:: MAIN SCRIPT OPTIONS
:: --------------------------------------------------------------

@echo off
setlocal EnableDelayedExpansion

:: --------------------------------------------------------------
:: FILE PARAMETERS (FOR USER TO ADJUST WHEN SETTING UP A NEW ENVIRONMENT)
:: --------------------------------------------------------------

:: SQL Server: instance holding the DBs
set serverName=MYSERVER\SQLEXPRESS

:: --------------------------------------------------------------
:: PROMPT PARAMETERS (FOR USER TO ADJUST DURING SCRIPT EXECUTION)
:: --------------------------------------------------------------

:: SQL Server: user name and user password for performing the update
set /p sqlUser=Enter the SQL server user for executing the update: 
set /p sqlPwd=Enter the SQL server password for user %sqlUser%:

:: --------------------------------------------------------------
:: CALCULATED VARIABLES (DO NOT MODIFY)
:: --------------------------------------------------------------

:: Folders: scripts working directory
set workingDir=%~dp0
set backupsDir=%workingDir%Backups\

:: --------------------------------------------------------------
:: SCRIPT PARAMETER VALIDATION & CLEANING
:: --------------------------------------------------------------

:: No validations required in this script. 

:: --------------------------------------------------------------
:: MAIN SCRIPT FLOW
:: --------------------------------------------------------------

:: 
:: Restore every db stored in the Backups folder
::
call :echo_header "Starting DB restore process"
for /f "delims=" %%a in ('dir %backupsDir% /b') do (
    set file=%%a
    set ext=!file:~-3!
    if !ext!==bak (
        set name=!file:~0,-4!
        echo Restoring database !name! from %backupsDir%!file!
        sqlcmd -b -S !serverName! -U !sqlUser! -P !sqlPwd! -Q "alter database [!name!] set single_user with rollback immediate"
        sqlcmd -b -S !serverName! -U !sqlUser! -P !sqlPwd! -Q "restore database [!name!] from Disk='%backupsDir%!file!' with replace"
        if errorlevel 1 goto :show_restore_error
        echo.   
    )
) 

::
:: End the process successfully
::
call :echo_header "SUCCESS: backups restored succesfully for DBs %sqlDBs%!"
goto :eof

:: --------------------------------------------------------------
:: SCRIPT FUNCTIONS
:: --------------------------------------------------------------

::
:show_restore_error
::
:: Function used to display a prominent error message to the user. 
::
:: @Return errorlevel 0 always. 
::
call :echo_header "ERROR: restoration of DBs failed. See console for details."
exit /b 0

::
:echo_header
::
:: Function used to display section titles inside the batch process. 
::
:: %1 the text to be displayed as a header
::
:: @Return errorlevel 0 always. 
::
echo.
echo //////////////////////////////////////////////////////////////////////////////////////////
echo %~1 
echo //////////////////////////////////////////////////////////////////////////////////////////
echo.
exit /b 0

I hope you find it useful.

All ideas and contributions are welcome!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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