Re: SQL_CURSOR_TYPE prepare execute issue - Mailing list pgsql-odbc

Don't worry about the '-1' test I was just trying to determine, in the server side prepare case, if it was adding
quotesi.e. the string I was supplying already had single quotes and was not accepted. 

I've had a quick look at the new patches.

patch 0001
To be extra extra careful maybe valid_int_literal() should check 'len' before accessing the first byte and second bytes
of'str' in case len is 0 or 1. This may not be necessary it depends on where 'str' comes from. 
Otherwise the patch looks fine but maybe you should add a couple of test cases for SQL_SMALLINT.


patch 0002
In convert.c you have added the comment and line
   /* XXX: should we use param_pgtype here instead? */
   *pgType = sqltype_to_bind_pgtype(conn, param_sqltype);
this really requires someone with more in depth knowledge than I have.
Again maybe a couple of test cases for SQL_SMALLINT.

I have done a few very quick tests myself and have not found anything wrong so far.

Looks like I opened up a can of worms here, oops, but the end result should be more secure and more consistent with
regardsto the server side and client side prepare behaviour, which is good. Also your helping people out by forcing
integercomparison more often. 
I will try to do some more testing next week.

Regards,
Jeremy Faith



________________________________________

From: Heikki Linnakangas [hlinnakangas@vmware.com]
Sent: 15 January 2015 15:58
To: Faith, Jeremy; pgsql-odbc@postgresql.org
Subject: Re: [ODBC] SQL_CURSOR_TYPE prepare execute issue

On 01/15/2015 02:22 PM, Faith, Jeremy wrote:
> As long as you are confident that the test suite is checking this code path that is fine, I just wanted to be sure
thatwas happening. 

Sure, thanks!

> I hadn't thought of
>    select x-?
> You are right that is a problem.
> What does the server prepare code path do?
> I did a quick check, sever side prepare, with the log turned on and it shows the following
>    LOG:  execute <unnamed>: select 1-$1
>    DETAIL:  parameters: $1 = '-1'
>    LOG:  execute <unnamed>: select 1-$1
>    DETAIL:  parameters: $1 = '1'
> But if the bound string is '-1' then I get
>    ERROR:  invalid input syntax for integer: "'-1'"   i.e. double quote,single quote,-,1,single quote,double quote
>    STATEMENT:  select 'wibble',1-$1

Huh. I cannot reproduce that. Can you modify the
test/src/param-conversions-test.c test case to demonstrate it?

> Also get invalid syntax error if it is just a minus sign.
>
> ok, server side prepare log when string is just 6 shows
>    LOG:  execute <unnamed>: select '555'>$1
>    DETAIL:  parameters: $1 = '6'
> and this return one row with a value of 0
> so this is doing select '555'>'6';
>
> So to be consistent with what the server side prepare does, I think it should quote the value.
> You should check this as I may have been getting confused.
> I realise this may have some implications for existing programs but given that sometimes it will be server side
prepareand sometimes client side it is likely to cause more issues if the results are different. 

Ugh, I didn't realize that happens with UseServerSidePrepare=1. But
looking at the code, it's quite obvious that it does; we never send the
datatypes with server-side parameters, so the server always treats them
as "unknown", that is, the same as a plain string literal. That usually
works fine, as the server deduces the right datatype from the context,
but for ambiguous cases like above, it goes wrong.

I think it's pretty clear that the UseServerSidePrepare=1 behaviour is
wrong in this case. In particular, if you bind a parameter as
SQL_INTEGER, and run query "SELECT '555' > ?", the argument should be
treated as an integer and the answer should be "true".

I propose the attached two patches. The first one fixes the "-" case and
negative integers, with UseServerSidePrepare=0. The second one changes
the UseServerSidePrepare=1 behaviour so that a datatype is sent along
with each parameter. String-like datatypes, SQL_CHAR, SQL_WCHAR,
SQL_VARCHAR etc. are still sent as "unknown". It makes sense for string
types IMHO.

Thoughts?
- Heikki
h
Tyco Safety Products/CEM Systems Ltd.

________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you
arenot the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any
actionin respect of any information contained in it. If you have received this e-mail in error, please notify the
senderimmediately by e-mail and immediately destroy this e-mail and its attachments.