1

I am using asp.net 4 and visual studio 2010 with ms-sql server 2008 express. I have used three different ways to insert data into a database table:

Way 1:

    DataSet dsTab = new DataSet("Table1");
    SqlDataAdapter adp = new SqlDataAdapter("Select * from Table1", con);
    adp.Fill(dsTab, "Table1");

    DataRow dr = dsTab.Tables["Table1"].NewRow();
    dr["col1"] = txtBox1.Text;
    dr["col2"] = txtBox5.Text;
    dr["col3"] = User.Identity.Name.ToString();
    dr["col4"] = "text";
    dr["col5"] = DateTime.Now;
    dr["col6"] = txtBox3.Text;
    dr["col7"] = txtBox2.Text;
    dsTab.Tables["Table1"].Rows.Add(dr);

    SqlCommandBuilder projectBuilder = new SqlCommandBuilder(adp);
    DataSet newSet = dsTab.GetChanges(DataRowState.Added);
    adp.Update(newSet, "Table1");

Way 2:

SqlDataAdapter AdapterMessage = new SqlDataAdapter();
AdapterMessage.InsertCommand = new SqlCommand();
AdapterMessage.InsertCommand.Connection = con;
AdapterMessage.InsertCommand.CommandText = "insert into Table1(col1,col2,col3,col4,col5,col6,col7) values ('" + txtBox1.Text + "','" + txtBox5.Text + "','" + User.Identity.Name.ToString(); + "','text','" + DateTime.Now + "','" + txtBox3.Text + "','" + txtBox2.Text + "')";
AdapterMessage.InsertCommand.ExecuteNonQuery();
AdapterMessage.Dispose();

Way 3:

string query = "insert into Table1(col1,col2,col3,col4,col5,col6,col7) values ('" + txtBox1.Text + "','" + txtBox5.Text + "','" + User.Identity.Name.ToString(); + "','text','" + DateTime.Now + "','" + txtBox3.Text + "','" + txtBox2.Text + "')";
int i;

SqlCommand cmd = new SqlCommand(query);
con.open();
i = cmd.ExecuteNonQuery();
con.close();

Which among the three is the most optimized way for usage in a website??

Abe Miessler
  • 75,910
  • 89
  • 276
  • 451
Jayesh
  • 1,328
  • 1
  • 13
  • 34
  • depends on the amount of data (single or bulk) You should also look at stored procedures as this would be my personal choice – ChrisBint Jun 28 '11 at 20:48
  • None of those three would be good - #2 and #3 are especially bad. You should check out **parametrized queries** instead of concatenating together your SQL statements - that's a bad practice and opens the doors to SQL injection attacks. – marc_s Jun 28 '11 at 20:51
  • Check the folowing: http://stackoverflow.com/questions/2149897/performance-of-bcp-bulk-insert-vs-table-valued-parameters In short, take a look at SQL Server Bulk Insert. Its amazingly fast if you have alot of data – Polity Jun 28 '11 at 20:52

6 Answers6

3

The only example you have shown that is NOT vulnerable to a SQL Injection attack is #1.

Since the other two are vulnerable, #1 is your only option (of the three you've presented). Go back through your code and fix any examples of #2 or #3 immediately. If you don't your site will get hacked.

Have you seen performance problems between the various ways you have tried? If not you would be significantly better off spending your time securing your website rather than over-engineering your code.

Take a look at the SqlParameter class. This is the proper way to submit a parametrized query to SQL Server and eliminates the possibility of SQL Injection attacks when used properly.

That said if this were my code I would not use any of those options. Here is the pseudocode for what seems like the best way to me:

using (SqlConnection connection = new SqlConnection(
           connectionString))
{
    SqlCommand command = new SqlCommand(queryString, connection);

    //add parameters here

    command.Connection.Open();
    command.ExecuteNonQuery();
}
Andriy M
  • 71,352
  • 17
  • 87
  • 142
Abe Miessler
  • 75,910
  • 89
  • 276
  • 451
  • I have used parameters many times. Parameter can be used with both DataAdapter as well as SqlCommand object. So are you suggesting to use SqlCommand over DataAdapter, alongwith parameters?? – Jayesh Jun 28 '11 at 21:08
  • If you are just inserting one record then yes. Are you inserting more than one? – Abe Miessler Jun 28 '11 at 21:20
  • You should probably also put the `SqlCommand` into a `using(...) { ... }` block ... – marc_s Jun 29 '11 at 04:39
  • @Abe Generally its a single record at a time. In some cases its more than one. @marc_s What benefits do i get for using "using" block? – Jayesh Jun 29 '11 at 06:01
  • @Abe If its more than 1 record then data adapter is preferable? Data Adapter manages the opening and closing of db connection, avoiding concurrency. So isn't it more preferable here too? – Jayesh Jun 30 '11 at 07:32
