FWD » Core Development » Database

The following test case has issues on FWD:

TestFindQueryUpdate.clsTestFindQueryUpdate.cls

BLOCK-LEVEL ON ERROR UNDO, THROW.

USING OpenEdge.Core.Assert FROM PROPATH.
USING tests.QueryHelper FROM PROPATH.

CLASS tests.TestFindQueryUpdate:
    @Setup.
    METHOD PUBLIC VOID Setup():
        CREATE adaptivept9.
        adaptivept9.k1 = 1.
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestFindQueryAndUpdate():
        DEFINE VARIABLE hdl AS QueryHelper NO-UNDO.
        hdl = NEW QueryHelper().

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1 NO-ERROR.
        Assert:IsTrue(AVAILABLE adaptivept9).
        adaptivept9.k1 = 0.

        Assert:IsFalse(hdl:IsAvailable(1)).

        FINALLY:
            FOR EACH adaptivept9:
                DELETE adaptivept9.
            END.
        END FINALLY.
    END METHOD.
END CLASS.

QueryHelper.clsQueryHelper.cls

CLASS tests.QueryHelper:
    METHOD PUBLIC LOGICAL IsAvailable(cId AS INTEGER):
        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1 NO-ERROR.
        RETURN AVAILABLE adaptivept9.
    END METHOD.
END CLASS.

FWD fails this test on the second assertion because the FIND FIRST statement finds the same record from the FIND FIRST statement.

I have analyzed the 4GL behavior more in depth. I have added a ON WRITE to check when the buffer gets written to the persistent database.

Code ACode A

CREATE adaptivept9.
adaptivept9.k1 = 1.

DEFINE VARIABLE i AS INTEGER NO-UNDO.

ON WRITE OF adaptivept9
DO:
    MESSAGE "Fired " + STRING(i).
    i = i + 1.
END.

DEFINE VARIABLE hdl AS tests.QueryHelper NO-UNDO.
hdl = NEW tests.QueryHelper().

FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1 NO-ERROR.
adaptivept9.k1 = 0.

MESSAGE "Before second find".

FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1 NO-ERROR.

MESSAGE "After second find".

FINALLY:
    IF VALID-OBJECT (hdl) THEN
        DELETE OBJECT hdl.
    FOR EACH adaptivept9:
        DELETE adaptivept9.
    END.
END FINALLY.

Code A works OK on FWD. The following sequence of message is:

Before second find
Fired 0
After second find

Code BCode B

CREATE adaptivept9.
adaptivept9.k1 = 1.

DEFINE VARIABLE i AS INTEGER NO-UNDO.

ON WRITE OF adaptivept9
DO:
    MESSAGE "Fired " + STRING(i).
    i = i + 1.
END.

DEFINE VARIABLE hdl AS tests.QueryHelper NO-UNDO.
hdl = NEW tests.QueryHelper().

FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1 NO-ERROR.
adaptivept9.k1 = 0.

MESSAGE "Before call".
MESSAGE(hdl:IsAvailable(1)).
MESSAGE "After call".

FINALLY:
    IF VALID-OBJECT (hdl) THEN
        DELETE OBJECT hdl.
    FOR EACH adaptivept9:
        DELETE adaptivept9.
    END.
END FINALLY.

Code B does not work on FWD (same test case from #10390-2). These two buffer are different, pointing to the same persistent database table. The first buffer has the value changed, but not released (committed to database), while the second buffer fetches the data from some sort of shared buffer from process. FWD create a new buffer and fetches the data from database.

Before call
no
After call
Fired 0

Teodor, there is a particularity of FIND unique in the sense that if there are ambiguous results, calling AMBIGUOUS adaptivept9 should return true. FWD should acknowledge the fact that there were multiple records for the same FIND unique and eventually execute some logic to match AMBIGUOUS. This doesn't explain #10390-8 per-se.

Quick question, as getDirtyInfo returns null, it means that !earlyInserts.containsKey(new RecordIdentifier<>(entity, foundID))) holds? In other words, the entity is not inside earlyInserts?

Alexandru Lungu wrote:

Teodor, there is a particularity of FIND unique in the sense that if there are ambiguous results, calling AMBIGUOUS adaptivept9 should return true. FWD should acknowledge the fact that there were multiple records for the same FIND unique and eventually execute some logic to match AMBIGUOUS. This doesn't explain #10390-8 per-se.

Quick question, as getDirtyInfo returns null, it means that !earlyInserts.containsKey(new RecordIdentifier<>(entity, foundID))) holds? In other words, the entity is not inside earlyInserts?

earlyInserts is actually null.

Ah, I see. The DMO is updated and validated. It is inserted in the RecordNursery for future use. It is required at some point, so it is made visible through the record nursery. As it is validated already, it won't be part of earlyInserts.

• if the piece of code can be changed and ChUI regression testing still passes, it is OK!
• if the piece of code is critical for cross-session dirty-share, mind wrapping it in DirtyShareSupport.isEnabledCrossSession().

