error 22021 - Npgsql can not determine parameter types for NpgsqlCommand.Prepare(). about npgsql HOT 20 CLOSED

Hi, Glen! @glenebob . Can you have a look at this issue? I can confirm this problem in my tests. Thanks in advance.

from npgsql.

Not sure why we need to make up error messages that bear no resemblance to the real one... This is what I get:

ERROR: 22021: invalid byte sequence for encoding "UTF8": 0x00.

from npgsql.

Hmmmm, I was playing with #125 and made a drastic change in NpgsqlParameter to return false for the UseCast property. When I run our testsuite, I could see a lot of tests failing with a message about not being able to determine the parameter type.

from npgsql.

This problem can be gotten around nicely by setting the type explicitly on the parameter (to NpgsqlDbType.Integer).

from npgsql.

Npgsql's parameter system has some serious issues. I was working on a rewrite some months ago (there's a PR work in progress), and I was never able to quite get it right; I couldn't get all the tests to pass. I never absorbed the type system sufficiently to understand it completely.

But there's no way I can devote enough time to it now to finish it, and I see no simple path at all to get it all working properly. Until someone can spend the time to rewrite the parameter type coercion code, it's just not going to work 100% of the time.

-Glen

from npgsql.

Thanks for having a look at this, Glen!
I agree with you about the Npgsql type system. It is really very ugly. I'll have a look at this.

from npgsql.

I think this problem is the same as this one: https://groups.google.com/forum/#!topic/npgsql-help/OAdtmKfCQ_o

from npgsql.

Ok, I found the culprit: when the parameter value is set, the useCast property isn't set to be used. It keeps its default value of false.

I think this somehow gives problem to the prepared statement logic of Npgsql.

I didn't check the code yet, but I think it expects to receive from the server the type of the pareameter when doing the bind. As the type isn't received, Npgsql seems to be sending null (0x00).

After I added a useCast = true to the setValue property of NpgsqlParameter, the test code given by @udoliess worked again.

That's why setting the DbType, as suggested by @glenebob, also made it work. It sets the useCast property to true too.

The problem is that after this change, one test started to fail in our testsuite:

NpgsqlTests.CommandTests("9.3").ParameterExplicitType2:
Npgsql.NpgsqlException : ERROR: 42883: operator does not exist: date = text

This is the command sent to server after the change when running this test:

LOG:  statement: SELECT * FROM data WHERE field_date=(('2008-1-1')::text)
ERROR:  operator does not exist: date = text at character 36
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.
STATEMENT:  SELECT * FROM data WHERE field_date=(('2008-1-1')::text)

This is caused by the fact that Npgsql has no way of knowing this field will be of type date and then it uses the default datatype which is text. But this test should work, Npgsql is the one which isn't being able to handle it correctly.

[Test]
        public void ParameterExplicitType2()
        {
            using (var command = new NpgsqlCommand(@"SELECT * FROM data WHERE field_date=:param", Conn))
            {
                var sqlParam = command.CreateParameter();
                sqlParam.ParameterName = "param";
                sqlParam.Value = "2008-1-1";
                //sqlParam.DbType = DbType.Object;
                command.Parameters.Add(sqlParam);
                command.ExecuteScalar();
            }
        }

So, it seems that if we don't set the parameter type, like @udoliess is doing, thus don't using the casting, we have problems with our prepare logic.

If we do use cast when setting the value, we end up with problems like the test above which is expecting that we don't send any cast at all and would be the job of postgresql to interpret the type of the value.

This makes me think about some reports I got in the past that Npgsql should not add casts and let the server interpret the value. I think this would give Npgsql a lot of flexibility to handle new types, like for example json which we have a request for: #168 . According to the documentation, json can be used just like text but as Npgsql doesn't know about json, it adds the cast to text and postgresql complains. The workaround done by the reporter was to change using reflection (as it is internal) the property useCast to false.

from npgsql.

Npgsql's parameter system has some serious issues. I was working on a rewrite some months ago (there's a PR work in progress), and I was never able to quite get it right; I couldn't get all the tests to pass. I never absorbed the type system sufficiently to understand it completely.

I checked your PR #102 and could pull a fix from it regarding the "text_nonbinary" type name. By changing it to "unknown" as your patch does, fixes #201 .

I'm going to check your other fixes and in the PR.

I'll keep this issue updated about what I find.

from npgsql.

Here is what I found so far:

As the parameter doesn't have its dbtype specified, the useCast keeps its value to false and then Npgsql sends the following command to prepare:

parse s1: SELECT CAST(($1) AS text)

Note that there is no type specified in the parameter.

This way, Postgresql tried to parse the value sent as text (the default type for unspecified types) and thus complains about the 0x00 byte sent. (int4 values like 1 are being sent as 0x00 0x01).