3

If you are inserting several rows you can use the SqlBulkCopy object. A brief example:

DataTable dt = new DataTable();
SqlConnection oConn = new SqlConnection(); 
SqlBulkCopy sbc = new SqlBulkCopy(oConn);
sbc.DestinationTableName = "dbo.SomeTable";
sbc.WriteToServer(dt);
sbc.Close();

Keep in mind the schema of your DataTable must be that of the database table you are inserting to.

FlyingStreudel
  • 4,164
  • 4
  • 27
  • 54
1

Way #1: Datasets are not objects, my only complaint.

Way #2: SQL Injection nightmare!!! SQL Injection

Way #3: Still vunerable to SQL Injection

How about extending #3:

  • Use SqlParameters to avoid SQL Injection problems.

  • Don't pass values from a form directly into a query until they've been validated.

Also Name your variables with relevant names, I did it in the example below but my names are terrible because I don't know the details of the data you're trying to insert.

public static void InsertRecordBasedOnView(String goodNameForTextBoxOne,  String goodNameForTextBoxTwo, String goodNameForTextBoxFive, String goodNameForTextBoxThree, String userName)
{
    const string query = "insert into Table1(col1,col2,col3,col4,col5,col6,col7) values (@valueOne, @valueTwo, @valueThree, @valueFour, @valueFive, @valueSix, @valueSeven)";

    SqlParameter columnOne = new SqlParameter("@valueOne", SqlDbType.NVarChar);
    SqlParameter columnTwo = new SqlParameter("@valueTwo", SqlDbType.NVarChar);
    SqlParameter columnThree = new SqlParameter("@valueThree", SqlDbType.NVarChar);
    SqlParameter columnFour = new SqlParameter("@valueFour", SqlDbType.NVarChar);
    SqlParameter columnFive = new SqlParameter("@valueFive", SqlDbType.DateTime);
    SqlParameter columnSix = new SqlParameter("@valueSix", SqlDbType.NVarChar);
    SqlParameter columnSeven = new SqlParameter("@valueSeven", SqlDbType.NVarChar);

    columnOne.Value = goodNameForTextBoxOne;
    columnTwo.Value = goodNameForTextBoxFive;
    columnThree.Value = userName;
    columnFour.Value = "Text";
    columnFive.Value = DateTime.Now;
    columnSix.Value = goodNameForTextBoxThree;
    columnSeven.Value = goodNameForTextBoxTwo;

    SqlCommand cmd = new SqlCommand(query);
    cmd.Parameters.Add(columnOne);
    cmd.Parameters.Add(columnTwo);
    cmd.Parameters.Add(columnThree);
    cmd.Parameters.Add(columnFour);
    cmd.Parameters.Add(columnFive);
    cmd.Parameters.Add(columnSix);
    cmd.Parameters.Add(columnSeven);

    using (SqlConnection connection = new SqlConnection(someConnectionString))
    {
        connection.Open();
        cmd.ExecuteNonQuery();
    }
}
N. Warfield
  • 236
  • 2
  • 14
0

I think that u can use a view in database!, or maybe EntityFramework like this

Entity;

       using (var context = new ADO_NET_Entities())
            {

                var newDataset = new stuff();

                stuff.row1InDatatable = "Something";
                stuff.row2InDatable = "Something";
                stuff.row3InInDatatable = "Something";

                context.stuff.AddObject(newDataset);
                context.SaveChanges();
            }

more here ; http://msdn.microsoft.com/en-us/library/aa697427(v=vs.80).aspx

0

Depends on the amount of data (single or bulk), but I would say that SqlCommand would probably have the least overhead. The other 2 examples create an additional DataTable or DataAdapter which is not required for a basic insert.

I would also look at stored procedures (using SqlCommand) as this would be my personal choice

ChrisBint
  • 12,532
  • 5
  • 34
  • 56
0

SQL Server 2008 supports multi row inserts, like this:

INSERT INTO MyTable (FirstCol, SecondCol)
VALUES ('First',1),
('Second',2),
('Third',3),
('Fourth',4),
('Fifth',5)

which probably removes some overhead.

jishi
  • 22,747
  • 6
  • 45
  • 73