mORMot Open Source

wloochacz

Member

Registered: 2015-01-03

Posts: 45

Possible field mapping error in TSQLRecordPropertiesMapping.Init

Hello,

Short version; calling MapFields more than once for a given TSQLRecord will always set the value of RowIDFieldName to name ID.
And never mind that previously it was mapping the ID to another field name....

Details:

I have a table like this (MS SQL):

CREATE TABLE dbo.tDfRmAtom (
  IdAtom dbo.uINT IDENTITY(1, 1) NOT NULL,
  Name dbo.uNAME_XL NOT NULL,
  Type dbo.uINT NOT NULL
  CONSTRAINT PK_tDfRmAtom PRIMARY KEY CLUSTERED (IdAtom)
)

Which corresponds to the following TSQLRecord class:

  TormtDfRmAtom = class(TBaseORMEntity)
  strict private
    FName: RawUTF8;
    FTypeKW: Integer;
  strict protected
    procedure SetName(const AValue : RawUTF8); virtual;
    procedure SetDescription(const AValue : TNullableUTF8Text); virtual;
    procedure SetCaption(const AValue : TNullableUTF8Text); virtual;
    procedure SetCustomData(const AValue : TNullableUTF8Text); virtual;
  public
    class function dfGetPKFieldName(): RawUTF8; override;
    class function dfGetTableName(): RawUTF8; override;
  published
    property IdAtom: TID read fID write fID;
    property Name: RawUTF8 read FName write SetName;
    property TypeKW: Integer read FTypeKW write FTypeKW;
  end;

Then executes the registration like this:

  VirtualTableExternalMap(DataModel, TormtDfRmAtom, FConnectionProperties, 'tDfRmAtom', [rpmNoCreateMissingField, rpmClearPoolOnConnectionIssue])
    .MapField('ID', 'IdAtom')
    .MapField('TypeKW', 'Type');

When running such code, mORMot reports an invalid SQL syntax exception...
The SQL that the TableHasRows method attempts to execute looks like this:

select top(1) ID from dbo.tDfRmAtom

And it should be like this:

select top(1) IdAtom as ID from dbo.tDfRmAtom

I found the cause in the TSQLRecordPropertiesMapping.Init method in the mORMot module.
The Init method is called for every field mapping.
And it always assigns:

.
fRowIDFieldName := 'ID';

.

So the first mapping will correctly assign ID to IdAtom field name, but each subsequent mapping will again set fRowIDFieldName := 'ID'
My test was that before assigning a value to fRowIDFieldName; it simply assigns a value only if fRowIDFieldName contains an empty string.

Everything seems to be working correctly now.

AB, is this a bug?

Last edited by wloochacz (2021-03-31 12:05:59)

ttomas

Member

Registered: 2013-03-08

Posts: 135

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

Is this typo in post or in your code
.MapField('TypeKW', 'Type')
vs
.MapField('Type', 'TypeKW')

wloochacz

Member

Registered: 2015-01-03

Posts: 45

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

Is this typo in post or in your code

A property named TypeKW has been declared in the class, which corresponds to a field in the table named Type.
Also, I don't see any typo in my post.

.MapField('TypeKW', 'Type')

This notation maps a class property named Type to a database table field named TypeKW.
That is, exactly the opposite of my example.
Besides, you can't declare a class property named Type because it's a Delphi/Pascal keyword.

vs
.MapField('Type', 'TypeKW')

This is the correct notation for my example and mORMot generates such a SelectSQL:

select 
  IdAtom as ID,IdAtom, 
  Name, 
  Type as TypeKW 
from dbo.tDfRmAtom

So absolutely correct.

Vitaly

Member

From: UAE

Registered: 2017-01-31

Posts: 168

Website

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

Could you explain the need for IdAtom published property? I just couldn't understand it. TSQLRecord has ID support already, you can use this field easily. ID mapping will also work without additional published property...

Besides, you can't declare a class property named Type because it's a Delphi/Pascal keyword.

As an option, you could use '&' for avoiding Delphi keyword and reducing mappings quantity

  TormtDfRmAtom = class(TBaseORMEntity)
  ...
  published
    // property IdAtom: TID read fID write fID;
    property Name: RawUTF8 read FName write SetName;
    property &Type: Integer read FType write FType;
  end;

  ...
  .MapField('ID', 'IdAtom');

wloochacz

Member

Registered: 2015-01-03

Posts: 45

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

I'll answer these questions below, but please note that they are actually not very important.
What is important is that the method code TSQLRecordPropertiesMapping.Init is probably wrong or the method call string when mapping fields is wrong - I do not know, this is a question for AB.

TSQLRecordPropertiesMapping contains the mapping information, in it there is a field named fRowIDFieldName which stores the name of the ID field (primary key in the database).
TSQLRecordPropertiesMapping.Init is called whenever MapFields is executed and it ALWAYS sets it like this: fRowIDFieldName := 'ID';