You can confirm that this is the case if you change the parameter value to "test". As it is a text, it works ok.

If you specify the dbtype by hand, the useCast is set and then Npgsql sends the following code:

parse s3: SELECT CAST(($1::int4) AS text)

This way, postgresql knows it should expect an int4 and thus doesn't complain.

The bug is that when we specify a parameter value we aren't changing the useCast value to true.

Another way to fix this is to use the support inside the Parse message where we can specify the type of the parameters.

NpgsqlParse parse = new NpgsqlParse(planName, preparedCommandText, new Int32[] { });

The last parameter is an array of the types oid of the parameters. Npgsql never used this option. But I think we would need to add support for this when we start to work to remove casts from Npgsql. This will be discussed in #125 .

I'll work in a fix for the useCast so this works again.

from npgsql.

I just confirmed that setting the parameter oid in the parse step makes the code works again without need for casts.

from npgsql.

This is an initial work to add support to oids in parse step of prepared statements:

In private void PrepareInternal()

            var parameterOids = new List<int>();
            var oids = Connector.OidToNameMapping;

            // Set parameter types of the query.
            foreach (NpgsqlParameter parameter in Parameters)
            {
                if (parameter.Direction == ParameterDirection.Input || parameter.Direction == ParameterDirection.InputOutput)
                    parameterOids.Add(oids[parameters[parameter.CleanName].TypeInfo.Name].OID);
            }

            NpgsqlParse parse = new NpgsqlParse(planName, preparedCommandText, parameterOids.ToArray());

I'm going to work in a proper pull request for it.

from npgsql.

I'm looking forward to a pull request. I'd like to see how this plays out!

I just got done merging the < operator and comment fixes into the 2,1
branch. All tests pass. I think we can look at doing a 2.1.3 release :)

-Glen

On Fri, Apr 4, 2014 at 3:08 PM, Francisco Figueiredo Jr. <
[email protected]> wrote:

This is an initial work to add support to oids in parse step of prepared
statements:

        var parameterOids = new List<int>();
        var oids = Connector.OidToNameMapping;

        // Set parameter types of the query.
        foreach (NpgsqlParameter parameter in Parameters)
        {
            if (parameter.Direction == ParameterDirection.Input || parameter.Direction == ParameterDirection.InputOutput)
                parameterOids.Add(oids[parameters[parameter.CleanName].TypeInfo.Name].OID);
        }

        NpgsqlParse parse = new NpgsqlParse(planName, preparedCommandText, parameterOids.ToArray());

I'm going to work in a proper pull request for it.

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

from npgsql.

One thing I noticed is that although this initial code works, it would need to handle the situation where the parameters are out of order.

For example, in a query like:

select :param2, :param1 

if I add param1 and then param2 to the NpgsqlCommand.Parameters collection, the list of parameters will be in the wrong order.

I think I'll need to elaborate it to fill the parameterOids list when the parameters are found in the query string. This way, the parsing would find param2 before the param1 and the parameteOids list would be filled correctly.

I'll work on it and will let you know what I get.

Regarding the 2.1.3 release, I think you are right. Let's do it! :)

from npgsql.

@glenebob and @franciscojunior, what's the status of this issue? Do you think you guys will have time to work on it for 3.0?

from npgsql.

I'll have a look at it.

From my initial idea, I think I'll need to add code to the lexer in order to find the parameters and its datatypes to fill the parameteOids list in the order they appear in the query. My dirty POC above fills it in the order they were provided in the parameters collection which is not right. :)

from npgsql.

When parsing the query, PostgreSQL tries to find out types for all parameters. It does this in the following order of precedence:

1. By reading the list of OIDs sent in the Parse message.
2. Looking at explicit cast expressions (cast(value as type) or value::type) with a parameter as input.
3. Trying to find out a type by looking at the context the parameter is in, for example assigning a table column, or comparing values.

If it didn't find a non-unknown type, the error message "could not determine data type for parameter $X" is thrown.

A common case when this happens is for queries like "Select $1", and a type wasn't specified in the Prepare message.

