Documentation wrong/incomplete regarding own function calls using select * from <function> about doc HOT 9 CLOSED

At the end of the day, this is about whether it's reasonable - and also common - to mix and match old and new versions of two software components, especially which work tightly together (e.g. LLBLGen and Npgsql)... For example, I think we usually see people upgrading both .NET and EF together, rather than attempting to run old EF versions on new .NET (that's not an example of tightly-bound components). I have the feeling (and this isn't backed by numbers) that users which tend to stay back on some components (e.g. LLBLGen) stay back on the others as well (e.g. Npgsql), especially when they're so related.

Mixing and matching is fundamentally problematic; it means forcing together two components which typically weren't tested together (in those versions), and so always risky.

There's still the question of how to handle cases where a user does attempt to mix and match... With nuget dependencies it can be a matter of providing the right lower and upper bounds on dependencies. Your case is obviously trickier, since you're not taking a dependency on Npgsql; that may be a slight drawback of that model (beyond checking assembly versions at runtime, which is possible).

If we set aside this mixing and matching, then what remains really is allowing some time for the change to propagate through the ecosystem. It's not pleasant during the moment of transition (i.e. right now), but it does pass.

I must say I rarely run into breaking changes in ado.net providers, if at all. Npgsql is the exception to that :)

I agree. I think it's also because ADO.NET providers generally don't evolve very much, with Npgsql being a bit of an exception there; where there's lots of change and enhancements, there's sometimes also more breaking changes. But I promise to keep this in mind and not do breaks lightly.

from doc.

Great! :) 👍

from doc.

Hey @FransBouma.

FWIW we had a discussion around all the points you raised. I know this is a very non-trivial breaking change, and we didn't do this lightly. Note that we don't have any plan to remove the opt-out switch at this point - it's one of those things that may well just stay as a compat flag forever.

[...] people simply reference the latest Npgsql from github and assume things will ' just work'
Why not enable these features as opt-in?

Doing this kind of thing as an opt in generally has little value, since a main point in this change is to make things smoother/more logical for new users. A new user coming to PostgreSQL and Npgsql expects CommandType.StoredProcedure to invoke, well, a stored procedure; if they try to do that and fail because it actually invokes a function, an obscure opt-in flag somewhere in the docs isn't going to help much... In addition, this makes Npgsql more compatible with other ADO.NET drivers.

At the end of the day, I do believe it's sometimes acceptable to do minor breaking changes so that the project continues to evolve (it's a major version, after all); I don't believe problems/bugs/warts should be kept around forever just because fixing them would be breaking change.

Especially with stored procedures which are seriously limited on PostgreSQL [...]

Which specific limitation are you thinking of? Regardless, users can obviously still use functions by simply using CommandType.Text with SELECT * FROM func() - exactly as it works with other databases.

The documentation you provide says, use $1, $2 etc. for parameters. But ... that doesn't work:

Can you provide the full code sample? Specifically, how are adding the parameters - they shouldn't be named if you're using positional placeholders ($1, $2). If that's the issue, then I can definitely amend the sample to make that clear.

Also, output parameters shouldn't be specified, which isn't clear from the example either

True, though this is only a brief breaking change note that isn't intended to cover everything about invoking functions/procedures. The full docs for that are here (see especially this section). We should link to that from the note though.

from doc.

@roji To make it more clear: my software (LLBLGen Pro) is now broken on Npgsql 7, unless the user switches off the change by using the switch. We'll release a change in our next version to work around this (by generating select * from ) but all existing versions (which go back 20 years) are broken. So all users of these versions generating code for PostgreSQL using functions as they used to will run into a crash when they reference Npgsql 7, which is the default version when referencing Npgsql.

So all in all, I'm pretty pissed off about this: it's a breaking change I can't engineer my way around it, as the software that's broken is already out there and released.

