[Mono-dev] [PATCH] Mono.Data.SqliteClient fixes from Beagle
Hi, Previously, SqliteClient did not properly check the return values of some of its calls into the sqlite API. The sqlite API can return BUSY in some locking-related situations, but this wasn't being handled appropriately, and strange things would happen. This patch wraps Sqlite errors into a new exception class, which allows the user to respond accordingly to BUSY states. For example, Beagle sleeps a bit and then retries when it sees a SqliteException with BUSY inside it. This work was done by Jon Trowbridge, but I've been maintaining it and keeping it up-to-date with the more recent SqliteClient changes applied to the tree. After a long period of testing in Beagle I feel its time this should be fixed in the main tree :) Any comments? Does this look ok for me to commit? Thanks, Daniel Index: SqliteDataReader.cs === --- SqliteDataReader.cs (revision 53531) +++ SqliteDataReader.cs (working copy) @@ -43,29 +43,30 @@ #region Fields private SqliteCommand command; - private ArrayList rows; + private IntPtr pVm; + private int version; + + private ArrayList current_row; + private ArrayList columns; private Hashtable column_names; - private int current_row; private bool closed; - private bool reading; - private int records_affected; #endregion #region Constructors and destructors - internal SqliteDataReader (SqliteCommand cmd, IntPtr pVm, int version) + internal SqliteDataReader (SqliteCommand cmd, IntPtr _pVm, int _version) { command = cmd; - rows = new ArrayList (); + pVm = _pVm; + version = _version; + + current_row = new ArrayList (); + columns = new ArrayList (); column_names = new Hashtable (); closed = false; - current_row = -1; - reading = true; - ReadpVm (pVm, version); - ReadingDone (); } #endregion @@ -81,11 +82,11 @@ } public object this[string name] { - get { return ((ArrayList) rows[current_row])[(int) column_names[name]]; } + get { return current_row [(int) column_names[name]]; } } public object this[int i] { - get { return ((ArrayList) rows[current_row])[i]; } + get { return current_row [i]; } } public bool IsClosed { @@ -93,90 +94,90 @@ } public int RecordsAffected { - get { return records_affected; } + get { return command.NumChanges (); } } #endregion #region Internal Methods - - internal void ReadpVm (IntPtr pVm, int version) + + internal bool ReadNextColumn () { int pN = 0; IntPtr pazValue = IntPtr.Zero; IntPtr pazColName = IntPtr.Zero; SqliteError res; - while (true) { -if (version == 3) { - res = Sqlite.sqlite3_step (pVm); - pN = Sqlite.sqlite3_column_count (pVm); -} else - res = Sqlite.sqlite_step (pVm, out pN, out pazValue, out pazColName); -if (res == SqliteError.ERROR) { - throw new ApplicationException ("Sqlite error"); -} -if (res == SqliteError.DONE) { - break; -} -// We have some data; lets read it -if (column_names.Count == 0) { - for (int i = 0; i < pN; i++) { - string colName = ""; - if (version == 2) { - IntPtr fieldPtr = (IntPtr)Marshal.ReadInt32 (pazColName, i*IntPtr.Size); - colName = Marshal.PtrToStringAnsi (fieldPtr); - } else { - colName = Marshal.PtrToStringAnsi (Sqlite.sqlite3_column_name (pVm, i)); - } - columns.Add (colName); - column_names [colName] = i; - } -} -ArrayList data_row = new ArrayList (pN); + if (version == 3) { +res = Sqlite.sqlite3_step (pVm); +pN = Sqlite.sqlite3_column_count (pVm); + } else +res = Sqlite.sqlite_step (pVm, out pN, out pazValue, out pazColName); + + if (res == SqliteError.DONE) { +return false; + } + + if (res != SqliteError.ROW) +throw new SqliteException (res); + + // We have some data; lets read it + + // If we are reading the first column, populate the column names + if (column_names.Count == 0) { for (int i = 0; i < pN; i++) { - string colData = ""; + string colName = ""; if (version == 2) { - IntPtr fieldPtr = (IntPtr)Marshal.ReadInt32 (pazValue, i*IntPtr.Size); - colData = Marshal.PtrToStringAnsi (fieldPtr); - data_row.Add (Marshal.PtrToStringAnsi (fieldPtr)); + IntPtr fieldPtr = (IntPtr)Marshal.ReadInt32 (pazColName, i*IntPtr.Size); + colName = Marshal.PtrToStringAnsi (fieldPtr); } else { - switch (Sqlite.sqlite3_column_type (pVm, i)) { - case 1: -Int64 sqliteint64 = Sqlite.sqlite3_column_int64 (pVm, i); -data_row.Add (sqliteint64.ToString ()); -break; - case 2: -double sqlitedouble = Sqlite.sqlite3_column_double (pVm, i); -data_row.Add (sqlitedouble.ToString ()); -break; - case 3: -colData = Marshal.PtrToStringAnsi (Sqlite.sqlite3_column_text (pVm, i)); -data_row.Add (colData); -
[Mono-dev] [PATCH] Mono.Data.SqliteClient fixes from Beagle
Hi, Previously, SqliteClient did not properly check the return values of some of its calls into the sqlite API. The sqlite API can return BUSY in some locking-related situations, but this wasn't being handled appropriately, and strange things would happen. This patch wraps Sqlite errors into a new exception class, which allows the user to respond accordingly to BUSY states. For example, Beagle sleeps a bit and then retries when it sees a SqliteException with BUSY inside it. This work was done by Jon Trowbridge, but I've been maintaining it and keeping it up-to-date with the more recent SqliteClient changes applied to the tree. After a long period of testing in Beagle I feel its time this should be fixed in the main tree :) Any comments? Does this look ok for me to commit? Thanks, Daniel Index: SqliteDataReader.cs === --- SqliteDataReader.cs (revision 53531) +++ SqliteDataReader.cs (working copy) @@ -43,29 +43,30 @@ #region Fields private SqliteCommand command; - private ArrayList rows; + private IntPtr pVm; + private int version; + + private ArrayList current_row; + private ArrayList columns; private Hashtable column_names; - private int current_row; private bool closed; - private bool reading; - private int records_affected; #endregion #region Constructors and destructors - internal SqliteDataReader (SqliteCommand cmd, IntPtr pVm, int version) + internal SqliteDataReader (SqliteCommand cmd, IntPtr _pVm, int _version) { command = cmd; - rows = new ArrayList (); + pVm = _pVm; + version = _version; + + current_row = new ArrayList (); + columns = new ArrayList (); column_names = new Hashtable (); closed = false; - current_row = -1; - reading = true; - ReadpVm (pVm, version); - ReadingDone (); } #endregion @@ -81,11 +82,11 @@ } public object this[string name] { - get { return ((ArrayList) rows[current_row])[(int) column_names[name]]; } + get { return current_row [(int) column_names[name]]; } } public object this[int i] { - get { return ((ArrayList) rows[current_row])[i]; } + get { return current_row [i]; } } public bool IsClosed { @@ -93,90 +94,90 @@ } public int RecordsAffected { - get { return records_affected; } + get { return command.NumChanges (); } } #endregion #region Internal Methods - - internal void ReadpVm (IntPtr pVm, int version) + + internal bool ReadNextColumn () { int pN = 0; IntPtr pazValue = IntPtr.Zero; IntPtr pazColName = IntPtr.Zero; SqliteError res; - while (true) { -if (version == 3) { - res = Sqlite.sqlite3_step (pVm); - pN = Sqlite.sqlite3_column_count (pVm); -} else - res = Sqlite.sqlite_step (pVm, out pN, out pazValue, out pazColName); -if (res == SqliteError.ERROR) { - throw new ApplicationException ("Sqlite error"); -} -if (res == SqliteError.DONE) { - break; -} -// We have some data; lets read it -if (column_names.Count == 0) { - for (int i = 0; i < pN; i++) { - string colName = ""; - if (version == 2) { - IntPtr fieldPtr = (IntPtr)Marshal.ReadInt32 (pazColName, i*IntPtr.Size); - colName = Marshal.PtrToStringAnsi (fieldPtr); - } else { - colName = Marshal.PtrToStringAnsi (Sqlite.sqlite3_column_name (pVm, i)); - } - columns.Add (colName); - column_names [colName] = i; - } -} -ArrayList data_row = new ArrayList (pN); + if (version == 3) { +res = Sqlite.sqlite3_step (pVm); +pN = Sqlite.sqlite3_column_count (pVm); + } else +res = Sqlite.sqlite_step (pVm, out pN, out pazValue, out pazColName); + + if (res == SqliteError.DONE) { +return false; + } + + if (res != SqliteError.ROW) +throw new SqliteException (res); + + // We have some data; lets read it + + // If we are reading the first column, populate the column names + if (column_names.Count == 0) { for (int i = 0; i < pN; i++) { - string colData = ""; + string colName = ""; if (version == 2) { - IntPtr fieldPtr = (IntPtr)Marshal.ReadInt32 (pazValue, i*IntPtr.Size); - colData = Marshal.PtrToStringAnsi (fieldPtr); - data_row.Add (Marshal.PtrToStringAnsi (fieldPtr)); + IntPtr fieldPtr = (IntPtr)Marshal.ReadInt32 (pazColName, i*IntPtr.Size); + colName = Marshal.PtrToStringAnsi (fieldPtr); } else { - switch (Sqlite.sqlite3_column_type (pVm, i)) { - case 1: -Int64 sqliteint64 = Sqlite.sqlite3_column_int64 (pVm, i); -data_row.Add (sqliteint64.ToString ()); -break; - case 2: -double sqlitedouble = Sqlite.sqlite3_column_double (pVm, i); -data_row.Add (sqlitedouble.ToString ()); -break; - case 3: -colData = Marshal.PtrToStringAnsi (Sqlite.sqlite3_column_text (pVm, i)); -data_row.Add (colData); -
Re: [Mono-dev] [PATCH] Mono.Data.SqliteClient fixes from Beagle
Daniel Drake wrote: Previously, SqliteClient did not properly check the return values of some of its calls into the sqlite API. In light of the issue on mono-list about a "CREATE TABLE ...; CREATE TABLE ...;" command, if my hasty conclusion was true that we can't prepare the whole thing before executing, and in which case SqliteCommand.cs needs to be rewritten, it might be better to hold off on modifying that file until after. At least, if I'm going to do the rewriting, that's my preference. But I won't have time to do that for a few weeks, so if you or anyone else wants to work on that before then, just ignore my opinion. :) -- - Joshua Tauberer http://taubz.for.net ** Nothing Unreal Exists ** ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Mono.Data.SqliteClient fixes from Beagle
Hello, > Any comments? Does this look ok for me to commit? It looks good to commit. Do you think you might be able to produce documentation for it as well? I have checked in the stubs, it might be worth at least documenting the pieces that will throw exceptions. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Mono.Data.SqliteClient fixes from Beagle
Hi Miguel Miguel de Icaza wrote: It looks good to commit. Do you think you might be able to produce documentation for it as well? I have checked in the stubs, it might be worth at least documenting the pieces that will throw exceptions. I will try and get around to documenting those parts when I commit this. I will wait for Joshua to complete his changes first, then I'll submit this again, just to check that everything looks ok. Thanks, Daniel ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list