In the near-future, DirtyShareContext will be completely replaced by #8388. Any effort in this area should be done just enough to avoid regressions, but still fix issues.

Alexandru Lungu wrote:

Ah, I see. The DMO is updated and validated. It is inserted in the RecordNursery for future use. It is required at some point, so it is made visible through the record nursery. As it is validated already, it won't be part of earlyInserts.

I think the QueryConstants.UNIQUE conditional is not reasonable. If removing it fixes your issue, I am OK with the approach. ATM, DirtyShareContext is so verbose and full of caveats in order to leverage the cross-session dirty-share support (that is currently active only for the ChUI regression testing). The protocol is:

• if the piece of code can be changed and ChUI regression testing still passes, it is OK!
• if the piece of code is critical for cross-session dirty-share, mind wrapping it in DirtyShareSupport.isEnabledCrossSession().

In the near-future, DirtyShareContext will be completely replaced by #8388. Any effort in this area should be done just enough to avoid regressions, but still fix issues.

ChUI regression testing passed with the current changes.

• the if (hasEnteredIteration) conditional from RAQ.executeImpl was moved because if we attempt the FIND FIRST and we fail to check the dmo record from dirty db, on the second iteration with FIND NEXT, if it still fails, the placeholder should be set to dmo and dmo to null. The if (hasEnteredIteration), when it is placed before the switch block, it does not get a chance for these two variables to be set.
• on processDirtyResults, the useDirtyDMO is already set to true and there are no changes required.

The full schema for adaptivept9 (taken from testcases project) is:

ADD TABLE "adaptivept9" 
  AREA "Schema Area" 
  DUMP-NAME "adaptivept9" 

ADD FIELD "k1" OF "adaptivept9" AS integer 
  FORMAT "->>,>>9" 
  INITIAL "0" 
  POSITION 2
  MAX-WIDTH 4
  ORDER 10

ADD FIELD "k2" OF "adaptivept9" AS integer 
  FORMAT "->>,>>9" 
  INITIAL "0" 
  POSITION 3
  MAX-WIDTH 4
  ORDER 20

ADD INDEX "idx3" ON "adaptivept9" 
  AREA "Schema Area" 
  UNIQUE
  PRIMARY
  INDEX-FIELD "k1" ASCENDING 
  INDEX-FIELD "k2" ASCENDING 

I have checked the bundle and the results when FIND FIRST is used.
When activeBundleKey is first, the old record is retrieved from database. Then, it checks the dirty database, retrieves the updated record (is not null). It gets into the switch where the activateNext() is called.
On activate next, the following bundle is generated BundleBundle

PRIMARY STATEMENTS
   select adaptivept9.recid from Adaptivept9__Impl__ as adaptivept9 where (adaptivept9.k1 = 1) and ((coalesce(adaptivept9.k1 = ?0, not (adaptivept9.k1 = adaptivept9.k1), coalesce(?1) is null)) and (coalesce(adaptivept9.k2 = ?2, not (adaptivept9.k2 = adaptivept9.k2), coalesce(?3) is null)) and adaptivept9.recid > ?4) order by adaptivept9.k1 asc, adaptivept9.k2 asc, adaptivept9.recid asc
   select adaptivept9.recid from Adaptivept9__Impl__ as adaptivept9 where (adaptivept9.k1 = 1) and ((coalesce(adaptivept9.k1 = ?0, not (adaptivept9.k1 = adaptivept9.k1), coalesce(?1) is null)) and (coalesce(adaptivept9.k2 > ?2, not (adaptivept9.k2 = adaptivept9.k2), coalesce(?3) is not null))) order by adaptivept9.k1 asc, adaptivept9.k2 asc, adaptivept9.recid asc
   select adaptivept9.recid from Adaptivept9__Impl__ as adaptivept9 where (adaptivept9.k1 = 1) and ((coalesce(adaptivept9.k1 > ?0, not (adaptivept9.k1 = adaptivept9.k1), coalesce(?1) is not null))) order by adaptivept9.k1 asc, adaptivept9.k2 asc, adaptivept9.recid asc
DIRTY STATEMENTS
   select adaptivept9.recid from Adaptivept9__Impl__ as adaptivept9 where (adaptivept9.k1 = 1) and ((coalesce(adaptivept9.k1 = ?0, not (adaptivept9.k1 = adaptivept9.k1), coalesce(?1) is null)) and (coalesce(adaptivept9.k2 = ?2, not (adaptivept9.k2 = adaptivept9.k2), coalesce(?3) is null)) and adaptivept9.recid > ?4) order by adaptivept9.k1 asc, adaptivept9.k2 asc, adaptivept9.recid asc
   select adaptivept9.recid from Adaptivept9__Impl__ as adaptivept9 where (adaptivept9.k1 = 1) and ((coalesce(adaptivept9.k1 = ?0, not (adaptivept9.k1 = adaptivept9.k1), coalesce(?1) is null)) and (coalesce(adaptivept9.k2 > ?2, not (adaptivept9.k2 = adaptivept9.k2), coalesce(?3) is not null))) order by adaptivept9.k1 asc, adaptivept9.k2 asc, adaptivept9.recid asc
   select adaptivept9.recid from Adaptivept9__Impl__ as adaptivept9 where (adaptivept9.k1 = 1) and ((coalesce(adaptivept9.k1 > ?0, not (adaptivept9.k1 = adaptivept9.k1), coalesce(?1) is not null))) order by adaptivept9.k1 asc, adaptivept9.k2 asc, adaptivept9.recid asc
