Redshift does not support startup parameter LC_MONETARY

[griffoff]

It seems that Amazon redshift does not support some of the functionality expected by postgres connections. When connecting to an SSL enabled cluster with the 2.1.0.beta1 driver the following error occurs:

permission denied to set parameter "lc_monetary" to "C"

I would like to request a feature whereby a connection string option can be set to specify that the connection is to redshift and subsequently decisions can be made in the code to avoid certain nuances with the implementation of the postgres-like schema.

[franciscojunior]

When this error occurs, are you able to continue connecting and use the server ?

[franciscojunior]

We need better handling of those cases.
Until now, the only reported problem was about the lc_monetary. Maybe we need to find another way to handle monetary values instead of relying on changing runtime configuration value.

Stackoverflow also has a question about that: http://stackoverflow.com/questions/20969365/permission-denied-to-set-parameter-lc-monetary-to-c

[griffoff]

No. The error stops the connection as it throws an error within the open method.

[griffoff]

I agree. An inability to set a parameter should not stop the connection.
But also this is an expected parameter in postgres so it is entirely
reasonable to expect it to be there.

I do appreciate that Redshift is an edge case and could potentially become
a branch to the driver rather than being accommodated in a over size fits
all driver.
On 12 Feb 2014 20:54, "Francisco Figueiredo Jr." notifications@github.com
wrote:

We need better handling of those cases.
Until now, the only reported problem was about the lc_monetary. Maybe we
need to find another way to handle monetary values instead of relying on
changing runtime configuration value.

Stackoverflow also has a question about that:
http://stackoverflow.com/questions/20969365/permission-denied-to-set-parameter-lc-monetary-to-c

Reply to this email directly or view it on GitHubhttps://github.com//issues/163#issuecomment-34916622
.

[mateusaubin]

I wrote some custom code to avoid two issues with Redshift. You can see it on my fork.

I could create a pull request if you guys find it usefull.

The issues I've found were:

• LC_MONETARY;
• varchar fields prefixed with E (i.e: E'string');

[khalidsalomao]

This issue was introduced in the versions +2.1.
Last version that worked fine is 2.0.14.3.

[Heucles]

Thanks @khalidsalomao for the tip. I am looking forward to have this bug fixed. Regardless, @franciscojunior great job with the lib, I'm starting to use, but I can already consider myself a big fan.

[franciscojunior]

Thank you for your feedback, @Heucles ! We are working in a series of changes in Npgsql. I hope Amazon create a new version of Redshift based on Postgresql 9.x. This would ease very much our lives :)

[roji]

@franciscojunior, so if I understand correctly, RedShift is based on Postgresql 8.0.2, which doesn't yet support the LC_MONETARY parameter, right?

8.0.2 is a truly ancient version: released in 2005 and reached EOL in 2010. I'm really surprised Amazon based Redshift on it, and a quick search doesn't show any plans on supporting newer Postgresql features. Npgsql supports the same versions Postgresql officially supports, so 9.0 and up. However, since Redshift is an important service it's tempting to try and maintain support if it's not too hard.

Are we aware of any other Npgsql issues with Redshift apart from LC_MONETARY? If that's all we can definitely try to check for Redshift (or simply the Postgresql backend version) and not send this parameter on startup.

@franciscojunior, what do you think? If so I will also undo the legacy cleanup I performed in #329.

[khalidsalomao]

Hi Guys,
I have been using Redshift for the last 6 months and the version before the
legacy cleanup works fine!

On Saturday, August 30, 2014, Shay Rojansky notifications@github.com
wrote:

@franciscojunior https://github.com/franciscojunior, so if I understand
correctly, RedShift is (based on Postgresql 8.0.2)[
http://docs.aws.amazon.com/redshift/latest/dg/c_redshift-and-postgres-sql.html],
which doesn't yet support the LC_MONETARY parameter, right?

8.0.2 is a truly ancient version: released in 2005 and reached EOL in
2010. I'm really surprised Amazon based Redshift on it, and a quick search
doesn't show any plans on supporting newer Postgresql features. Npgsql
supports the same versions Postgresql officially supports, so 9.0 and up.
However, since Redshift is an important service it's tempting to try and
maintain support if it's not too hard.

Are we aware of any other Npgsql issues with Redshift apart from
LC_MONETARY? If that's all we can definitely try to check for Redshift (or
simply the Postgresql backend version) and not send this parameter on
startup.

@franciscojunior https://github.com/franciscojunior, what do you think?
If so I will also undo the legacy cleanup I performed in #329
#329.

