Problems with parameter adding and NpgsqlParameterCollection

[Francisco Figueiredo Jr.]

Hi all!

I just received a report from a user that he is having problems with 2.1.x releases.

He is using LLBLGen with Npgsql and after 2.1, he started to get exceptions: 

"The parameter already belongs to a collection."

This is being caused by the following line: 

if (value.Collection != null)

            {

                throw new InvalidOperationException("The parameter already belongs to a collection");

            }

This line was added by this commit:  https://github.com/npgsql/Npgsql/commit/76cfde1dab27dfe7835e81d68b15df4636fc8cef

It seems LLBLGen is reusing the parameter to be added to more than one NpgsqlParameterCollection. 

I wonder if this pattern is ok (add a parameter to more than one collection) and if so, I think we have to change Npgsql to not complain about that.

What do you all think?

--
Regards,

Francisco Figueiredo Jr.
Npgsql Lead Developer
http://www.npgsql.org
http://gplus.to/franciscojunior
http://fxjr.blogspot.com
http://twitter.com/franciscojunior

[Glen Parker]

Hi guys.

I believe this behavior matches SqlParameterCollection.  It's been a few months since I wrote that, and I don't have time to double check at the moment.  But it's also important because it allows a parameter to invalidate its collection's lookup indexes when its name is changed.

-Glen

--
You received this message because you are subscribed to the Google Groups "Npgsql Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to npgsql-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Francisco Figueiredo Jr.]

On Mon, Apr 14, 2014 at 12:52 PM, Glen Parker <glen...@gmail.com> wrote:

Hi guys.

I believe this behavior matches SqlParameterCollection.  It's been a few months since I wrote that, and I don't have time to double check at the moment.  But it's also important because it allows a parameter to invalidate its collection's lookup indexes when its name is changed.

Hi, Glen!

Thanks for having a look at this.

I can confirm with a simple program that sqlparametercollection does exactly as you said. It complains if a parameter is added in more than one Parameters collection.

I used this simple program:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
 
using System.Data;
using System.Data.SqlClient;
 
namespace ConsoleApplication5
{
    class Program
    {
        static void Main(string[] args)
        {
 
            SqlCommand c = new SqlCommand();
 
            SqlParameter p1 = new SqlParameter();
 
            c.Parameters.Add(p1);
 
            SqlCommand c2 = new SqlCommand();
 
            c2.Parameters.Add(p1);
 
        }
    }
}

I'll report that back to the user and ask him to check how LLBL is using the parameters.

[frans...@gmail.com]

Hi all, I wrote LLBLGen Pro. The code crashes at a point where a query has been generated (with parameters) and it becomes a derived table in another query: at that point the parameters are added to the outer query. However, the same code works properly on sql server and all other databases we support. I'll check whether something is off in our code or whether removing a parameter from the npgsql parameter collection doesn't reset its collection property. I'll report back if I have found something.

[frans...@gmail.com]

Bug is in NpgsqlParameterCollection.Clear:

        /// <summary>
        /// Removes all items from the collection.
        /// </summary>
        public override void Clear()
        {
            NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Clear");
            this.InternalList.Clear();
            this.InvalidateHashLookups();
        }

You don't reset the parent in the parameters you remove with this.InternalList.Clear(). Classic mistake with IList.Clear() implementations ;)

Hope this helps. I've created a ticket at github. https://github.com/npgsql/Npgsql/issues/231

I'll see if I can create a fix for it in the next 10 minutes.

On Monday, April 14, 2014 11:20:42 PM UTC+2, Francisco Figueiredo Jr. wrote:

[frans...@gmail.com]

Pull: https://github.com/npgsql/Npgsql/pull/234

Also cleaned up the Remove* methods, as they re-implemented the same things 6 times (and each time using slightly different constructs, they now all end up in 1 method).

Hopefully this resolves the issue. Cheers, :)

Frans

 

------------------------------------------------------------------------

Lead developer of LLBLGen Pro (O/R mapper/designer) and ORM Profiler.

http://www.llblgen.com | http://www.ormprofiler.com

Blog: http://weblogs.asp.net/fbouma

Twitter: http://twitter.com/FransBouma

------------------------------------------------------------------------

On Monday, April 14, 2014 11:20:42 PM UTC+2, Francisco Figueiredo Jr. wrote:

[Francisco Figueiredo Jr.]

On Wed, May 7, 2014 at 1:11 PM, <frans...@gmail.com> wrote:

Pull: https://github.com/npgsql/Npgsql/pull/234

Also cleaned up the Remove* methods, as they re-implemented the same things 6 times (and each time using slightly different constructs, they now all end up in 1 method).

Hopefully this resolves the issue. Cheers, :)

Frans

 

Hey, Frans! Nice to see you here!

That's excellent! 

Thanks for your patch and for having a look at this! I suspected there could be something wrong with Npgsql about that. 

I'll run your patch and test in our build server now and will let you know if we got any regression.