Reading data from table then insert into table

Question

Is that correct way of reading data from table then insert into table ?

I am trying to read the max number or rec_id to create seq column the +1 to insert my data record into same table

                //Opening the connection with Oracle Database
            OracleConnection conn = new OracleConnection(oradb);
            conn.Open();

            //Create the oracle statment and insert data into oracle database 
            OracleCommand cmd1 = new OracleCommand();
            cmd1.Connection = conn;

            cmd1.CommandText = "SELECT NVL (MAX (rec_id), 0) + 1 FROM backup_check";
            OracleDataReader dr = cmd1.ExecuteReader();
            dr.Read();
            label1.Text = dr.GetString(0);
            conn.Dispose();

            OracleCommand cmd = new OracleCommand();
            cmd.Connection = conn;
            //OracleTransaction trans = conn.BeginTransaction(); -- add rollback in case of transaction not complate for some reason
            cmd.CommandText = "insert into my table(" + "'" + v_rec_id + "'" + ",OFFICE_CODE,DUMP_NAME,DUMP_STATUS,SYSTEM,CHECK_DATE)values(null,null," + "'" + keyword + "'" + ",'0','abc',sysdate)";
            cmd.ExecuteNonQuery();

            //Closing the connection with oracle databae
            conn.Dispose();

No, there are many problems here. MAX cannot work reliably in a multiuser environment, creating sql text concatenating strings is A MONUMENTAL BAD practice, missing to use the using statement around disposable objects leads to resource leaks — Steve, Feb 27 '16 at 12:53
Also, that v_rec_id is the result of MAX not the name of the column,,,, — Steve, Feb 27 '16 at 12:56
yea v_rec_id for testing the result of max ... and please what is ur suggestion for enhancing the code like what should I do to avoid creating sql text concatenating strings ?... and for the using dispose I though that for security reason I am closing the connection with database after finishing the transaction ... kindly ur suggestion are really appreciated — samer, Feb 27 '16 at 13:13

Steve · Accepted Answer · 2016-02-27T13:52:43.880

1

First of all, you should forget to use the MAX function to retrieve the next value to insert in an 'so called autoincrement column'. The problem arises from the fact that your code running on one machine cannot prevent another user from another machine to execute the same code concurrently with yours. This could result in both users receiving the same MAX result and thus creating invalid duplicate keys.

In Oracle you should mark your table to have a SEQUENCE for your primary key (REC_ID)

Something like this (writing here on the fly not tested now....)

CREATE TABLE myTable 
(
    rec_id NUMBER PRIMARY KEY
    .... other columns follow
);
CREATE SEQUENCE  myTable_seq MINVALUE 1 START WITH 1 INCREMENT BY 1 CACHE 20;

CREATE OR REPLACE TRIGGER myTable_trg
BEFORE INSERT ON myTable
   for each row
   begin
      select myTable_seq.nextval into :new.rec_id from dual;
   end;

At this point, when you try to insert a new record the triggers kicks in and calculate the next value to assign to your primary key.
The C# code is simplified a lot and becomes:

string cmdText = @"insert into myTable
         (OFFICE_CODE,DUMP_NAME,DUMP_STATUS,SYSTEM,CHECK_DATE)
         values(null,:keyw,'0','abc',sysdate)";
using(OracleConnection conn = new OracleConnection(oradb))
using(OracleCommand cmd = new OracleCommand(cmdText, conn))
{
    conn.Open();
    //using(OracleTransaction trans = conn.BeginTransaction())
    //{
    cmd.Parameters.AddWithValue(":keyw",keyword);
    cmd.ExecuteNonQuery();
    // trans.Commit();
    //}
}

No need to read anything before, command text not concatenated avoid SQL Injection, closing and disposing is automatic exiting from the using block....

edited Feb 27 '16 at 13:52

answered Feb 27 '16 at 13:08

Steve

213,761
22
232
286

Thanks alot for your explanation... would you mind to tell me what the difference happen when using oracle command with using using(OracleConnection conn = new OracleConnection(oradb)) what the benefit of using the "using" like this ? – samer Feb 27 '16 at 13:22
[Using Statement](https://msdn.microsoft.com/en-us/library/yh598w02.aspx) in short, if your disposable objects are created in a using statement then you don't need to call Dispose EVEN if an exception triggers inside the using block. Using is like a try/finally where the finally part always contains the Dispose call on the object created at the start of the using block. And Dispose will also close the connection. – Steve Feb 27 '16 at 13:30
one last question Steve please I was planning to use the using(OracleTransaction trans = conn.BeginTransaction()) to handle any exception may happen and rollback the whole transaction in the catch part ... ur opinion much appreciated – samer Feb 27 '16 at 13:44
Not verified with Oracle, but it should be the same: [Will a using statement rollback a database transaction if an error occurs?](http://stackoverflow.com/questions/641660/will-a-using-statement-rollback-a-database-transaction-if-an-error-occurs) – Steve Feb 27 '16 at 13:50
On the other end, [Is it a better practice to explicitly call transaction rollback or let an exception trigger an implicit rollback?](http://stackoverflow.com/questions/6418992/is-it-a-better-practice-to-explicitly-call-transaction-rollback-or-let-an-except) – Steve Feb 27 '16 at 13:54
thanks alot Steve.. can I message you in some way u prefer ? – samer Feb 28 '16 at 15:39
your opinion my friend will be really appreciated http://stackoverflow.com/questions/35684669/is-that-correct-way-to-execute-function-within-c-sharp-environment – samer Feb 28 '16 at 16:15

Reading data from table then insert into table

1 Answers1