FWD » Core Development » Database

Denormalized datetime-tz extent fields need to be checked, too.

Alexandru, please create a 7355a branch and cache the total number of SQL fields in a table at DmoMeta (so not just datetime-tz field count). This will make things clear at FqlToSqlConverter and DmoMeta will be responsible of computing this value only once.

Ovidiu: Please review.

Alexandru, there is PropertyReader.propertySize() which gives the number of SQL columns for a DMO property. Is it possible to use that?

I am OK with the code from rev. 14572. It will clearly bring more performance than my initial solution. An alternative would be to compute this number in c'tor, when the properties are added to the map (line 406).

There is another RowStructure c'tor which must also be called with the right arguments:

--- src/com/goldencode/p2j/persist/orm/DirectAccessHelper.java
+++ src/com/goldencode/p2j/persist/orm/DirectAccessHelper.java
@@ -277,7 +277,7 @@
       ResultSet rs = response.getResultSet();
       if (rs.next())
       {
-         RowStructure rowStructure = new RowStructure(meta.dmoImplInterface, meta.getSqlFieldCount());
+         RowStructure rowStructure = new RowStructure(meta.dmoImplInterface, meta.propsByName.size());
          return (Record) SQLQuery.hydrateRecord(rs, rowStructure, 1, session);
       }
       else

(‡) This gave me a possible idea for an implementation of the expansion of the extent fields, although things could get messy, with the accent in case of datetime-tz extents fields.

If I am going to use this is the c'tor and use propertySize, I can do:

boolean normalized = property.extent > 0 && property.index == 0;
if (!normalized)
{
   Class<?> fwdType = property._fwdType;
   if (BaseDataType.class.isAssignableFrom(fwdType))
   {
      fieldsCount += TypeManager.getDataHandler((Class<? extends BaseDataType>) fwdType).propertySize();
   }
   else
   {
      // is this a good fall-back?
      fieldsCount++;
   }
}

In fact, I committed this to 7355a/rev. 14573 (including some javadoc changes). Tested this and didn't see any obvious issues.

We need some tests with datetime-tz extent fields (I think is enough to have tt1.f0 as extent in the example). Test with both normalized and denormalized extents. If this works, then the fix is OK.

I tested with a complex example using extents and datetime-tz, but only normalized (which was ok). I am trying to test the denormalized extents. AFAIK, there is a "denorm-extents" option in p2j.cfg.xml, but it is only about the generated DDL for persistent databases. For the DMO's part, these are the same for normalized and denormalized. However, I don't know exactly the option to trigger temporary tables to use denormalized extents. Can you help me on this? I thought a directory.xml options is used, but there is nothing with "denorm" or "extent".

The denormalization of dynamic temp-tables is governed by persistence/denormalize-dynamic-temp-tables directory key (default is false).

The denormalization of static extent tables was disabled as part of #6561. To force it here you need to use hints. Create a <same-name-as-4gl-source>.hints file with the following content:

<?xml version="1.0"?>

<hints>
   <schema>
      <table name="[your-table-which-contains-extent-fields]">
         <custom-extent/>
      </table>
   </schema>
</hints>

Then just reconvert and you'll get the DMO using expanded fields.

Alexandru,
I mistaken the 7355a with another branch and I have just rebased it to latest trunk. Since I am doing the operation rather blindly on devsrv01 I realized only when my intended branch refused to update. Sorry for any inconvenience.

Ovidiu Maxiniuc wrote:

Then just reconvert and you'll get the DMO using expanded fields.

Got it! The changes are OK. Tested with a 4-table join: first with datetime-tz extent, second with int extent, third with datetime-tz extent and last with no extents. The sql field count was computed correctly for each table. The result was as expected.

I mistaken the 7355a with another branch and I have just rebased it to latest trunk. Since I am doing the operation rather blindly on devsrv01 I realized only when my intended branch refused to update. Sorry for any inconvenience.

This is good; I don't have to rebase it myself now :) As the tests are OK and customer applications are fine with the change, should I commit this to trunk?

Alexandru Lungu wrote:

This is good; I don't have to rebase it myself now :) As the tests are OK and customer applications are fine with the change, should I commit this to trunk?

Push it to 7156a for now, as a patch.

Do we need any additional review or testing before merging to trunk?

I've run the changes across customer applications and the individual set of tests (without issues). The core of the changes is in #7355-12.

Constantin, can you confirm that your scenario which caused #7355-1 is fixed now?

Alexandru Lungu wrote:

Constantin, can you confirm that your scenario which caused #7355-1 is fixed now?

Confirmed, the #7229 test is fixed.

Can we merge this to trunk?

Greg Shah wrote:

Can we merge this to trunk?

Is OK from my part.

OK, it can go after 7241a.

Doing the rebase now. Planning to merge asap.

Merged 7355a to trunk as rev. 14577. Archived 7355a.