—
Reply to this email directly or view it on GitHub
#163 (comment).

[franciscojunior]

Hi, @khalidsalomao . Which version before the legacy cleanup is working for you? Are you talking about the 2.0.14, 2.1.x or some custom built from git?

Are we aware of any other Npgsql issues with Redshift apart from LC_MONETARY? If that's all we can definitely try to check for Redshift (or simply the Postgresql backend version) and not send this parameter on startup.

I think there is the E' ' construction support which redshift doesn't support, according to @mateusaubin

[roji]

@franciscojunior, I think we already have a check for E' ' compatibility, I hope it means everything is OK with Redshift.

Like you say, once @khalidsalomao confirms which version was the last to work well we can check what happened. If it's only a matter of adding a version condition around the LC_MONETARY startup parameter, it's pretty trivial.

[franciscojunior]

@franciscojunior, I think we already have a check for E' ' compatibility, I hope it means everything is OK with Redshift.

Whooops, you are right! I was basing my comment in the @mateusaubin 's fork. But the code has already been modified. Thanks for the heads up! :)

Like you say, once @khalidsalomao confirms which version was the last to work well we can check what happened. If it's only a matter of adding a version condition around the LC_MONETARY startup parameter, it's pretty trivial.

Yes. But we will need to remove its initialization from the startup package because the startup package is sent before any query can be made.

Another option is to study a way of handling monetary type without needing to change the lc_monetary startup parameter. This change is similar to the extra_float_digits. I use it in order to get a consistent result from the server regarding negative values and decimal point. I noticed that in some locales, the negative monetary value is returned inside parenthesis while in others, it is returned with a leading minus signal.

[khalidsalomao]

@franciscojunior the version that we're using is the 2.0.14. We've been
using it heavily and so far it has been working fine.

On Sat, Aug 30, 2014 at 3:27 PM, Francisco Figueiredo Jr. <
notifications@github.com> wrote:

@franciscojunior https://github.com/franciscojunior, I think we already
have a check for E' ' compatibility, I hope it means everything is OK with
Redshift.

Whooops, you are right! I was basing my comment in the @mateusaubin
https://github.com/mateusaubin 's fork. But the code has already been
modified. Thanks for the heads up! :)

Like you say, once @khalidsalomao https://github.com/khalidsalomao
confirms which version was the last to work well we can check what
happened. If it's only a matter of adding a version condition around the
LC_MONETARY startup parameter, it's pretty trivial.

Yes. But we will need to remove its initialization from the startup
package because the startup package is sent before any query can be made.

Another option is to study a way of handling monetary type without needing
to change the lc_monetary startup parameter. This change is similar to the
extra_float_digits. I use it in order to get a consistent result from the
server regarding negative values and decimal point. I noticed that in some
locales, the negative monetary value is returned inside parenthesis while
in others, it is returned with a leading minus signal.

—
Reply to this email directly or view it on GitHub
#163 (comment).

[franciscojunior]

@franciscojunior the version that we're using is the 2.0.14. We've been using it heavily and so far it has been working fine.

Thanks for the information, @khalidsalomao ! We will check how to make it work again on latest versions.

[InsidiousForce]

More people using redshift I guess. The version I pulled from nuget today has this issue still. (2.2.1) - solution is to still manually patch this? thanks

[roji]

Yes, 2.2.1 still has this issue.

We do intend to fix this, but since it introduces breakage it probably won't happen before 3.0...

[mateusaubin]

There's nothing related to Npgsql in my GAC, I'm referencing the dll directly.
I ran my test using DbProviderFactories but now I've also run them by directly instantiating NpgsqlConnection and got the same results.

Where can I find the code with your patches so I can build it locally?

[franciscojunior]

You can find it here:
https://github.com/npgsql/npgsql/tree/2.2

And patch here:
#487

Thanks for testing.

[mateusaubin]

I think i've sorted it out.

the problem was:

• I hadn't set AmazonRedshift=true;
• NpgsqlConnector attempts to initialize the state sending a NpgsqlStartpPacket which relies on the connectionstring parameter
• Exception is throw due to LC_MONETARY being set before even asking the db version and verifying whether it's running redshift or not

also I think that relying on version detection and also defining a connectionstring parameter is a little too much. if the user is explicitly defining a parameter saying it's redshift the library should simply comply.
or the library could completely automate detection through version(), either one or the other way, not both.

