C# writing to Access DB getting duplicates error.

Hi,
I have a C# application that is using OLEDB to write data to an Access database.  For various reasons I do not want to use an autonumber field as the ID for the table that I am writing to.

In my code I am using an insert statement to store the data I also use a class module that maps each of the three fields to a class property and I also have a routine in the class module that gets the next ID number. It does this by running this SQL code:

"Select Top 1 * FROM EmailTo ORDER BY EmailToID Desc;"

This then gets me the last record in the database, I then read the ID field and add 1 to it and thus I have the next ID number.

To try to gain performance in the code that writes the data to the Access database in my main routine I have this code:

for (x = 0; x <= lm[n].To.Count - 1; x++)
{
    if (!Object.ReferenceEquals(null, lm[n].To[x]))
    {
        // Add to database to the EmailTo table with link to DataExtractTable.
        EmailTo e = new EmailTo();         //This is the class that maps to the table.
        e.EmailToID=e.GetNextID();        // This is the call to get the next ID
        e.fkDataExtractTableID = det.DataExtractTableID;                 //A field I am populating.
        e.ToEmailAddress = lm[n].To[x].ToString();                // Another field I am populating
                                    
        //Then build INSERT statement using the class properties.
        strSQL = "INSERT INTO EmailTo (EmailToID, fkDataExtractTableID, ToEmailAddress) ";
        strSQL += "VALUES (" + Convert.ToString(e.EmailToID) + ", " + Convert.ToString(e.fkDataExtractTableID) + ", \"" + e.ToEmailAddress + "\");";

        if(cn!=null)
        {
            cmd.CommandText = strSQL;
            int retVal=cmd.ExecuteNonQuery();         //Here's where I am hitting the issue with the ID field being duplicated!
            if (retVal == 0)
                throw new Exception("To Address record not saved.");
            }
            
           if(e!=null)
                e.Dispose(); //Clean up.
                                
    } //if (!Object.ReferenceEquals(null, lm[n].To[x]))
                            
} //for (x = 0; x <= lm[n].To.Count - 1; x++)

Open in new window


So it appears that my GetNextID routine is getting the ID OK but for some unknown reason the Access database has not updated to show the last record I inserted and is thus getting the wrong next ID number.

Is there some way that I can force the database to update when I INSERT a record that won't slow things down too much so that when I call GetNextID I can be sure the Access Table is up to date.
SivAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Ryan ChongCommented:
can you post what you have in function GetNextID() ?
0
SivAuthor Commented:
Yep!
        public int GetNextID()
        {
            String strSQL = "";
            OleDbDataReader dr = null;
            OleDbConnection cn = null;
            OleDbCommand cmd = null;
            int ID = 0;

            try
            {
                strSQL = "Select Top 1 * FROM EmailTo ORDER BY EmailToID Desc;";

                cn = new OleDbConnection(Common.DefCon + Common.DBFilePath);
                cn.Open();

                cmd = new OleDbCommand(strSQL, cn);
                dr = cmd.ExecuteReader(CommandBehavior.CloseConnection);

                if (dr.HasRows)
                {
                    dr.Read();
                    ID = Convert.ToInt32(dr["EmailToID"]);
                    ID += 1;
                }
                else
                {
                    ID = 1;
                }

                return ID;
            }
            catch (Exception ex)
            {
                Common.PEH("GetNextID", "EmailTo Class Module", ex.Message);
                return 0;
            }
            finally
            {
                if (cmd != null)
                {
                    cmd.Dispose();
                }

                if (dr != null)
                {
                    dr.Close();
                }
            }
        }//End of GetNextID

Open in new window

0
Ryan ChongCommented:
you may try this instead and see if that's worked? I have made some changes to the function.

        public int GetNextID()
        {
            String strSQL = "";
            OleDbDataReader dr = null;
            OleDbConnection cn = null;
            OleDbCommand cmd = null;
            int ID = 0;

            try
            {
                strSQL = "Select Max(EmailToID) MaxID FROM EmailTo ";

                cn = new OleDbConnection(Common.DefCon + Common.DBFilePath);
                cn.Open();

                cmd = new OleDbCommand(strSQL, cn);
                dr = cmd.ExecuteReader(CommandBehavior.CloseConnection);

                if (dr.Read())
                {
                    ID = Convert.ToInt32(dr["MaxID"]);
                    ID += 1;
                }
                else
                {
                    ID = 1;
                }

                return ID;
            }
            catch (Exception ex)
            {
                Common.PEH("GetNextID", "EmailTo Class Module", ex.Message);
                return 0;
            }
            finally
            {
                if (cmd != null)
                {
                    cmd.Dispose();
                }

                if (dr != null)
                {
                    dr.Close();
                }
            }
        }//End of GetNextID