GETTERS
   public abstract com.goldencode.p2j.util.integer com.goldencode.testcases.dmo.tstcasesdb.Adaptivept9.getK1()
   public abstract com.goldencode.p2j.util.integer com.goldencode.testcases.dmo.tstcasesdb.Adaptivept9.getK2()
   public abstract java.lang.Long com.goldencode.p2j.persist.Persistable.primaryKey()

In this form, that third FQL statement seems valid to me and is normal to return a record from database, since the adaptivept9 k1 1 is located into the database, and 1 is greater than 0, but I don't quite understand his purpose.

In this form, that third FQL statement seems valid to me and is normal to return a record from database, since the adaptivept9 k1 1 is located into the database, and 1 is greater than 0, but I don't quite understand his purpose.

It should not retrieve any record. Unless the placeholder is wrong and uses 0 as parameter for k1 > ?. From your statement I understand that this is the case.

adaptivept9.k1 = 0.

This update never reached primary database. Also, the primary database shouldn't be interrogated with information from dirty. In other words, if primary database didn't find out that k1 was updated to 0, then no query should presume that.
From placeholder = dmo; I understand that the placeholder should have had 1 instead of 0. Why is the query emitted with 0 as subst. param?

Because persistence.load (Session.getImpl) returns the cached version of the dmo. The cached version of the dmo contains value k1 0 and k2 0.

Teodor, is this an issue with the #9457 customer app?

Yes, there are 2 unit tests that suffers this issue, but I have didn't considered this high priority to be in trunk because it is being solved by #8388.

Added Andrei as a watcher. Teodor, let me know when this is ready for review.

I have starting working on this task and I have might found the root cause for each issue.
With the latest trunk, this is the following result:

╷
├─ JUnit Jupiter ✔
├─ JUnit Vintage ✔
├─ JUnit Platform Suite ✔
└─ FWD Test ✔
   └─ tests.Test10390a ✔
      ├─ testFindQueryAndUpdate ✘ Expected: FALSE but was: yes
      ├─ testFindQueryOnSameBuffer ✔
      ├─ test10083TestcaseIfWorks ✔
      └─ testFindQueryModifyIndexSecondField Expected: FALSE but was: yes

For the testFindQueryAndUpdate testcase, which is the original issue for this task, the issue happens because the dmo reference from RAQ.executeImpl is the same as the one from the Session cache. When the record gets updated, the record from Session cache also gets updated. This breaks the second FIND FIRST which uses the updated DMO parameters for second iteration NEXT (or dirty share fetch). I have disabled the cache and the test is passing. I think I will change the Session.getCached or Persistence.load to return a copy of DMO from cache.

For the testFindQueryModifyIndexSecondField, this is a real issue from RAQ.executeImpl which is related by DirtyShareContext.getDirtyInfo (the code which should be removed, #10390-17), and by RAQ.processDirtyResults (compDirtyDMO is null but for this case, it should actually execute the code bellow).

Teodor, please mind that Andrei is already working is a close area. Please have a chat with him first to see if you can share knowledge, tests and fixes. After that, I can take a look at the attempt on 10390a and discuss with you on it.

Alexandru Lungu wrote:

Teodor, please mind that Andrei is already working is a close area. Please have a chat with him first to see if you can share knowledge, tests and fixes. After that, I can take a look at the attempt on 10390a and discuss with you on it.

I have discussed and we discussed more about the issue and the conclusion is the following:
- the latest fix from #11027 fixes FIND NEXT/FOR EACH (Dynamic mode) but regresses FIND FIRST, making the execution to be in a infinite loop.
- description: since the DMO returned from Session cache reflects the modified DMO and not what is in the PRIMARY database. The hydrated field values no longer reflects the FQL WHERE criteria which leads to the wrong parameter substitution on the NEXT iteration.
- proposed solution: if the record is not being found in the dirty database (but marked as CACHED and CHANGED), it should attempt with NEXT using the correct substitution values from pristine record.

I will remove the partial fix for the first testcase and I will keep the fix for FIND UNIQUE.

Rebased task branch 10390a to trunk rev 16377. This includes changes from #11027.

Alexandru, when you have time, please review the latest changes from #10390. It contains the changes which we already discussed in the previews code reviews, which we thought that are fine.

Alex: please review this.

About the testcases cross-session tests, they have passed.

I have run the ChUI regression testing and I see two unexpected fails. I have tested them by hand and I see that they pass. I will test with a clean trunk with the same revision as was tested with 10390a and check if it actually comes from trunk.

ChUI regression testing has passed.