UncachedGetColumns and numeric/decimal

[abonic]

BUG:

function TZInterbase6DatabaseMetadata.UncachedGetColumns (from unit ZDbcInterbase6Metadata) recognize numeric/decimal columns only if the field is 8 bytes long. In other cases the column is reported as smallint or integer.

TEST:

Code: Select all

procedure TForm1.GetMeta;
var
  rs: IZResultSet;
begin
  rs := ZConnection1.DbcConnection.GetMetadata.GetColumns('', '', 'NUMTEST', '');
  while rs.Next do
    Memo1.Lines.Add( Format('%s : %s (%s, %s)', [
      rs.GetString(ColumnNameIndex),
      rs.GetString(TableColColumnTypeNameIndex),
      rs.GetString(TableColColumnSizeIndex),
      rs.GetString(TableColColumnDecimalDigitsIndex)
    ]));
end;

before fix :

N51 : INTEGER (0, 1) // col : type (precision, scale)
N41 : SMALLINT (0, 1)
D51 : INTEGER (0, 1)

after fix:

N51 : NUMERIC (5, 1)
N41 : NUMERIC (4, 1)
D51 : DECIMAL (5, 1)

FIX:

Code: Select all

function TZMsSqlDatabaseMetadata.UncachedGetColumns  
...
(* bug fix start, line 1794 in the original code *)
        // TYPE_NAME
        case TypeName of
          7  : Result.UpdateString(TableColColumnTypeNameIndex, 'SMALLINT');
          8  : Result.UpdateString(TableColColumnTypeNameIndex, 'INTEGER' );
          37 : Result.UpdateString(TableColColumnTypeNameIndex, 'VARCHAR'); // Instead of VARYING
        else
            Result.UpdateString(TableColColumnTypeNameIndex, GetString(ColumnIndexes[8]));
        end;		
	if (TypeName in [7, 8, 16]) then 
          if (SubTypeName = 1) then
            Result.UpdateString(TableColColumnTypeNameIndex, 'NUMERIC')
		else if (SubTypeName = 2) then
            Result.UpdateString(TableColColumnTypeNameIndex, 'DECIMAL');
	// COLUMN_SIZE.
        case TypeName of
          7, 8, 16: Result.UpdateInt(TableColColumnSizeIndex, GetInt(ColumnIndexes[9]));
          37, 38: Result.UpdateNull(TableColColumnSizeIndex);  //the defaults of the resultsets will be used if null
        else
          Result.UpdateInt(TableColColumnSizeIndex, GetInt(ColumnIndexes[10]));
        end;		
(* bug fix end *)

