Menu

#336 Postgres regression (Bin binding reverse)

7.3-alfa
wont-fix
None
2019-03-19
2019-03-14
Pavel Mash
No

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;

Related

Bugs: #335

Discussion

  • Pavel Mash

    Pavel Mash - 2019-03-14
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -4,7 +4,6 @@
     I propose to add a strict condition to both TZPostgreSQLPreparedStatementV3.InternalBindDouble & InternalBindInt. This gracefully close  #335 and this issue also. Patch:
    
     ```
    -index 4ddc3ccb..7ee8fdd4 100644
     --- a/src/dbc/ZDbcPostgreSqlStatement.pas
     +++ b/src/dbc/ZDbcPostgreSqlStatement.pas
     @@ -1705,7 +1705,7 @@ end;
    @@ -103,8 +102,4 @@
     +    stCurrency: Currency2PGNumeric(Value, FPQparamValues[Index], FPQparamLengths[Index]);
     +  end;
      end;
    - 
    - {**
    --- 
    -
     ```
    
     
  • EgonHugeist

    EgonHugeist - 2019-03-15

    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
  • Pavel Mash

    Pavel Mash - 2019-03-15

    I have this regression with SynDB

    var Query: ISQLDBStatement;
    Query := conn.NewStatementPrepared('select eq_id, eq_name from equipment where eq_name=?'); 
    Query.Bind(0, 100200); // businnes login mistake, int instead of string
    Query.ExecutePrepared; // postgres error "operator does not exist: text = bigint"
    

    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]

     
  • EgonHugeist

    EgonHugeist - 2019-03-18
    • status: open --> wont-fix
    • assigned_to: EgonHugeist
     
  • EgonHugeist

    EgonHugeist - 2019-03-18

    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?

     
  • Pavel Mash

    Pavel Mash - 2019-03-19

    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

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.