12

I am trying to enter data into my database, but it is giving me the following error:

Invalid column name

Here's my code

string connectionString = "Persist Security Info=False;User ID=sa;Password=123;Initial Catalog=AddressBook;Server=Bilal-PC";

using (SqlConnection connection = new SqlConnection(connectionString))
{
  SqlCommand cmd = new SqlCommand();

  cmd.CommandText = "INSERT INTO Data (Name,PhoneNo,Address) VALUES (" + txtName.Text + "," + txtPhone.Text + "," + txtAddress.Text + ");";
  cmd.CommandType = CommandType.Text;
  cmd.Connection = connection;

  connection.Open();
  cmd.ExecuteNonQuery();
}
tom redfern
  • 30,562
  • 14
  • 91
  • 126
Snake
  • 337
  • 2
  • 7
  • 23
  • 12
    Your code is open to [SQL Injection](http://en.wikipedia.org/wiki/SQL_injection). – Oded Dec 05 '11 at 18:12
  • 3
    Name is a reserved word in some databases you may need to put it in [] or escape it based on driver being used allowed values. Everyone else mentioned the SQL injection so I won't bother :D – xQbert Dec 05 '11 at 18:20
  • Can someone explain why this is **_"open to SQL Injection"_** ? Because i also use similar code. I am new to Sql and C# – Munawir Feb 26 '16 at 17:25
  • @Munawir -- look at the accepted answer for the proper way to issue this query via `SqlCommand` WITHOUT exposing yourself to SQL Injection attacks. Briefly, using the parameters collection of `SqlCommand` object to specify the variable values for the query provides this protection. When you simply concatenate the text values from the UI into your query string, that is when you are vulnerable. Hope that helps. – David Tansey Mar 16 '16 at 21:00

11 Answers11

31

You probably need quotes around those string fields, but, you should be using parameterized queries!

cmd.CommandText = "INSERT INTO Data ([Name],PhoneNo,Address) VALUES (@name, @phone, @address)";
cmd.CommandType = CommandType.Text;
cmd.Parameters.AddWithValue("@name", txtName.Text);
cmd.Parameters.AddWithValue("@phone", txtPhone.Text);
cmd.Parameters.AddWithValue("@address", txtAddress.Text);
cmd.Connection = connection;

Incidentally, your original query could have been fixed like this (note the single quotes):

"VALUES ('" + txtName.Text + "','" + txtPhone.Text + "','" + txtAddress.Text + "');";

but this would have made it vulnerable to SQL Injection attacks since a user could type in

'; drop table users; -- 

into one of your textboxes. Or, more mundanely, poor Daniel O'Reilly would break your query every time.

Adam Rackis
  • 82,527
  • 56
  • 270
  • 393
  • Parameterized is unquestionably better overall, but it won't be more performant than the original unparameterized query, since SQL Server will auto-parameterize and cache the original. See: http://stackoverflow.com/a/2734158/14606 – MusiGenesis Dec 05 '11 at 18:39
  • @MusiGenesis - I didn't know SQL Server auto-parameterized the query. That's really cool. Updating my answer. – Adam Rackis Dec 05 '11 at 18:41
  • Yeah, SQL Server does quite a lot under the hood now to make ad hoc queries efficient and fast, such that the old CW about stored procs being faster is no longer true (since SQL Server 2005 at least). I think MS just gave up on trying to get developers to always use procs. – MusiGenesis Dec 05 '11 at 18:44
  • Here's the white paper on auto-parameterization for anyone interested. The list of disqualifications is long, but OP's original simple query would still be auto-parameterized by my reading http://msdn.microsoft.com/en-us/library/ee343986.aspx thanks to @MusiGenesis for pointing it out – Adam Rackis Dec 05 '11 at 18:46
  • @MusiGenesis: While this is possible with SQL Server 2008, I don't think this is true with a default installation, I noticed quite a bit of query plan pollution before converting some queries into fully parametrized queries - anything I am overlooking? – BrokenGlass Dec 05 '11 at 19:52
  • @BrokenGlass: many (most?) queries *can't* be auto-parameterized at all (like anything other than a simple one-table INSERT, SELECT or UPDATE), so that's probably what you're seeing. When I tested the performance of ad hoc queries on an installation of SQL Server 2005, I saw the same speed as stored procs, but obviously this was only for queries that could actually be auto-parameterized. – MusiGenesis Dec 05 '11 at 20:06
  • @MusiGenesis: After reading your linked answer more closely that makes sense now - thank you! – BrokenGlass Dec 05 '11 at 20:17
27

Always try to use parametrized sql query to keep safe from malicious occurrence, so you could rearrange you code as below:

Also make sure that your table has column name matches to Name, PhoneNo ,Address.

using (SqlConnection connection = new SqlConnection(connectionString))
{
    SqlCommand cmd = new SqlCommand("INSERT INTO Data (Name, PhoneNo, Address) VALUES (@Name, @PhoneNo, @Address)");
    cmd.CommandType = CommandType.Text;
    cmd.Connection = connection;
    cmd.Parameters.AddWithValue("@Name", txtName.Text);
    cmd.Parameters.AddWithValue("@PhoneNo", txtPhone.Text);
    cmd.Parameters.AddWithValue("@Address", txtAddress.Text);
    connection.Open();
    cmd.ExecuteNonQuery();
}
Elias Hossain
  • 4,410
  • 1
  • 19
  • 33
  • SqlConnection automatically opens the connection, this makes the `connection.Open();` unnecessary. – Trontor Nov 26 '13 at 11:14
  • 1
    Trontor, I get an error: "ExecuteNonQuery requires an open and available Connection. The connections's current state is closed." So yes, the connection.Open(); is necessary. – olf Mar 21 '14 at 13:52
  • if (connection.State != ConnectionState.Open) { connection.Open(); } // You can use this conditional connection open - so that error will not occur when another connection is already open, thanks.... – Elias Hossain Mar 22 '14 at 05:45
6

Change this line:

cmd.CommandText = "INSERT INTO Data (Name,PhoneNo,Address) VALUES (" + txtName.Text + "," + txtPhone.Text + "," + txtAddress.Text + ");";

to this:

cmd.CommandText = "INSERT INTO Data (Name,PhoneNo,Address) VALUES ('" + txtName.Text + "','" + txtPhone.Text + "','" + txtAddress.Text + "');";

Your insert command is expecting text, and you need single quotes (') between the actual value so SQL can understand it as text.

EDIT: For those of you who aren't happy with this answer, I would like to point out that there is an issue with this code in regards to SQL Injection. When I answered this question I only considered the question in point which was the missing single-quote on his code and I pointed out how to fix it. A much better answer has been posted by Adam (and I voted for it), where he explains the issues with injection and shows a way to prevent. Now relax and be happy guys.

MilkyWayJoe
  • 9,082
  • 2
  • 38
  • 53
  • 5
    -1 - The [SQL Injection](http://en.wikipedia.org/wiki/SQL_injection) is still present. – Oded Dec 05 '11 at 18:14
  • 4
    You should mention the problem with the code. However, +1 for valid code. @Oded: Clearly the OP is a beginner and it is good to explain why the current code doesn't work instead of just yelling "SQL Injection!" One step at a time. – Toast Dec 05 '11 at 18:22
  • 1
    @Toast - The link I posted in _every_ place I mentioned SQL Injection describes the issue quite well. Perhaps you missed it? – Oded Dec 05 '11 at 18:23
  • @Oded You misunderstood what I meant. The link you place is helpful to learn what SQL injection is and how to prevent it. I am in no way criticizing that. It just doesn't show you why this doesn't function. – Toast Dec 05 '11 at 18:28
5

You problem is that your string are unquoted. Which mean that they are interpreted by your database engine as a column name.

You need to create parameters in order to pass your value to the query.

 cmd.CommandText = "INSERT INTO Data (Name, PhoneNo, Address) VALUES (@Name, @PhoneNo, @Address);";
 cmd.Parameters.AddWithValue("@Name", txtName.Text);
 cmd.Parameters.AddWithValue("@PhoneNo", txtPhone.Text);
 cmd.Parameters.AddWithValue("@Address", txtAddress.Text);
Pierre-Alain Vigeant
  • 22,635
  • 8
  • 65
  • 101
4

You should never write code that concatenates SQL and parameters as string - this opens up your code to SQL injection which is a really serious security problem.

Use bind params - for a nice howto see here...

Yahia
  • 69,653
  • 9
  • 115
  • 144
2

Code To insert Data in Access Db using c#

Code:-

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Data.SqlClient;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace access_db_csharp
{
public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }
   public SqlConnection con = new SqlConnection(@"Place Your connection string");
            
           private void Savebutton_Click(object sender, EventArgs e)
    {
         SqlCommand cmd = new SqlCommand("insert into  Data (Name,PhoneNo,Address) values(@parameter1,@parameter2,@parameter3)",con);
                cmd.Parameters.AddWithValue("@parameter1", (textBox1.Text));
                cmd.Parameters.AddWithValue("@parameter2", textBox2.Text);
                cmd.Parameters.AddWithValue("@parameter3", (textBox4.Text));
                cmd.ExecuteNonQuery();

                }

    private void Form1_Load(object sender, EventArgs e)
    {
        con.ConnectionString = connectionstring;
        con.Open();
    }
}
}
Heemanshu Bhalla
  • 3,603
  • 1
  • 27
  • 53
2
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
using System.Data.SqlClient;
using System.Data;

namespace WpfApplication1
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
        }

        private void btnAdd_Click(object sender, RoutedEventArgs e)
        {
            SqlConnection conn = new SqlConnection(@"Data Source=WKS09\SQLEXPRESS;Initial Catalog = StudentManagementSystem;Integrated Security=True");
            SqlCommand insert = new SqlCommand("insert into dbo.StudentRegistration(ID, Name,Age,DateOfBirth,Email,Comment) values(@ID, @Name,@Age,@DateOfBirth,@mail,@comment)", conn);
            insert.Parameters.AddWithValue("@ID", textBox1.Text);
            insert.Parameters.AddWithValue("@Name", textBox2.Text);
            insert.Parameters.AddWithValue("@Age", textBox3.Text);
            insert.Parameters.AddWithValue("@DateOfBirth", textBox4.Text);
            insert.Parameters.AddWithValue("@mail", textBox5.Text);
            insert.Parameters.AddWithValue("@comment", textBox6.Text);

            if (textBox1.Text == string.Empty)
            {
                MessageBox.Show("ID Cannot be Null");
                return;
            }
            else if (textBox2.Text == string.Empty)
            {
                MessageBox.Show("Name Cannot be Null");
                return;
            }


            try
            {
                conn.Open();
                insert.ExecuteNonQuery();
                MessageBox.Show("Register done !");
            }
            catch (Exception ex)
            {
                MessageBox.Show("Error" + ex.Message);
                conn.Close();
            }
        }

        private void btnRetrive_Click(object sender, RoutedEventArgs e)
        {
            bool temp = false;
            SqlConnection con = new SqlConnection("server=WKS09\\SQLEXPRESS;database=StudentManagementSystem;Trusted_Connection=True");
            con.Open();
            SqlCommand cmd = new SqlCommand("select * from dbo.StudentRegistration where ID = '" + textBox1.Text.Trim() + "'", con);
            SqlDataReader dr = cmd.ExecuteReader();
            while (dr.Read())
            {
                textBox2.Text = dr.GetString(1);
                textBox3.Text = dr.GetInt32(2).ToString(); 
                textBox4.Text = dr.GetDateTime(3).ToString();
                textBox5.Text = dr.GetString(4);
                textBox6.Text = dr.GetString(5);
                temp = true;
            }
            if (temp == false)
                MessageBox.Show("not found");
            con.Close();
        }

        private void btnClear_Click(object sender, RoutedEventArgs e)
        {
            SqlConnection connection = new SqlConnection("Data Source=WKS09\\SQLEXPRESS;Initial Catalog = StudentManagementSystem;Integrated Security=True");
            string sqlStatement = "DELETE FROM dbo.StudentRegistration WHERE ID = @ID";
            try
            {
                connection.Open();
                SqlCommand cmd = new SqlCommand(sqlStatement, connection);
                cmd.Parameters.AddWithValue("@ID", textBox1.Text);
                cmd.CommandType = CommandType.Text;
                cmd.ExecuteNonQuery();
                MessageBox.Show("Done");
            }
            finally
            {
                Clear();
                connection.Close();
            }
        }

        public void Clear()
        {
            textBox1.Text = "";
            textBox2.Text = "";
            textBox3.Text = "";
            textBox4.Text = "";
        }
    }
}
1