The first call to MapFields is correct, but each call thereafter assigns me fRowIDFieldName := 'ID';
And that's exactly the problem.

Could you explain the need for IdAtom published property? I just couldn't understand it.

First - this is not an ORM First type project, let's say it's a legacy database.
Why do I need IdAtom?
Because in my database, no primary key field is simply called ID (by the way, I consider it a conceptual error, but whatever).
It's always IdSomething, and that's how I want a field with that name in TSQLRecord, not ID.
Simply put.

TSQLRecord has ID support already, you can use this field easily. ID mapping will also work without additional published property...

First please take a good look at my TSQLRecord declaration, and you'll probably notice that I'm using the ID field - note the read and write of the IdAtom field.

Besides, you can't declare a class property named Type because it's a Delphi/Pascal keyword.

As an option, you could use '&' for avoiding Delphi keyword and reducing mappings quantity

Yes, I know that, but it seems sloppy to me and I just don't like that kind of code.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

You should not publish the IDAtom field, even if it redirects to fID.
The ORM will create a new column, which is not what you expect.
You need to map the existing ID field as IDAtom in the DB.

wloochacz

Member

Registered: 2015-01-03

Posts: 45

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

You should not publish the IDAtom field, even if it redirects to fID.
The ORM will create a new column, which is not what you expect.
You need to map the existing ID field as IDAtom in the DB.

Yes, you're right and I noticed that earlier, but I forgot about it while writing the database ORM class generator.
This is easily fixed by moving the IdAtom field declaration to the public section.
Then everything looks good to mORMot, and I achieve the expected effect.

But what about the mapping problem I described above?
Can you confirm if the problem exists, AB?

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

For me, TSQLRecordPropertiesMapping.Init method is not called for every MapField() call.
It is just called once.

So I don't see which problem do you refer to, sorry.

wloochacz

Member

Registered: 2015-01-03

Posts: 45

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

OK, you're right AB, and I apologize for misleading you, I've already become looped in all of this myself.
However, the problem exists, take a look please;

- When mapping like this:

  VirtualTableExternalMap(DataModel, TeoDfRmAtomT, FConnectionProperties, 'tDfRmAtom', [rpmNoCreateMissingField, rpmClearPoolOnConnectionIssue]).MapField('ID', 'IdAtom')
    .MapField('ID', 'IdAtom')
    .MapField('TypeKW', 'Type');

TSQLRecordPropertiesMapping.Init only calls once.

However, if the mapping is done this way:

  VirtualTableExternalMap(DataModel, TeoDfRmAtomT, FConnectionProperties, 'tDfRmAtom', [rpmNoCreateMissingField, rpmClearPoolOnConnectionIssue]).MapField('ID', 'IdAtom');
  VirtualTableExternalMap(DataModel, TeoDfRmAtomT, FConnectionProperties, 'tDfRmAtom', [rpmNoCreateMissingField, rpmClearPoolOnConnectionIssue]).MapField('TypeKW', 'Type');

TSQLRecordPropertiesMapping.Init will be called by every time.
Then the following call sequence occurs:
VirtualTableExternalMap -> VirtualTableExternalRegister -> TSQLModel.VirtualTableRegister -> ExternalDB(TSQLRecordPropertiesMapping).Init

And I use multiple VirtualTableExternalMap calls for the same TSQLRecord because I find it easier to write the code generator that way.

I solved the problem by setting an extra flag in TSQLRecordPropertiesMapping.Init to not allow the Init method to be called multiple times for the same TSQLRecordPropertiesMapping instance.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

The second way is incorrect usage.

It works as expected: each time you call VirtaulTableExternalMap(), you make a new mapping.
If you want to call MapField() several times, just use a local variable.

wloochacz

Member

Registered: 2015-01-03

Posts: 45

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

OK, I see.
Since such a call is invalid, don't you think an exception should be raised there?

procedure TSQLRecordPropertiesMapping.Init(Table: TSQLRecordClass;
  const MappedTableName: RawUTF8; MappedConnection: TObject;
  AutoComputeSQL: boolean; MappingOptions: TSQLRecordPropertiesMappingOptions);
begin
  // I added such a private field to protect against re-initialization.
  if fInitialized then
    Exit // raise exception?
  else
    fInitialized := True;

  // and then the appropriate initialization code  
end;

Of course I would prefer it to work as I wrote above
But if the convection is that such a call is invalid, then IMO an exception should definitely be raised there.
Otherwise you can hit some really weird problems, don't you think AB?

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Possible field mapping error in TSQLRecordPropertiesMapping.Init

It is not invalid, it is as designed.

There may be some cases when you want to reset the mapping.

I don't see what is the benefit from your side of calling

 VirtualTableExternalMap(DataModel, TeoDfRmAtomT, FConnectionProperties, 'tDfRmAtom', [rpmNoCreateMissingField, rpmClearPoolOnConnectionIssue])

several times.
It is slow, and error prone.