Another regressin in Postgres. Case is reverse to described in [#335].
In case from DB point of view parameter is string, but binded as number pg report an errro operator does not exist: text = bigint
I propose to add a strict condition to both TZPostgreSQLPreparedStatementV3.InternalBindDouble & InternalBindInt. This gracefully close #335 and this issue also. Patch:
--- a/src/dbc/ZDbcPostgreSqlStatement.pas +++ b/src/dbc/ZDbcPostgreSqlStatement.pas @@ -1705,7 +1705,7 @@ end; procedure TZPostgreSQLPreparedStatementV3.InternalBindDouble(Index: Integer; SQLType: TZSQLType; const Value: Double); -var PGSQLType: TZSQLType; +var PGSQLType, DBSQLType: TZSQLType; { move the string conversions into a own proc -> no (U/L)StrClear} procedure SetAsRaw; begin @@ -1722,10 +1722,17 @@ var PGSQLType: TZSQLType; SetRawByteString(Index{$IFNDEF GENERIC_INDEX}+1{$ENDIF}, fRawTemp) end; begin + DBSQLType := BindList[Index].SQLType; + if not (DBSQLType in [stBoolean..stTimestamp]) then begin + SetAsRaw; exit; + end; PGSQLType := OIDToSQLType(Index, SQLType); + if not (PGSQLType in [stBoolean..stTimestamp]) then begin + SetAsRaw; exit; + end; if PGSQLType in [stCurrency, stBigDecimal] then SetCurrency(Index{$IFNDEF GENERIC_INDEX}+1{$ENDIF}, Value) - else if (Ord(PGSQLType) < Ord(stGUID)) and Boolean(PGSQLType) then begin + else begin if PGSQLType in [stBoolean, stFloat, stSmall, stInteger, stDate] then begin BindList.Put(Index, PGSQLType, P4Bytes(@Value)); LinkBinParam2PG(Index, BindList._4Bytes[Index], 4); @@ -1748,39 +1755,44 @@ begin then DateTime2PG(Value, PInt64(FPQparamValues[Index])^) else DateTime2PG(Value, PDouble(FPQparamValues[Index])^); end; - end else SetAsRaw; + end; end; procedure TZPostgreSQLPreparedStatementV3.InternalBindInt(Index: Integer; SQLType: TZSQLType; Value: {$IFNDEF CPU64}Integer{$ELSE}Int64{$ENDIF}); -var PGSQLType: TZSQLType; +var PGSQLType, DBSQLType: TZSQLType; { move the string conversions into a own proc -> no (U/L)StrClear} procedure SetAsRaw; begin SetRawByteString(Index{$IFNDEF GENERIC_INDEX}+1{$ENDIF}, IntToRaw(Value)); end; begin + DBSQLType := BindList[Index].SQLType; + if not (DBSQLType in [stBoolean..stCurrency]) then begin + SetAsRaw; exit; + end; PGSQLType := OIDToSQLType(Index, SQLType); - if (Ord(PGSQLType) < Ord(stGUID)) and Boolean(PGSQLType) then begin - if (FPQparamValues[Index] = nil) or (BindList[Index].SQLType <> PGSQLType) then - if ZSQLType2PGBindSizes[PGSQLType] <= 4 then begin - BindList.Put(Index, PGSQLType, P4Bytes(@Value)); - LinkBinParam2PG(Index, BindList._4Bytes[Index], ZSQLType2PGBindSizes[PGSQLType]); - end else if ZSQLType2PGBindSizes[PGSQLType] = 8 then begin - BindList.Put(Index, PGSQLType, P8Bytes({$IFNDEF CPU64}@faBuffer[0]{$ELSE}@Value{$ENDIF})); - LinkBinParam2PG(Index, BindList._8Bytes[Index], 8); - end else - LinkBinParam2PG(Index, BindList.AquireCustomValue(Index, PGSQLType, - ZSQLType2PGBindSizes[PGSQLType]), ZSQLType2PGBindSizes[PGSQLType]); - case PGSQLType of - stBoolean: PByte(FPQparamValues[Index])^ := Ord(Value); - stSmall: SmallInt2PG(Value, FPQparamValues[Index]); - stInteger, - stDate: Integer2PG(Value, FPQparamValues[Index]); - stFloat: Single2PG(Value, FPQparamValues[Index]); - stLongWord: Cardinal2PG(Value, FPQparamValues[Index]); - stLong: Int642PG(Value, FPQparamValues[Index]); - stDouble: Double2PG(Value, FPQparamValues[Index]); - stCurrency: Currency2PGNumeric(Value, FPQparamValues[Index], FPQparamLengths[Index]); - end; - end else SetAsRaw; + if not (PGSQLType in [stBoolean..stCurrency]) then begin + SetAsRaw; exit; + end; + if (FPQparamValues[Index] = nil) or (DBSQLType <> PGSQLType) then + if ZSQLType2PGBindSizes[PGSQLType] <= 4 then begin + BindList.Put(Index, PGSQLType, P4Bytes(@Value)); + LinkBinParam2PG(Index, BindList._4Bytes[Index], ZSQLType2PGBindSizes[PGSQLType]); + end else if ZSQLType2PGBindSizes[PGSQLType] = 8 then begin + BindList.Put(Index, PGSQLType, P8Bytes({$IFNDEF CPU64}@faBuffer[0]{$ELSE}@Value{$ENDIF})); + LinkBinParam2PG(Index, BindList._8Bytes[Index], 8); + end else + LinkBinParam2PG(Index, BindList.AquireCustomValue(Index, PGSQLType, + ZSQLType2PGBindSizes[PGSQLType]), ZSQLType2PGBindSizes[PGSQLType]); + case PGSQLType of + stBoolean: PByte(FPQparamValues[Index])^ := Ord(Value); + stSmall: SmallInt2PG(Value, FPQparamValues[Index]); + stInteger, + stDate: Integer2PG(Value, FPQparamValues[Index]); + stFloat: Single2PG(Value, FPQparamValues[Index]); + stLongWord: Cardinal2PG(Value, FPQparamValues[Index]); + stLong: Int642PG(Value, FPQparamValues[Index]); + stDouble: Double2PG(Value, FPQparamValues[Index]); + stCurrency: Currency2PGNumeric(Value, FPQparamValues[Index], FPQparamLengths[Index]); + end; end;
Diff:
IIRC you want that all parameters are bound as String, correct? If so that's not what i want..
Do you have this regression with the mORMot TSQLRecod classes or did you bound the values by hand with the IZPreparedStatements?
Could you provide a little example to show me what's going on?
Last edit: EgonHugeist 2019-03-15
I have this regression with SynDB
In Zeos 7.2 all work as expected in this example.
So - I want InternalBindDouble & InternalBindInt to use binary format only if type of bound value (PGSQLType) can be casted to the type of parameter exctacted from execution plane (DBSQLType). If it can - use binary format, if not - string.
We can use binaty format in InternalBindInt only in case
(DBSQLType in [stBoolean..stCurrency] ) AND (PGSQLType in [stBoolean..stCurrency])
in other case use string.
For double to use binary both type should be
in [stBoolean..stTimestamp]
I made my decision.. It took a while, sorry.
Solutions:
1. (prefered) plz fix your business logic mistakes
2. (also prefered) raise a ticked on the pg bugtracker. IMHO a conversion from an ordinal value to a text value should always be possilbe.
3. reopen a second ticked as a feature request. We could improve your regressions with a property to have a fallback to the slow deprecated behavior.
Why i made my decison: I just want fast bindings with less mem-allocs as possible . Other team members are also against all the conversion zeos makes which mostly do cost to much code. See https://sourceforge.net/p/zeoslib/tickets/281/ . Therefore an new branch has been made -> obsolete behavior need to be refactored.
To be clear: there are other types of stmts where the "plan"(you're talking about) never comes to shove. But binary bindings are welcome. Zeos currently does prepare and ask for paramtypes only (in case of PG) if the request is kind of: select/insert/update/delete. All other "undetectable" queries are handled with pgexecparams see https://www.postgresql.org/docs/9.1/libpq-exec.html.
Sorry, won't fix. Hope you agree?
After digging deeper into zeos sources I agree with you. Query do not always "prepared". I think I try to solve propblem with parameters on my side