You have to use '"+texbox1.Text+"','"+texbox2.Text+"','"+texbox3.Text+"'

Instead of "+texbox1.Text+","+texbox2.Text+","+texbox3.Text+"

Notice the extra single quotes.

Dima Tisnek
  • 11,241
  • 4
  • 68
  • 120
0

Your issue seems to be the Name keyword. Rather use FullName or firstName and lastName, always try and remember to use CamelCase too.

  • Kindly post answer if you are giving any additional value add or new idea than other answers. – Raju Apr 11 '16 at 13:39
-1

first create database name "School" than create table "students" with following columns 1. id 2. name 3. address

now open visual studio and create connection:

namespace school
{
    public partial class Form1 : Form
    {
        SqlConnection scon;


        public Form1()
        {

            InitializeComponent();

            scon = new SqlConnection("Data Source = ABC-PC; trusted_connection = yes; Database = school; connection timeout = 30");
        }

//create command

SqlCommand scom = new SqlCommand("insert into students (id,name,address) values(@id,@name,@address)", scon);

//pass parameters

scom.Parameters.Add("id", SqlDbType.Int);
scom.Parameters["id"].Value = textBox1.Text;

           scom.Parameters.Add("name", SqlDbType.VarChar);
            scom.Parameters["name"].Value = this.textBox2.Text;

