[Mono-dev] PATH: PostgreSQL Linq2Sql provider fix

[Jonathan Pryor]

On Fri, 2009-06-12 at 10:19 +0200, Federico Di Gregorio wrote:
> Currently it is impossible to use the DbLinqProvider  and
> DbLinqConnectionType connection parameters with PostgreSQL because
> Npgsql does not like extra aguments. The attached patch fixes this.

Four comments.

1. It would likely be easier to integrate if you worked against upstream
DbLinq, if at all possible.

	http://code.google.com/p/dblinq2007/source/checkout

2. The resulting behavior of Vendor.CreateDbConnection() is broken.  If
the connection string contains only DbLinqProvider but not
DbLinqConnectionType, it'll use the DbLinqProvider value as the
DbLinqConnectionType value.  This isn't bad, necessarily, though it will
result in a confusing error message (complaining about
DbLinqConnectionType when it isn't present).

The worse behavior is that if *both* DbLinqConnectionType and
DbLinqProvider are specified, the regex will extract the *first* one
specified.  Given a connection string of:

	DbLinqProvider=PostgreSQL; DbLinqConnectionType=SomeType

it will try to use PostgreSQL as the ConnectionType, NOT SomeType, which
will be wrong.

3. My bigger problem is a conceptual one.  Usually it is preferable for
the code that is parsing for a given parameter to remove it (if
necessary), so that the entire code base doesn't need to be known to
understand a local method.

Specifically, I'm leery about Vendor needing to know about -- and remove
-- the DbLinqProvider parameter, as Vendor doesn't actually care about
that parameter (DbLinqProvider is needed in order to get the Vendor in
the first place).  If we add additional DbLinq parameters in the future,
it means that we'll need to modify Vendor again as well, further
spreading the logic for handling the parameter throughout the code base.

Consequently, I would far prefer that the DbLinqProvider removal code be
within DataContext, as that's where the parameter is searched for and
used.

4. You should add unit tests to check the removal logic
to .../src/DbLinq/Test/DataContextTest.cs, which also contains other
tests for DbLinqConnectionType behavior.

 - Jon