This isn't some 'minor' small thing, you broke my software in a big way. There's no way for me to change this other than to hope my users read your breaking changes docs but likely will tell me that my stuff is broken. I already had one customer tell me that, luckily I remembered the breaking changes docs and could tell him there was a switch, but they're not happy with this workaround as they know these switches inevitably will get removed too.

I know it's a major version, and breaking changes are sometimes inevitable. However you also know that some changes are not always the wisest choice. I have several changes I want to make but can't because I'll break 20 years of code out there if I do which will enforce my users to do a major rewrite.

Which specific limitation are you thinking of? Regardless, users can obviously still use functions by simply using CommandType.Text with SELECT * FROM func() - exactly as it works with other databases.

My users use this indirectly and I'm sure there are others who use this indirectly too via other means. the limitation is merely returning a resultset, you can't do that with a stored procedure afaik.
select * from func() is a syntax that's rarely known because people used functions as stored procedures. Sure, how this is setup in Npgsql is far from ideal due to legacy decisions you weren't part of, but the thing is: it's done this way and sometimes you have to live with that. Like I said elsewhere: there's not really a 'right' decision here considering the legacy that's out there: damned if you do and damned if you don't. IMHO the 'right' thing to do in these cases is to special case the 'new' feature and keep what's out there and known.

Can you provide the full code sample? Specifically, how are adding the parameters - they shouldn't be named if you're using positional placeholders ($1, $2). If that's the issue, then I can definitely amend the sample to make that clear.

I did name the parameters and thus that doesn't work apparently indeed, tho how can one not name a parameter? Specifying an empty parameter name? It's totally unclear from the example. I name the parameters in queries like :p1, etc. but for procs I named the parameters like the parameters in the procedure. I had to prefix these with : tho to make it work.

from doc.

So all users of these versions generating code for PostgreSQL using functions as they used to will run into a crash when they reference Npgsql 7, which is the default version when referencing Npgsql.

So all in all, I'm pretty pissed off about this: it's a breaking change I can't engineer my way around it, as the software that's broken is already out there and released.

I'm sorry, but if that's your bar, then it's never possible to do any breaking changes, ever - since your old versions would generate code that wouldn't be compatible with it.

IMHO it's simply unreasonable to expect to be able to stay on old versions of LLBLGen (or whatever), and at the same time use the latest version of Npgsql... If you want to never be broken, don't upgrade either LLBLGen or Npgsql; if you want to be on the latest, upgrade both. I don't know the specifics of your support policies, but saying that old versions of LLBLGen aren't compatible with Npgsql 7.0 doesn't seem unreasonable to me.

This isn't some 'minor' small thing, you broke my software in a big way. There's no way for me to change this other than to hope my users read your breaking changes docs but likely will tell me that my stuff is broken. I already had one customer tell me that, luckily I remembered the breaking changes docs and could tell him there was a switch, but they're not happy with this workaround as they know these switches inevitably will get removed too.

I don't understand - the Npgsql 7.0 release notes clearly discuss this breaking change and show the switch. Is it too much to expect a user who upgrades to a major version to read that, to the extent that we should avoid doing such a change?

I know it's a major version, and breaking changes are sometimes inevitable. However you also know that some changes are not always the wisest choice. I have several changes I want to make but can't because I'll break 20 years of code out there if I do which will enforce my users to do a major rewrite.

I agree with this, and believe me, in an ideal world there are many more changes I'd do and which I don't, because of this. But the 4 of us discussed this specific question and came to the conclusion that although it will cause some minor pain during the transition, that pain is worth it to land us in a better place in this instance.

Can you provide the full code sample? Specifically, how are adding the parameters - they shouldn't be named if you're using positional placeholders ($1, $2). If that's the issue, then I can definitely amend the sample to make that clear.

I did name the parameters and thus that doesn't work apparently indeed, tho how can one not name a parameter? Specifying an empty parameter name? It's totally unclear from the example. I name the parameters in queries like :p1, etc. but for procs I named the parameters like the parameters in the procedure. I had to prefix these with : tho to make