            scom.Parameters.Add("address", SqlDbType.VarChar);
            scom.Parameters["address"].Value = this.textBox6.Text;


            scon.Open();
            scom.ExecuteNonQuery();
            scon.Close();
            reset();

        }

also check solution here: http://solutions.musanitech.com/?p=6

Hani Mehdi
  • 187
  • 4
  • 9
-1
con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=C:\Users\Yna Maningding-Dula\Documents\Visual Studio 2010\Projects\LuxuryHotel\LuxuryHotel\ClientsRecords.mdf;Integrated Security=True;User Instance=True");
        con.Open();
        cmd = new SqlCommand("INSERT INTO ClientData ([Last Name], [First Name], [Middle Name], Address, [Email Address], [Contact Number], Nationality, [Arrival Date], [Check-out Date], [Room Type], [Daily Rate], [No of Guests], [No of Rooms]) VALUES (@[Last Name], @[First Name], @[Middle Name], @Address, @[Email Address], @[Contact Number], @Nationality, @[Arrival Date], @[Check-out Date], @[Room Type], @[Daily Rate], @[No of Guests], @[No of Rooms]", con);
        cmd.Parameters.Add("@[Last Name]", txtLName.Text);
        cmd.Parameters.Add("@[First Name]", txtFName.Text);
        cmd.Parameters.Add("@[Middle Name]", txtMName.Text);
        cmd.Parameters.Add("@Address", txtAdd.Text);
        cmd.Parameters.Add("@[Email Address]", txtEmail.Text);
        cmd.Parameters.Add("@[Contact Number]", txtNumber.Text);
        cmd.Parameters.Add("@Nationality", txtNational.Text);
        cmd.Parameters.Add("@[Arrival Date]", txtArrive.Text);
        cmd.Parameters.Add("@[Check-out Date]", txtOut.Text);
        cmd.Parameters.Add("@[Room Type]", txtType.Text);
        cmd.Parameters.Add("@[Daily Rate]", txtRate.Text);
        cmd.Parameters.Add("@[No of Guests]", txtGuest.Text);
        cmd.Parameters.Add("@[No of Rooms]", txtRoom.Text);
        cmd.ExecuteNonQuery();
Yna
  • 1