(* old code, replaced by the bug fix 
        // TYPE_NAME
        case TypeName of
          7  : Result.UpdateString(TableColColumnTypeNameIndex, 'SMALLINT');
          8  : Result.UpdateString(TableColColumnTypeNameIndex, 'INTEGER' );
          16 :
            begin
              if (SubTypeName = 0) then
                Result.UpdateString(TableColColumnTypeNameIndex, GetString(ColumnIndexes[8]));
              if (SubTypeName = 1) then
                Result.UpdateString(TableColColumnTypeNameIndex, 'NUMERIC');
              if (SubTypeName = 2) then
                Result.UpdateString(TableColColumnTypeNameIndex, 'DECIMAL');
            end;
          37 : Result.UpdateString(TableColColumnTypeNameIndex, 'VARCHAR'); // Instead of VARYING
        else
            Result.UpdateString(TableColColumnTypeNameIndex, GetString(ColumnIndexes[8]));
        end;        
		// COLUMN_SIZE.
        case TypeName of
          7, 8 : Result.UpdateInt(TableColColumnSizeIndex, 0);
          16   : Result.UpdateInt(TableColColumnSizeIndex, GetInt(ColumnIndexes[9]));
          37, 38: Result.UpdateNull(TableColColumnSizeIndex);  //the defaults of the resultsets will be used if null
            {if ( ConSettings.ClientCodePage.ID = 0 ) then //CharcterSet 'NONE'
              Result.UpdateInt(TableColColumnSizeIndex, GetFieldSize(SQLType, ConSettings,
                GetInt(ColumnIndexes[10]), GetConnection.GetIZPlainDriver.ValidateCharEncoding(SubTypeName).CharWidth, nil, True)) //FireBird return Char*Bytes for Varchar
            else
              Result.UpdateInt(TableColColumnSizeIndex, GetFieldSize(SQLType, ConSettings,
                GetInt(ColumnIndexes[10]), ConSettings.ClientCodePage.CharWidth, nil, True)); //FireBird return Char*Bytes for Varchar}
        else
          Result.UpdateInt(TableColColumnSizeIndex, GetInt(ColumnIndexes[10]));
        end;
*)

[marsupilami]

Hello abonic,

thank you for the patch. I hit that problem in the past too but decided to not (yet) debug the issue. I will add your patch but curently I have to admint that I will probably modify it because I dislike the way that some things happen in the case statement but not always there because the decisions that were done there might be revised late in the code and ... Maybe it is because it is late in the evening but I want to make things more clear and easy to understand.

With best regards,

Jan

[marsupilami]

Hello abonic,

I suggest to use the following code as the fix. Although it is a bit more lengthy, it makes decisions about the type of a field very clear, I think.

Code: Select all

        // TYPE_NAME
        case TypeName of
          7:
            case SubTypeName of
              1: Result.UpdateString(TableColColumnTypeNameIndex, 'NUMERIC');
              2: Result.UpdateString(TableColColumnTypeNameIndex, 'DECIMAL');
              else  Result.UpdateString(TableColColumnTypeNameIndex, 'SMALLINT');
            end;
          8:
            case SubTypeName of
              1: Result.UpdateString(TableColColumnTypeNameIndex, 'NUMERIC');
              2: Result.UpdateString(TableColColumnTypeNameIndex, 'DECIMAL');
              else Result.UpdateString(TableColColumnTypeNameIndex, 'INTEGER' );
            end;
          16:
            case SubTypeName of
              1: Result.UpdateString(TableColColumnTypeNameIndex, 'NUMERIC');
              2: Result.UpdateString(TableColColumnTypeNameIndex, 'DECIMAL');
              else Result.UpdateString(TableColColumnTypeNameIndex, GetString(ColumnIndexes[8]));
            end;
          37 : Result.UpdateString(TableColColumnTypeNameIndex, 'VARCHAR'); // Instead of VARYING
        else
            Result.UpdateString(TableColColumnTypeNameIndex, GetString(ColumnIndexes[8]));
        end;
        // COLUMN_SIZE.
        case TypeName of
          7, 8, 16: Result.UpdateInt(TableColColumnSizeIndex, GetInt(ColumnIndexes[9]));
          37, 38: Result.UpdateNull(TableColColumnSizeIndex);  //the defaults of the resultsets will be used if null
        else
          Result.UpdateInt(TableColColumnSizeIndex, GetInt(ColumnIndexes[10]));
        end; 

What do you think?

[abonic]

If it works, it's fine by me.

While we at numeric type, another, more important problem:

This is how DBGrid displays some values of numeric(4,2) type:

00.10 -> 0.100000001490116
00.80 -> 0.800000011920929
99.99 -> 99.9899978637695

Display problem can be solved if we set Precision for each numeric column (default = 15 for TFloatFiled). However, it is interesting that no corrections are necessary for numeric(5,2).

The problem is in TZSQLDA.GetFieldSqlType (from unit ZDbcInterbase6Utils) which converts type SQL_SHORT (SqlScale> 0) into stFloat, although that numeric ends up as a double in TFloatField. I'm not sure how things works in Delphi versions with TSingleField, but IMHO numeric/decimal should be always at least a double, when TBCDField is not an option.

Code: Select all

function TZSQLDA.GetFieldSqlType 
...
    SQL_LONG:
      begin
        if SqlScale = 0 then
          Result := stInteger
        else
          Result := stDouble;
      end;
    SQL_SHORT:
      begin
        if SqlScale = 0 then
          Result := stSmall
        else
(* bug fix start *)
          Result := stDouble;
(* bug fix end *)          
(* old code         
          Result := stFloat; 
*)          
       end;

Similar to the previous case, there is a problem how DBGrid displays stFloat type (when TSingleField is not available). Precision adjustment for each column can be avoided by the following correction in unit ZAbstractRODataset:

Code: Select all

procedure TZAbstractRODataset.InternalOpen;
...
    if DefaultFields and not FRefreshInProgress then
    begin
      CreateFields;
(* bug fix start *)
      for I := 0 to Fields.Count - 1 do
        if (Fields[I] is TFloatField) and (ResultSet.GetMetadata.GetColumnType(I{$IFNDEF GENERIC_INDEX}+1{$ENDIF}) = stFloat)
        then
          TFloatField(Fields[I]).Precision := 7;
(* bug fix end *)
      if not (doNoAlignDisplayWidth in FOptions) then
        for i := 0 to Fields.Count -1 do
          if Fields[i].DataType in [ftString, ftWideString{$IFDEF WITH_FTGUID}, ftGUID{$ENDIF}] then

[abonic]

Corrections from my previous post will fix the problem with float and small numeric, but not with decimal type. For some reason, decimal is always stBigDecimal and TExtendedField on Delphi side. (Numeric is TFloatField - why the difference?) To fix problems with decimal type, function TZSQLDA.GetFieldSqlType is corrected to be in sync with results produced by function ConvertInterbase6ToSqlType.

Code: Select all

function TZSQLDA.GetFieldSqlType(const Index: Word): TZSQLType;
...
    SQL_LONG:
      begin
        if SqlScale = 0 then
          Result := stInteger
(* bug fix start *)
        else if SqlSubType = RDB_NUMBERS_DECIMAL then  
          Result := stBigDecimal
(* bug fix end *)
        else
          Result := stDouble;
      end;
    SQL_SHORT:
      begin
        if SqlScale = 0 then
          Result := stSmall
        else
(* bug fix start *)
          Result := stDouble;
(* bug fix end *)
(* old code
          Result := stFloat;
*)
       end;
    SQL_FLOAT:
      Result := stFloat;
    SQL_DOUBLE, SQL_D_FLOAT:
      Result := stDouble;
    SQL_DATE: Result := stTimestamp;
    SQL_TYPE_TIME: Result := stTime;
    SQL_TYPE_DATE: Result := stDate;
    SQL_INT64:
      begin
        if SqlScale = 0 then
          Result := stLong
(* bug fix start *)
        else if SqlSubType = RDB_NUMBERS_NUMERIC then  
          Result := stDouble
(* bug fix end *)
        else
          Result := stBigDecimal;
      end;

Again, the code could be more clear, but I am trying to minimize changes.

[abonic]

My messages still waiting on approval or the server swallowed them... Really frustrating system you have here.

[marsupilami]

Hello abonic,

I am sorry for the inconvenience. New users need three approved posts to be moved to a group where their posts don't need any approval. I didn't get to do the work until now. But you should be able to post without restrictions now.

I have to admit, I couldn't read through your new proposals yet, so I have to ask you to be patient with me.

With best regards,

Jan

[marsupilami]

Hello abonic,

the first bug was fixed under Ticket #192. I will start working on the second bug tomorrow.
With best regards,

Jan

[abonic]

OK Jan... I'll wait until you finish that before sending implementation of numeric/decimal to TBCDField mapping for Firebird.

[marsupilami]

Hello abonic,

we can do it like that. It would help me a lot if you could create tickets on Sourceforge (https://sourceforge.net/p/zeoslib/tickets/). This way I don't have to copy your informaion from the forums to the tiket system. Also it helps to not forget these things. ; )

With best regards,

Jan

[marsupilami]

I created a ticket for this on SF: https://sourceforge.net/p/zeoslib/tickets/193/

[marsupilami]

Hello Abonic,

I had a look into the problem with TFloatField and its precision. First of all TFloatField implements a field for double precision data types. So generally restricting its precision to 7 does't make sense. If we keep mapping NUMERIC and DECIMAL fields to TFloatField and TSingleField, it makes sense to set the precision property to the fields scale. In your example of NUMERIC(5,1) precision should be set to 1.

Regarding your suggestion to map NUMERIC / DECIMAL to TBCDField: We cannot change that for Zeos 7.2 anymore because the guidelines prohibit us from changing behaviour if Zeos already is in Beta stage. There is code out there that relies on NUMERIC and DECIMAL fields being mapped to TFloatField.
But we can incorporate such code into Zeos 7.3, if we want to because Zeos 7.3 isn't in Beta stage yet.

With best regards,

Jan

[miab3]

@abonic, @marsupilami,

NUMERIC in FirebirdSQL:
https://www.slideshare.net/mindthebird/ ... -datatypes
http://stackoverflow.com/questions/2612 ... n-dividing
https://ib-aid.com/download/docs/firebi ... types.html


Michal

[abonic]

Hi Jan,

I had a look into the problem with TFloatField and its precision. First of all TFloatField implements a field for double precision data types. So generally restricting its precision to 7 does't make sense.

TFloatField is double but stFloat is single and only in that case Precision is set to 7. If you left the Precision on default value you have garbage on the screen with stFloat.

Code: Select all

      for i := 0 to Fields.Count - 1 do
        if (Fields[i] is TFloatField) and (ResultSet.GetMetadata.GetColumnType(I{$IFNDEF GENERIC_INDEX}+1{$ENDIF}) = stFloat) then
          TFloatField(Fields[i]).Precision := 7;

If we keep mapping NUMERIC and DECIMAL fields to TFloatField and TSingleField, it makes sense to set the precision property to the fields scale. In your example of NUMERIC(5,1) precision should be set to 1.

Why? There are no garbage with numeric/decimals when you fix inconsistencies between TZSQLDA.GetFieldSqlType and ConvertInterbase6ToSqlType. I could agree that Precision of stFloat in TFloatField is cosmetics, but TZSQLDA.GetFieldSqlType is broken and that need to be fixed.

Regarding your suggestion to map NUMERIC / DECIMAL to TBCDField: We cannot change that for Zeos 7.2 anymore because the guidelines prohibit us from changing behaviour if Zeos already is in Beta stage. There is code out there that relies on NUMERIC and DECIMAL fields being mapped to TFloatField.
But we can incorporate such code into Zeos 7.3, if we want to because Zeos 7.3 isn't in Beta stage yet.

OK. I have posted the code as SVN patch in Ticket 194.