9
\$\begingroup\$

Below is the current code which is used to update/insert records from Oracle view to SQL Server table using Dapper. There is not a field to check last record updated date in the oracle view so I have added a method to get hashcode by using property values. Since Oracle table has more than 15k records and each record has more more than 60 columns, this approach take more than 5 minutes. Any ideas/suggestions to improve below code?

using System;
using System.Configuration;
using System.Data.OracleClient;
using System.Data.SqlClient;
using System.Linq;
using System.Reflection;
using Dapper;

namespace SyncSQLSvrWithHRDB
{
    internal class Program
    {
        public static PropertyInfo[] PropertyNames = typeof(Employee).GetProperties();

        private static void Main(string[] args)
        {
            var OracleConStr = ConfigurationManager.ConnectionStrings["OracleCon"].ConnectionString;
            var SqlSvrConStr = ConfigurationManager.ConnectionStrings["SqlSvrCon"].ConnectionString;
            using (OracleConnection OraCon = new OracleConnection(OracleConStr))
            {
                var res = OraCon.Query<Employee>(Constants.SelectSql).ToList();

                res = res.GroupBy(x => x.EmpNumber.ToUpper()).Select(x => x.LastOrDefault()).ToList();
                using (SqlConnection Sqlcon = new SqlConnection(SqlSvrConStr))
                {
                    Sqlcon.Open();
                    for (int i = 0; i < res.Count; i++)
                    {
                        var item = Sqlcon.Query<Employee>(Constants.SelectEmpSql, new { EmpNumber= res[i].EmpNumber}).FirstOrDefault();
                        if (item == null) // new record found
                        {
                            Sqlcon.Execute(Constants.InsertSql, res[i]);
                        }
                        else if (GetHashcode(res[i]) != GetHashcode(item)) // record updated
                        {
                            Sqlcon.Execute(Constants.UpdateSql, res[i]);
                        }
                    }
                }
            }
        }

        public static int GetHashcode(Employee o)
        {
            int ret = 0;
            foreach (var prop in PropertyNames)
            {
                object propValue = o.GetType().GetProperty(prop.Name).GetValue(o, null);

                if (propValue != null)
                {
                    ret += propValue.GetHashCode();
                }
            }

            return ret;
        }
    }

    public class Employee
    {
        public String EmpNumber{ get; set; }

        // ... other properties (70)
    }
}
\$\endgroup\$
3
  • 4
    \$\begingroup\$ Consider creating a linked server to Oracle from your SQL Server. You can then perform your necessary work in T-SQL, in a set-based manner. Consider using the new MERGE syntax in SQL Server too. \$\endgroup\$
    – gvee
    Sep 23, 2013 at 7:29
  • 2
    \$\begingroup\$ As an aside, you should really implement GetHashCode on Employee, both for better encapsulation and for speed. Hash code generation is supposed to be fast, and reflecting the fields like that are the opposite of fast. \$\endgroup\$
    – Dan Lyons
    Nov 15, 2013 at 18:57
  • \$\begingroup\$ Have you done any profiling? You're running a zillion queries against SQL instead of merging the two data sets, I'd guess that's your problem. \$\endgroup\$
    – user7412
    Nov 18, 2013 at 23:17

2 Answers 2

7
\$\begingroup\$

One thing I would say is that you don't need to create those connection string variables, you are only using them once in this program and you have them nicely hidden where they should be, so you can just call them when you use them, which in this case is once.

so instead of this

var OracleConStr = ConfigurationManager.ConnectionStrings["OracleCon"].ConnectionString;
var SqlSvrConStr = ConfigurationManager.ConnectionStrings["SqlSvrCon"].ConnectionString;
using (OracleConnection OraCon = new OracleConnection(OracleConStr))
{
    var res = OraCon.Query<Employee>(Constants.SelectSql).ToList();

    res = res.GroupBy(x => x.EmpNumber.ToUpper()).Select(x => x.LastOrDefault()).ToList();
    using (SqlConnection Sqlcon = new SqlConnection(SqlSvrConStr))
    {

Do this instead

using (OracleConnection OraCon = new OracleConnection(ConfigurationManager.ConnectionStrings["OracleCon"].ConnectionString))
{
    var res = OraCon.Query<Employee>(Constants.SelectSql).ToList();

    res = res.GroupBy(x => x.EmpNumber.ToUpper()).Select(x => x.LastOrDefault()).ToList();
    using (SqlConnection Sqlcon = new SqlConnection(ConfigurationManager.ConnectionStrings["SqlSvrCon"].ConnectionString))
    {

You have already named them nicely so you know exactly what they are, meaning there is no reason to create variables for these, they are only used once.


and instead of a for loop here:

using (SqlConnection Sqlcon = new SqlConnection(SqlSvrConStr))
{
    Sqlcon.Open();
    for (int i = 0; i < res.Count; i++)
    {
        var item = Sqlcon.Query<Employee>(Constants.SelectEmpSql, new { EmpNumber= res[i].EmpNumber}).FirstOrDefault();
        if (item == null) // new record found
        {
            Sqlcon.Execute(Constants.InsertSql, res[i]);
        }
        else if (GetHashcode(res[i]) != GetHashcode(item)) // record updated
        {
            Sqlcon.Execute(Constants.UpdateSql, res[i]);
        }
    }
}

you could use a foreach loop

using (SqlConnection Sqlcon = new SqlConnection(SqlSvrConStr))
{
    Sqlcon.Open();

    foreach (var record in res)
    {
        var item = Sqlcon.Query<Employee>(Constants.SelectEmpSql, new { EmpNumber= record.EmpNumber}).FirstOrDefault();
        if (item == null) // new record found
        {
            Sqlcon.Execute(Constants.InsertSql, record);
        }
        else if (GetHashcode(record) != GetHashcode(item)) // record updated
        {
            Sqlcon.Execute(Constants.UpdateSql, record);
        }
    }
}

This is a little bit cleaner and is more straight to the point about what you are doing. I would probably change some variable names around, but I will let you have that fun.

\$\endgroup\$
2
\$\begingroup\$

This code queries the SQL server 15K times (for each and every employee in the Oracle view). Suggest generating the SQL query based on the Oracle view data set and retrieve the data from SQL in one go and then do the comparison in memory. You can generate the insert and update queries in the loop and execute them outside the loop as two batches if you have Dapper Plus ;)

Otherwise, you will have to instantiate a DataTable and populate it in the loop and use SQLBulkCopy for inserts.

Additionally, GetHashCode can be optimized as per below;

public static int GetHashcode(Employee o)
{
    int hashCode = 0;
    foreach (var property in PropertyNames)
    {
        object propValue = property.GetValue(o, null);
        if (propValue != null)
        {
            hashCode += propValue.GetHashCode();
        }
     }
    return hashCode;
}
\$\endgroup\$

Your Answer

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

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