Open in new window

0
Ultimate Tool Kit for Technology Solution Provider

Broken down into practical pointers and step-by-step instructions, the IT Service Excellence Tool Kit delivers expert advice for technology solution providers. Get your free copy now.

ste5anSenior DeveloperCommented:
The problem is concurrency. When two processes use your GetNextID() method at the same time, respectively the SELECT TOP1 query, they'll get the same number.

This is a known issue with all databases, when you create your own auto-number.

Normally, you would use an approach which locks that table, so that the access gets serialized. But this is a NP hard task ;) Especially in Access any locking would impose normally an huge performance bottleneck, as it requires to lock the entire database.

There are two  solutions:

1. using the exception: When you'll get an duplicate error on your ID, just retrieve a new one and replay the insert.

2. using a subquery: use a subquery in the INSERT statement to get the new ID and requery the inserted row by a candidate key to get the used ID.

(3. ) Use 1. AND a mutex/semaphore mechanism in your C# application to serialize the INSERT statements. Thus you will reduce the number of exceptions thrown..
0
SivAuthor Commented:
Ran your code and get an error:

"Syntax error (missing operator) in query expression 'Max(EmailToID) MaxID'.
0
AndyAinscowFreelance programmer / ConsultantCommented:
A simple way around this is to do what you are doing.
When the exception is thrown with this error you trap that (as you would do with any potential problem when coding), then just add 1 to the ID and repeat.  Keep doing this until it saves without problem.
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
SivAuthor Commented:
@ste5an
I am only running this in one process (as far as I am aware), so there should not be another routine calling GetNextID other than this one.  Admittedly I am looping through quickly and the routine will be called multiple times in quick succession.

I had thought of calling it the first time and then just adding 1 each subsequent loop round?
0
AndyAinscowFreelance programmer / ConsultantCommented:
>>I had thought of calling it the first time and then just adding 1 each subsequent loop round?

That is what I just said as a simple solution.  (Locking the DB would involve a lot more work)
0
SivAuthor Commented:
Is there not some neater way of doing this like calling some kind of flush command that will force the database to update so that when I get to the GetNextID call it will always be up to date!? These solutions seem a bit messy and possibly error prone.

I seem to remember (it's a while since I have used Access as the database as I am more used to using SQL Server, but this job requires using MS Access) that there used to be a way of doing a BeginTransaction .. EndTransaction type thing where you could ensure integrity, would something like this force the database to update?
0
AndyAinscowFreelance programmer / ConsultantCommented:
Why should it be error prone?  If the ID is duplicated then access will throw the exception.  If it is not duplicated then access will save the record.  (As to flush - what difference should that make, the earlier record is written otherwise you would not be seeing this duplicate record error)
0
SivAuthor Commented:
@AndyAinscow,

In the sense of it updating the system enough that when I call my GetNextID instead of it having out of date data it is up to date and therefore is not causing an error. What I am after is some method that forces the database to be up to date, rather than where I am now and it's not as it's telling me the last ID in the EmailTo table, it's giving me the previous one.

The point of what I was doing was to try and get the performance up, this feels like it would not be any quicker as I am looping around retrying the save data until it works.  Or are we saying that Access/Jet is always slow at updating when you update its records programmatically and I just have to suck it up! :)
0
AndyAinscowFreelance programmer / ConsultantCommented:
I think the slowness comes from the massive change between how .net handles databases.  (Disconnected vs connected).
You could use transactions BUT that might have a knock on effect elsewhere should this app be for multi users.
Doing your own primary key values is always prone to various issues.  
Some time ago I had to generate a next in order (but start at the seed for each new year) ID - I had a table that had ONLY that purpose.  An autonumber for the other records and two fields, year and ID.  My Get ID code would repeatedly add to that table until successful then I had an ID I could use for the other records.
0
SivAuthor Commented:
@AndyAinscow,

I have implemented your idea and it certainly works and it looks like the performance isn't too bad.
It seems to me that the issue is that Jet just takes a small but nonetheless appreciable amount of time to reflect the change and your fix does sidestep the issue!