Give the parameter docs a read - here's a quick summary. PostgreSQL's native parameters are positional/unnamed, and Npgsql supports that; here's how you can not name a parameter:

await using var cmd = new NpgsqlCommand("INSERT INTO table (col1) VALUES ($1), ($2)", conn)
{
    Parameters =
    {
        new() { Value = "some_value" },
        new() { Value = "some_other_value" }
    }
};

await cmd.ExecuteNonQueryAsync();

However, at some point long ago, someone decided that Npgsql should also support named parameters, by parsing SQL in the driver, identifying the named placeholders and replacing them for you. This is why you're able to use parameters like :p1 or @p1. Positional parameter support was finally added to Npgsql 6.0, while preserving the named mode (see, we don't always break 😉).

So it's perfectly fine if you want to use :p1; the more native (and slightly more performant) way would be to use positional parameters.

from doc.

So all users of these versions generating code for PostgreSQL using functions as they used to will run into a crash when they reference Npgsql 7, which is the default version when referencing Npgsql.
So all in all, I'm pretty pissed off about this: it's a breaking change I can't engineer my way around it, as the software that's broken is already out there and released.

I'm sorry, but if that's your bar, then it's never possible to do any breaking changes, ever - since your old versions would generate code that wouldn't be compatible with it.

IMHO it's simply unreasonable to expect to be able to stay on old versions of LLBLGen (or whatever), and at the same time use the latest version of Npgsql... If you want to never be broken, don't upgrade either LLBLGen or Npgsql; if you want to be on the latest, upgrade both. I don't know the specifics of your support policies, but saying that old versions of LLBLGen aren't compatible with Npgsql 7.0 doesn't seem unreasonable to me.

That's the thing, I can't control that. My software works with DbProviderFactory, and factories on .netcore+, it's not tied to a single npgsql version. :) So the user can use whatever npgsql version there is. Sure using an old npgsql version when a new llblgen pro version expects a given npgsql version might not work, but using a relatively new llblgen pro version (e.g. our latest GA) and the latest Npgsql version should work fine (but it doesn't!).

None of my versions work currently with npgsql 7 while they work fine with earlier versions due to this breaking change. Again, I understand this is a major version change and breaking changes will happen, but people referencing things through nuget won't know that: they see '7 is the latest, let's use that', and run into this.

This isn't some 'minor' small thing, you broke my software in a big way. There's no way for me to change this other than to hope my users read your breaking changes docs but likely will tell me that my stuff is broken. I already had one customer tell me that, luckily I remembered the breaking changes docs and could tell him there was a switch, but they're not happy with this workaround as they know these switches inevitably will get removed too.

I don't understand - the Npgsql 7.0 release notes clearly discuss this breaking change and show the switch. Is it too much to expect a user who upgrades to a major version to read that, to the extent that we should avoid doing such a change?

If they upgrade, that can be expected. If they reference 7 because they generated code today for the first time, then no I don't think they can be expected to read that. Also, they expect my software to work, right? And it won't work out of the box. Every user has to use the documented switch with the defaults.

I know it's a major version, and breaking changes are sometimes inevitable. However you also know that some changes are not always the wisest choice. I have several changes I want to make but can't because I'll break 20 years of code out there if I do which will enforce my users to do a major rewrite.

I agree with this, and believe me, in an ideal world there are many more changes I'd do and which I don't, because of this. But the 4 of us discussed this specific question and came to the conclusion that although it will cause some minor pain during the transition, that pain is worth it to land us in a better place in this instance.

It's not minor for people like me, sadly...

Can you provide the full code sample? Specifically, how are adding the parameters - they shouldn't be named if you're using positional placeholders ($1, $2). If that's the issue, then I can definitely amend the sample to make that clear.