I'd prefer the connectionstring parameter and avoid any detection, which could have to change in future versions due to changes in redshift. since redshift is sold like a service and has frequent (and automatic) updates such change would be a major concern for anyone relying on Npgsql.

[franciscojunior]

also I think that relying on version detection and also defining a connectionstring parameter is a little too much.

Ahh, sorry about that. There are 2 ways of handling the problem of the lc_monetary. One was through the connection string parameter which is already committed in the 2.2 branch. The other one was checking the return value of select version(). I didn't remove the connection string parameter support yet. :( Sorry for not making this clear.

But on the other side, did it work to connect to Amazon redshift ? Did you find any other problem?
Another side effect of this change is that money support is broken in Amazon redshift. Are you using this datatype in your projects?

I'd prefer the connectionstring parameter and avoid any detection, which could have to change in future versions due to changes in redshift. since redshift is sold like a service and has frequent (and automatic) updates such change would be a major concern for anyone relying on Npgsql.

Hmmmm, I see. I think both ways have pro and cons. The connection string parameter has the con of asking users to change the connection string parameter and the dynamic checking has the cons you said also the performance penalty everyone else would be paying.

@roji , what do you think?

[mateusaubin]

But on the other side, did it work to connect to Amazon redshift ? Did you find any other problem?
Another side effect of this change is that money support is broken in Amazon redshift. Are you using this datatype in your projects?

Yes, it worked just fine.
Regarding the money datatype we haven't used it since it is explicitly unsupported by redshift, see Unsupported PostgreSQL Data Types and Data Types.

Attempting to create a table with a money column leads to an error:

ERROR: 0A000: Column "#testtable.lotsof" has unsupported type "money".

[roji]

@franciscojunior, I'd implement SupportsSetLcMonetary as a check for server_version >= 8.1.0. Then I'd check this property in NpgsqlConnector.Open() and add LC_MONETARY='C' to sbInitQueries just like we currently do with SupportsExtraFloatDigits3.

[franciscojunior]

@franciscojunior, I'd implement SupportsSetLcMonetary as a check for server_version >= 8.1.0. Then I'd check this property in NpgsqlConnector.Open() and add LC_MONETARY='C' to sbInitQueries just like we currently do with SupportsExtraFloatDigits3.

Done! Sorry for taking so long.

I just created a pull request with those changes. #489

I'll now revert the change which add the connection string parameter support.

[roji]

Great! I've triggered the 2.2. build for your PR, hopefully it doesn't do any more slowdown issues.

[franciscojunior]

Great! I've triggered the 2.2. build for your PR, hopefully it doesn't do any more slowdown issues.

I triggered the net 2.0 build just to make a test and the slowdown doesn't occur there.
But in the 487 it still occurs.
Right now I can see the builds happening in the 4 and a half minutes time frame. If we trigger the 487 pull request build, we can see that it occurs in the 17 minutes timeframe. :(

[franciscojunior]

I just reverted the pull request (#477) which added amazon redshift connection string parameter. Github interface worked like a charm! (#490) :)

I'll rebase #489 with this new 2.2.

[roji]

Great. I think we can forget about #487 and merge #489, that should do it.

[franciscojunior]

Yes. There is only a missing line in #489 . I'm committing it now....

[franciscojunior]

Done! Now #489 is 100%.

[franciscojunior]

All set!
@mateusaubin , @khalidsalomao you can download latest 2.2 binaries with latest amazon redshift support here: https://build.npgsql.org/viewLog.html?buildId=14051&tab=artifacts&buildTypeId=npgsql_All22

Please, give it a try and let us know if you find any issues. Thanks in advance.

[khalidsalomao]

Hi @franciscojunior, @roji ,

It worked! 😄

Tested several operations and all is fine!

Thanks guys!

[roji]

Great. @franciscojunior, go ahead and merge #489 into 2.2. Then we can see if there's anything else we want to do for 2.2.5.

[franciscojunior]

Great. @franciscojunior, go ahead and merge #489 into 2.2. Then we can see if there's anything else we want to do for 2.2.5.

Done!

[ReinsBrain]

This issue comes back again when you use pgbouncer ( which cannot support startup parameters for various reasons ). If I may weigh in on the matter, I don't think the driver should be setting parameters - please let the users do that :)

[roji]

@ReinsBrain this issue is 5 years old, which means I've been maintaining Npgsql for too long :)

More importantly though, Npgsql hasn't been sending LC_MONETARY for several years now - which exact parameter incompatibility are you seeing with pgbouncer? For reference, here's the code which sends the parameters on startup.