Thanks for you help.
0
SivAuthor Commented:
The add one on fix seems to work OK so thanks a lot for getting me past this issue!
0
ste5anSenior DeveloperCommented:
@Siv: The behavior, you have seen, is by design in a multi-user database.

The only solution in Access is to open the database in exclusive mode and to the change.

In other RDBMS you would use a lock(-escalation) strategy, which ensures that you have exclusive access to your table.

btw, use using instead of a finally block.
0
SivAuthor Commented:
ste5an,
Thanks for the advice, can you demonstrate what you mean re the "Using" statement, I am an ex VB'er so any help getting C# right is appreciated.
0
AndyAinscowFreelance programmer / ConsultantCommented:
https://msdn.microsoft.com/en-us/library/yh598w02.aspx

The 'using' will cleanly dispose of the objects - no need to do it explicitly in the finally block you have.  It means a bit less code for you to write.
1
SivAuthor Commented:
In the link you supplied I get how you would use "using" but am struggling to see how I would wrap that around my GetNextID routine?
0
ste5anSenior DeveloperCommented:
E.g.

 
        public int GetNextID()
        {
            const string SQL_SELECT = "SELECT MAX(EmailToID) MaxID FROM EmailTo;";
            int result = 0;
            try
            {
                using (OleDbConnection cn = new OleDbConnection(Common.ConnectionString()))
                {
                    cn.Open();
                    using (OleDbCommand cmd = new OleDbCommand(SQL_SELECT, cn))
                    {
                        using (OleDbDataReader dr = cmd.ExecuteReader(CommandBehavior.CloseConnection))
                        {
                            result = 0;
                            if (dr.Read())
                            {
                                result = Convert.ToInt32(dr["MaxID"]);
                            }

                            result += 1;
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                Common.PEH("GetNextID", "EmailTo Class Module", ex.Message);                
            }

            return result;
        }

Open in new window


Use string (language type) in favor to String (framework type).

Instead of Common.DefCon + Common.DBFilePath use one call to Common.ConnectionString(). Consider the case that you want to change the database from Access to SQL. Then you need to recode that part. Here I also like factory methods, thus using a method CreateConnection() is often more flexible.
Does your Common.PEH() only log errors? If so, then rethrow the original exception. Exception swallowing is not a good idea.
0
SivAuthor Commented:
@ste5an,

Wow, it's taking me a while to get my head around the changes you have made to that routine! I noticed in the MSDN example code that AndyAinscow pointed me to, that it does say that the compiler effectively does the same as how I had it written i.e. it implements a "finally" so although I get the general gist that the use of "Using" should save me some coding as I don't have to write the finally block, it does seem that to get "using" to work in this instance is a lot of work and to my less experienced eye looks a lot less readable for someone who has to work on the code after me.

I am very grateful for your insight here though I just need to wrap my brain around it!

You mention using "string" instead of "String" I often wondered why there was two versions of most of these types of variable. Would you mind explaining why the language version is better. I am keen to understand the difference.

Common.PEH is just a common routine I use to display error messages without having to write it in each catch block.  

Again would you mind explaining why rethrowing the error is a good thing.  I have noticed that the template for try ,,, catch ,,, finally does insert the rethrow and I have wondered why you would do that.  My feeling is that I want to catch the error and display it to the user so that they can report it to me so that I am aware of the bug in my code.  (The PEH call displays the error message and tells me which routine in which form triggered it and shows my phone number or a local admin contact's number who will do first level support and then pass on to me if it's not a user fixable issue).
0
AndyAinscowFreelance programmer / ConsultantCommented:
>>You mention using "string" instead of "String" I often wondered why there was two versions of most of these types of variable. Would you mind explaining why the language version is better. I am keen to understand the difference.

You might want to open a new question for that.  It would probably get rather more experts contributing.
0
SivAuthor Commented:
I was really replying to Ste5an as he raised it in one of his comments and thought he would reply.
0
ste5anSenior DeveloperCommented:
Just post it as new questions and link also to this..
0
AndyAinscowFreelance programmer / ConsultantCommented:
When a new question is asked then numbers of experts will receive an email notification.  Continuing in an old (and closed) question will mean other experts will only see it if they happen to look here or have made a comment (= receive an email that someone else has made a comment).
Which is best:  One viewpoint or lots of experts with **possibly** different info.  
Note I am not implying any criticism of Ste5an.
0
SivAuthor Commented:
Reposted as a new question.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
C#

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.