The response of the Describe message is a Parameter Description, which contains the data types for each parameter the backend has chosen. It is these data types the backend expects when it receives the Bind message. Npgsql could read this to be sure to send data in the correct format. Now it ignores this message (https://github.com/npgsql/Npgsql/blob/596c5d08c0000f4129ee6ea4ea615ea1712f1a99/Npgsql/Npgsql/NpgsqlConnector.cs#L777).

In my opinion, I think the correct way of handling parameters is to always by default cast the parameter value to the correct type (either by putting the OID in the parameter list of the Parse message, or writing an explicit cast in the query), except when the user has put a string as parameter value and at the same time has not assigned an explicit type for the parameter. That way we solve these problems:

• We can continue to support unknown types, where the user can insert a literal string and let the database infer the type depending on context, json for example.
• When a parameter type is something else than a string, the database will never treat is wrongly.

One problem might however be if there are multiple .NET types that correspond to multiple PostgreSQL types, or vice versa, but that probably? still works because PostgreSQL has implicit casting between compatible data types, for example, date and timestamp.

If we do this (which means values set as unknown will always be literal strings), we don't have to check the Parameter Description message, since unknown values will be sent as text and we know the type of everything else.

What do you think?

from npgsql.

The response of the Describe message is a Parameter Description, which contains the data types for each parameter the backend has chosen. It is these data types the backend expects when it receives the Bind message. Npgsql could read this to be sure to send data in the correct format. Now it ignores this message (https://github.com/npgsql/Npgsql/blob/596c5d08c0000f4129ee6ea4ea615ea1712f1a99/Npgsql/Npgsql/NpgsqlConnector.cs#L777).

As we moved to using explicit casts before adding support for protocol 3, we ended up not using this support when it appeared.

In my opinion, I think the correct way of handling parameters is to always by default cast the parameter value to the correct type (either by putting the OID in the parameter list of the Parse message, or writing an explicit cast in the query), except when the user has put a string as parameter value and at the same time has not assigned an explicit type for the parameter. That way we solve these problems:

Agreed. Today we try to set the dbtype and npgsqldbtype when the user sets the parameter value. I noticed though that sqlclient doesn't set the dbtype and sqldbtype when the parameter is set. I think it does something like what you said.
Sqlclient has the variant sqldbtype which is the default sqldbtype when creating a new sqlparameter. Maybe we could create a Unknown Npgsqldbtype parameter to mark parameters which didn't have its parameter explicitly set by user.

One problem might however be if there are multiple .NET types that correspond to multiple PostgreSQL types, or vice versa, but that probably? still works because PostgreSQL has implicit casting between compatible data types, for example, date and timestamp.

The only issue I found was in the case of function parameters. Postgresql wasn't able to infer some types without explicit cast.
We have a sample of this in the ambiguousParameterType test case: https://github.com/npgsql/Npgsql/blob/master/tests/CommandTests.cs#L1485

Without casts, this is what you get when running the function:

npgsql_tests=# select * from ambiguousParameterType(2,2,2,'test', 'test', 'test');
ERROR:  function ambiguousparametertype(integer, integer, integer, unknown, unknown, unknown) does not exist
LINE 1: select * from ambiguousParameterType(2,2,2,'test', 'test', '...
                      ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

Not that it tries to convert the value 2 to integer. The call only succeeds when you add the casts:

npgsql_tests=# select * from ambiguousParameterType(2::int2,2::int4,2::int8,'test', 'test', 'test');
 ambiguousparametertype 
------------------------
                      4
(1 row)

If we do this (which means values set as unknown will always be literal strings), we don't have to check the Parameter Description message, since unknown values will be sent as text and we know the type of everything else.

After read that: "(which means values set as unknown will always be literal strings)",
I tried it to this specific case and it indeed works:

npgsql_tests=# select * from ambiguousParameterType('2','2','2','test', 'test', 'test');
 ambiguousparametertype 
------------------------
                      4
(1 row)

I think it would work very well. And it would bring back the flexibility to use Npgsql with any type, as you said.

I think this would also fix the problem I spotted before about the prepared statements (postgresql was trying to interpret a integer as an utf-8 string because we didn't said which type the parameter was and Postgresql assumed it was string), because as the value would be sent as string, Postgresql would parse it correctly.

from npgsql.

After read that: "(which means values set as unknown will always be literal strings)",

Yeah, for those parameters we put unknown in the type oid description, we could assume sending it as text will always work.

I think this would also fix the problem I spotted before about the prepared statements (postgresql was trying to interpret a integer as an utf-8 string because we didn't said which type the parameter was and Postgresql assumed it was string), because as the value would be sent as string, Postgresql would parse it correctly.

Yep. More detailed: it sees the cast to text and therefore thinks the parameter is of type text. It then sends a Parameter Description that indicates an OID of the text data type. Then Npgsql sends the integer value 1 as binary and boom.

from npgsql.

This issue is no longer relevant for v3, where the entire parameter and type handling has been rewritten.

Note that in v3 prepared statements must have all their parameters' types explicitly set with NpgsqlDbType or DbType, and not type-inferred. This seems to be the standard ADO.NET behavior and is the SqlClient. So the code sample originally posted above wouldn't work (although for very different reasons).

from npgsql.