Docs
AI Docs
DPR
Analysis of Stock Comparison Report Upload/Download Optimization (Commit `e1c41f0`)

Analysis of Stock Comparison Report Upload/Download Optimization (Commit `e1c41f0`)

This report provides a detailed code review and analysis of the optimization changes implemented in commit e1c41f0. The optimization focused on transitioning from slow iterative operations to batch processing via memory-mapped DataTable and SQL bulk copies, query optimization (avoiding duplicate CTEs), transaction safety, and improved Web UI asynchronous and exception handling patterns.

1. Summary of Changes

Repository Layer: InventoryReportRepository.cs

  • Data Insertion Optimization: Replaced row-by-row iteration with memory-based C# DataTable and SqlBulkCopy for bulk-loading imported rows into a temporary table (#temp).
  • Query Optimization: Replaced redundant Common Table Expressions (CTEs) that compiled multiple times with a static temporary database table (#sys), dramatically reducing compilation and execution overhead.
  • SARGable Query Design: Replaced non-SARGable absolute-sum condition checks (ABS(balanceqty)+ABS(balanceqty1) > 0) with index-friendly direct comparison expressions ((balanceqty <> 0 OR balanceqty1 <> 0)).
  • Transaction Safety: Wrapped the database setup, copy, and write queries in a single database transaction (DbTransaction) to ensure atomic execution.

Web UI Layer: SubTrnImport.razor

  • Blazor Lifecycle Alignment: Changed asynchronous UI event handlers from async void to async Task to enable proper exception tracking and await behaviors within the Blazor component.
  • Error Handling: Wrapped the file import process in a robust try-catch-finally block to prevent UI state locking (infinite loading spinners) on exceptions.
  • Resource Leak Prevention: Replaced loose file stream closes with await using block statements for guaranteed disposal of local file handles.

2. Line-by-Line Code Block Analysis

A. Repository Changes (InventoryReportRepository.cs)

1. SQL Query Restructuring

Old Code: 
var inserttempquery = @"INSERT INTO #temp (trnid, subtrnid) VALUES (@trnid, @subtrnid)";

// List of System trnId which balqty is not 0 & shortclose is not yet done 
var trnIdList = @"WITH A(trnid, subtrnid,balanceqty,balanceqty1) AS
(
    SELECT trnid,subtrnid, balanceqty, balanceqty1 FROM DprSubTrn 
    WHERE ABS(balanceqty)+ABS(balanceqty1) > 0
),
B(trnid, subtrnid, balanceqty, balanceqty1) AS
(
    SELECT trnid, subtrnid, balanceqty, balanceqty1 FROM A
    WHERE NOT EXISTS(SELECT trnid FROM DprShortClose D 
    WHERE A.trnid = D.trnid )  
)";

// Updated bal. for existing records
var updateqtyquery = $@"{trnIdList}
UPDATE DprSubTrnComparison SET qty = B.balanceqty, qty1 = B.balanceqty1 FROM B 
WHERE DprSubTrnComparison.trnid = B.trnid AND DprSubTrnComparison.subtrnid = B.subtrnid";

// Insert those records which is in System but not in comparison table
var insertSystemRecords = $@"{trnIdList},
C(trnid, subtrnid, balanceqty, balanceqty1) AS
(
    SELECT trnid, subtrnid, balanceqty, balanceqty1 FROM B
    WHERE NOT EXISTS (SELECT trnid, subtrnid FROM DprSubTrnComparison D 
    WHERE B.trnid = D.trnid AND B.subtrnid = D.subtrnid )
)
INSERT INTO DprSubTrnComparison (trnid, subtrnid, inventorytype, qty, qty1)
SELECT trnid, subtrnid, 'S' AS inventoryType, balanceqty, balanceqty1 FROM C";
New Code: 
var createsystemtemp = @"SELECT trnid, subtrnid, balanceqty, balanceqty1
INTO #sys
FROM DprSubTrn
WHERE (balanceqty <> 0 OR balanceqty1 <> 0)
AND NOT EXISTS (SELECT 1 FROM DprShortClose D WHERE D.trnid = DprSubTrn.trnid)";

var updateqtyquery = @"UPDATE DprSubTrnComparison SET qty = S.balanceqty, qty1 = S.balanceqty1
FROM #sys S
WHERE DprSubTrnComparison.trnid = S.trnid
AND DprSubTrnComparison.subtrnid = S.subtrnid";

var insertSystemRecords = @"INSERT INTO DprSubTrnComparison (trnid, subtrnid, inventorytype, qty, qty1)
SELECT S.trnid, S.subtrnid, 'S' AS inventoryType, S.balanceqty, S.balanceqty1
FROM #sys S
WHERE NOT EXISTS (
    SELECT 1
    FROM DprSubTrnComparison D
    WHERE D.trnid = S.trnid
    AND D.subtrnid = S.subtrnid
)";
Explanation & Rationale:
  • What was changed: The CTE query definitions compiled into two different variables have been refactored into a single #sys temp table query creation query (createsystemtemp). The update and insert queries now perform simple joins/checks directly against #sys. Additionally, the index-blocking ABS(balanceqty)+ABS(balanceqty1) > 0 condition was replaced by (balanceqty <> 0 OR balanceqty1 <> 0).
  • Why it was needed:
    • Submitting repeated CTEs in different SQL queries forces the database to perform costly table/index scans twice on the original database tables (DprSubTrn and DprShortClose).
    • Mathematical functions in CTE filters like ABS(...) prevent SQL Server from utilizing indexes on quantity columns (violating SARGability), leading to full table scans.
  • Improvement: Reduces database load, utilizes database indexes on DprSubTrn and DprShortClose columns, simplifies overall SQL logic, and optimizes performance.

2. Data Insertion and Transaction Flow

Old Code: 
using var connection = _DapperContext.SetClientConnection(dbname);
connection.Open();
connection.Execute(createtemp);
foreach (var dataLine in listdata)
{
    connection.Execute(inserttempquery, new
    {
        trnid = dataLine.QrDescription[..7],
        subtrnid = dataLine.QrDescription.Substring(8, 3)
    });
}
connection.Execute(finalinsertquery);
connection.Execute(updateqtyquery);
connection.Execute(insertSystemRecords);
New Code: 
using var connection = (SqlConnection)_DapperContext.SetClientConnection(dbname);
connection.Open();
using var transaction = connection.BeginTransaction();

connection.Execute(createtemp, transaction: transaction);

var tempTable = new System.Data.DataTable();
tempTable.Columns.Add("trnid", typeof(string));
tempTable.Columns.Add("subtrnid", typeof(string));

foreach (var dataLine in listdata)
{
    tempTable.Rows.Add(dataLine.QrDescription[..7], dataLine.QrDescription.Substring(8, 3));
}

using (var bulkCopy = new SqlBulkCopy(connection, SqlBulkCopyOptions.Default, transaction))
{
    bulkCopy.DestinationTableName = "#temp";
    bulkCopy.ColumnMappings.Add("trnid", "trnid");
    bulkCopy.ColumnMappings.Add("subtrnid", "subtrnid");
    bulkCopy.WriteToServer(tempTable);
}

connection.Execute(createsystemtemp, transaction: transaction);
connection.Execute(finalinsertquery, transaction: transaction);
connection.Execute(updateqtyquery, transaction: transaction);
connection.Execute(insertSystemRecords, transaction: transaction);
transaction.Commit();
Explanation & Rationale:
  • What was changed:
    • A cast to SqlConnection was added.
    • A database transaction is explicitly initialized.
    • An in-memory C# System.Data.DataTable object (tempTable) is created and populated with rows from listdata.
    • SqlBulkCopy is configured to stream this DataTable to #temp in a single bulk operation.
    • Queries are executed using the active transaction context.
  • Why it was needed:
    • Row-by-row iteration resulted in one database connection round-trip per row in the imported list, causing huge latencies during upload operations.
    • Executing state-changing updates and inserts across multiple queries without an explicit transaction risked database inconsistencies if an error occurred midway.
  • Improvement:
    • Performance: High optimization (from $\mathcal{O}(N)$ database queries down to $\mathcal{O}(1)$ query calls for data loading).
    • Data Integrity: Atomicity ensures that the entire batch either succeeds completely or rolls back to the initial state on failure.

B. Web Page Changes (SubTrnImport.razor)

1. Async Void to Task

Old Code: 
private async void SubTrnImportPage()
public async void SubTrnComparisonXL()
public async void ImportTypeOnChange(string importtype)
public async void DownloadModel()
New Code: 
private async Task SubTrnImportPage()
public async Task SubTrnComparisonXL()
public async Task ImportTypeOnChange(string importtype)
public async Task DownloadModel()
Explanation & Rationale:
  • What was changed: Method signatures were changed from returning void to returning Task.
  • Why it was needed: In Blazor and ASP.NET Core, async void triggers run in a detached state. The framework cannot await their completion, meaning UI rendering cycles can trigger before the operation completes, and any exceptions thrown inside them will crash the Blazor circuit or application pool.
  • Improvement: Ensures exceptions are properly propagated, caught, and handled. Allows Blazor’s renderer to await asynchronous processes correctly before updating the UI state.

2. Try-Catch-Finally Exception Handling & Safe Streams

Old Code: 
isProcessing = true;
StateHasChanged();

Stream stream = uploadedfile.OpenReadStream();
string pathdirname2 = _IFilePathService.GetReportsPath(_PostLogin.dbname);
var pathfilename2 = _IFilePathService.GetFullPath(pathdirname2);
if (!Directory.Exists(pathfilename2))
{
    Directory.CreateDirectory(pathfilename2);
}

Random rnd = new();
var randomnumber = rnd.Next(1000000000).ToString().Trim();
var newxlfilename = $"{randomnumber}_{uploadedfile.Name}";
var savefilepath = Path.Combine(pathfilename2, newxlfilename);

FileStream fs = File.Create(savefilepath);
await stream.CopyToAsync(fs);
stream.Close();
fs.Close();

var xlfiledata = "";
var userfilename = "";

if(_ImportSubTrn.ImportType == "A") 
{
    xlfiledata = await _IInventoryReportService.ShortCloseImport(_PostLogin.dbname, newxlfilename);
    userfilename  = "ShortCloseImportXL.xlsx";
}
else
{
    xlfiledata = await _IInventoryReportService.SubTrnComparisonImport(_PostLogin.dbname, newxlfilename);
    userfilename  = "StockComparisonImportXL.xlsx";
}
 
xlfiledata = xlfiledata.Replace("\"", "");

isProcessing = false;
StateHasChanged();

if (xlfiledata == "1")
{
    _ISnackbar.Add($"Please upload correct model.", Severity.Error);
}
...
New Code: 
isProcessing = true;
StateHasChanged();

try
{
    Stream stream = uploadedfile.OpenReadStream();
    string pathdirname2 = _IFilePathService.GetReportsPath(_PostLogin.dbname);
    var pathfilename2 = _IFilePathService.GetFullPath(pathdirname2);
    if (!Directory.Exists(pathfilename2))
    {
        Directory.CreateDirectory(pathfilename2);
    }

    Random rnd = new();
    var randomnumber = rnd.Next(1000000000).ToString().Trim();
    var newxlfilename = $"{randomnumber}_{uploadedfile.Name}";
    var savefilepath = Path.Combine(pathfilename2, newxlfilename);

    await using (FileStream fs = File.Create(savefilepath))
    {
        await stream.CopyToAsync(fs);
        stream.Close();
    }

    var xlfiledata = "";
    var userfilename = "";

    if(_ImportSubTrn.ImportType == "A") 
    {
        xlfiledata = await _IInventoryReportService.ShortCloseImport(_PostLogin.dbname, newxlfilename);
        userfilename  = "ShortCloseImportXL.xlsx";
    }
    else
    {
        xlfiledata = await _IInventoryReportService.SubTrnComparisonImport(_PostLogin.dbname, newxlfilename);
        userfilename  = "StockComparisonImportXL.xlsx";
    }
     
    xlfiledata = xlfiledata.Replace("\"", "");

    if (xlfiledata == "1")
    {
        _ISnackbar.Add($"Please upload correct model.", Severity.Error);
    }
    ...
}
catch (HttpRequestException ex)
{
    _ISnackbar.Add($"Import failed: {ex.Message}", Severity.Error);
}
catch (Exception ex)
{
    _ISnackbar.Add($"Import failed: {ex.Message}", Severity.Error);
}
finally
{
    isProcessing = false;
    StateHasChanged();
}
Explanation & Rationale:
  • What was changed:
    • The business logic steps inside SubTrnImportPage were wrapped in a try block.
    • The state resetting (isProcessing = false and StateHasChanged()) was moved inside a finally block to guarantee it runs.
    • User file copying was wrapped inside an await using (FileStream fs = ...) block.
    • Added HTTP request and general exception catch handlers.
  • Why it was needed:
    • Without a try-catch, failure during file IO or network calls would leave the application loading indicator stuck, preventing subsequent attempts and showing no feedback.
    • File streams that are not inside a using block can leak file descriptors if exceptions occur, causing subsequent file write errors due to file locking.
  • Improvement: Greatly improved reliability, user experience, and resource safety.

3. Data Flow Comparison 

graph TD
    %% Flow Before Change %%
    subgraph Flow Before Change
        A1[User uploads file] --> B1[Start SubTrnImportPage async void]
        B1 --> C1[Open and write FileStream without using blocks]
        C1 --> D1[Call API Service]
        D1 --> E1[Open DbConnection without Transaction]
        E1 --> F1[Iterate List: Execute INSERT for each item O-N latency]
        F1 --> G1[Execute Updates & Inserts with redundant CTE evaluation]
        G1 --> H1[Complete operation - no error fallback]
    end

    %% Flow After Change %%
    subgraph Flow After Change
        A2[User uploads file] --> B2[Start SubTrnImportPage async Task inside try]
        B2 --> C2[Write stream using safe 'await using' block]
        C2 --> D2[Call API Service inside try-catch]
        D2 --> E2[Open DbConnection, cast to SqlConnection & Begin Transaction]
        E2 --> F2[Load imported rows into memory DataTable]
        F2 --> G2[Perform SqlBulkCopy into #temp O-1 latency]
        G2 --> H2[Materialize active records into #sys temp table once]
        H2 --> I2[Execute final inserts and updates joining #sys]
        I2 --> J2[Commit Transaction]
        J2 --> K2[Finally block resets loading states and notifies UI]
    end

    style Flow Before Change fill:#ffe6e6,stroke:#ff6666,stroke-width:2px;
    style Flow After Change fill:#e6ffe6,stroke:#66cc66,stroke-width:2px;

4. Verification of Business Logic

We have verified that the business logic was kept exactly the same. No business rules have been altered:

  1. SQL Expressions Logic Comparison:
    • Old Filter: ABS(balanceqty)+ABS(balanceqty1) > 0
    • New Filter: (balanceqty <> 0 OR balanceqty1 <> 0)
    • Equivalence: Since quantities are numeric, taking the sum of absolute values to see if it is $>0$ is mathematically equivalent to verifying that at least one of the quantities is non-zero.
  2. Short-Close checks & Exclusions:
    • Both old and new implementations exclude items in DprShortClose by looking up the primary trnid key.
  3. Data transformations:
    • Both write identical data lines (QrDescription[..7] to trnid and QrDescription.Substring(8, 3) to subtrnid) into the #temp destination table.
  4. Operations performed:
    • The same tables are populated, updated, and queried in the same order (#temp $\to$ #sys $\to$ DprSubTrnComparison).
effective width divide by zero fixed