I did name the parameters and thus that doesn't work apparently indeed, tho how can one not name a parameter? Specifying an empty parameter name? It's totally unclear from the example. I name the parameters in queries like :p1, etc. but for procs I named the parameters like the parameters in the procedure. I had to prefix these with : tho to make

Give the parameter docs a read - here's a quick summary. PostgreSQL's native parameters are positional/unnamed, and Npgsql supports that; here's how you can not name a parameter:

await using var cmd = new NpgsqlCommand("INSERT INTO table (col1) VALUES ($1), ($2)", conn)
{
    Parameters =
    {
        new() { Value = "some_value" },
        new() { Value = "some_other_value" }
    }
};

await cmd.ExecuteNonQueryAsync();

However, at some point long ago, someone decided that Npgsql should also support named parameters, by parsing SQL in the driver, identifying the named placeholders and replacing them for you. This is why you're able to use parameters like :p1 or @p1. Positional parameter support was finally added to Npgsql 6.0, while preserving the named mode (see, we don't always break 😉).

So it's perfectly fine if you want to use :p1; the more native (and slightly more performant) way would be to use positional parameters.

Alright, I'll go for that in this case as I then can save on some string concatenations :) Thanks for the info.

from doc.

That's the thing, I can't control that. My software works with DbProviderFactory, and factories on .netcore+, it's not tied to a single npgsql version. :) So the user can use whatever npgsql version there is. Sure using an old npgsql version when a new llblgen pro version expects a given npgsql version might not work, but using a relatively new llblgen pro version (e.g. our latest GA) and the latest Npgsql version should work fine (but it doesn't!).

I understand... You could use reflection at runtime and check ADO.NET assembly versions, throwing an informative exception when something is incompatible - without taking a reference on anything. If you don't want to get into that (I'd understand), then apart from both of us comitting to never doing breaking changes (which I'm not ready to do), it may be good to have an LLBLGen compatibility table which tells users which driver versions can be used with which LLBLGen version (you maybe already have that). I understand that your users may not look at it, but that can't be a reason for us to hold software back forever.

None of my versions work currently with npgsql 7 while they work fine with earlier versions due to this breaking change. Again, I understand this is a major version change and breaking changes will happen, but people referencing things through nuget won't know that: they see '7 is the latest, let's use that', and run into this.

Yes, I get that too. Though given that Npgsql 7 was released just two weeks ago, I don't think that's an outrageous state of affairs: the ecosystem does need a bit of time to react. Hopefully it's something that sorts itself out: new people installing the latest of everything don't have an issue (since the latest LLBLGen supports the latest Npgsql), whereas people using an older LLBLGen would maybe also keep using their older Npgsql. The issue is with people trying to mix and match, which is always going to be problematic.

Alright, I'll go for that in this case as I then can save on some string concatenations :) Thanks for the info.

FWIW there are also some edge cases where the internal SQL parser gets it wrong, so using positional parameters avoids that (a database driver shouldn't parse SQL!), that's another reason I felt we should do this.

Sorry again that this change is causing trouble - I mean it. We discussed it and decided unanimously that this specific change is worth it, but it's possible we all tend to that side of the spectrum.

from doc.

Versions of our software that is out there used by developers might be slightly out of support on our side, but still works fine (and is allowed to continue to be used), and can produce code that runs on .net 7 etc. so it's not unreasonable to assume these people will run into this and never look at a table I might add to documentation as their documentation won't change anymore. Hence this is a delegate issue.

The main issue I think is that ado.net providers are used by middleware that is the medium used by the actual developer, so issuing a breaking change in an ado.net provider has far reaching consequences to projects and it trickles down through the middleware.

I must say I rarely run into breaking changes in ado.net providers, if at all. Npgsql is the exception to that :) Might be a thing to consider in the future to not introduce breaking changes but live with it. (tho this particular issue is quite jarring, and doesn't have a right solution)

from doc.

@FransBouma merged some clarifications to the breaking change note based on the discussion above, thanks for reaching out (even when it includes complaining 🤣 )

from doc.