Bug #10455: Persistent table CAN-FIND FIRST issue - Database - Golden Code Redmine

Bug #10455

Persistent table CAN-FIND FIRST issue

Added by Teodor Gorghe 11 months ago. Updated 6 months ago.

Status:

Closed

Priority:

Normal

Assignee:

Teodor Gorghe

Target version:

Start date:

Due date:

% Done:

100%

billable:

vendor_id:

GCD

case_num:

version_reported:

version_resolved:

reviewer:

Constantin Asofiei

production:

env_name:

topics:

TestMethodCanFind.cls (12.6 KB) Teodor Gorghe, 08/29/2025 02:34 AM

Related issues

History

#2 Updated by Teodor Gorghe 11 months ago

Related to Bug #10390: FIND FIRST buffer field change does not propagate into the next FIRST FIRST statements. added

#3 Updated by Teodor Gorghe 11 months ago

The following test fails on FWD:

USING OpenEdge.Core.Assert FROM PROPATH.

BLOCK-LEVEL ON ERROR UNDO, THROW.

CLASS tests.TestMethodCanFind:
    @Setup.
    METHOD PUBLIC VOID Setup():
        DeleteAllRecords().
        CREATE adaptivept9.
        adaptivept9.k1 = 1.
        adaptivept9.k2 = 2.
        RELEASE adaptivept9.
    END METHOD.

    @Cleanup.
    METHOD PUBLIC VOID Cleanup():
        DeleteAllRecords().
    END METHOD.

    METHOD PUBLIC VOID DeleteAllRecords():
        FOR EACH adaptivept9:
            DELETE adaptivept9.
        END.
    END METHOD.

    METHOD PUBLIC INTEGER GetValue():
        DEFINE BUFFER bpv FOR adaptivept9.
        IF CAN-FIND(FIRST bpv WHERE bpv.k1 = 1) THEN
            RETURN 1.
        RETURN 100.
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssign():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = GetValue().
        Assert:Equals(100, iVal).
    END METHOD.
END CLASS.

The expected value for iVal is 100, but FWD returns 1, because the CAN-FIND returns TRUE, which actually finds the record that was created before. (old state, with k1=1).

#4 Updated by Teodor Gorghe 11 months ago

I have investigated more about this issue.
The original code that was here, actually worked, it was just the dirty intra session disabled (testcases dataset comes with this setting).
I have adjusted the code and it actually fails.

The difference that makes it now to fail is because of the batch block.
Example:

in the code below, the value of iVal is 1.

         batch(() -> 
         {
            adaptivept9.setK2(new integer(2));
            adaptivept9.setK1(new integer(2));
            iVal.assign(getValue());
         });

but in this code, the value of iVal is 100:

         batch(() -> 
         {
            adaptivept9.setK2(new integer(2));
            adaptivept9.setK1(new integer(2));
         });
         iVal.assign(getValue());

The difference is that when DirtyShareContext.getDirtyInfo is called, on the first case, this returns null, because dirtyState is NONE, but on the second case, the dirtyState is CHANGE and it actually returns the uncommitted record.

#5 Updated by Teodor Gorghe 11 months ago

I have continued the research for this problem, and I have noticed the following:

On DirtyShareContext.getDirtyInfo, the dirtyState is DirtyInfo.NONE because the changes map from DefaultDirtyShareManager is empty. On non-batch assignment mode, this map has only 1 change, which makes this to return DirtyInfo.CHANGE.
On non-batch assignment mode, this map gets updated by RecordNursey.makeVisible: updatedRecords.makeVisible, but on batch-assignment mode, the records map is also empty.
On non-batch assignment mode, the records map gets updated on every RecordBuffer set, by validateMaybeFlush on RecordBuffer.Handler.invoke. On batch assignment mode, the buffer is just placed on the BufferManager.batchModeStack, by BufferManager.addDirtyBatchBuffer method. This gets updated on the dirty database when the batch mode ends.
In the batch mode, when a new Block scope is being opened, on the BufferManager.batchModeStack, it is placed a new BatchModeData, which gets removed from the stack, at the Block end of scope.

I am thinking about adding a logic right when a Block scope gets opened, which updates the dirty state for the current BatchModeData from the stack. The validateMaybeFlush call here may be not the best way, but I need to check the 4GL behavior in this case.

#6 Updated by Teodor Gorghe 11 months ago

Assignee set to Teodor Gorghe
Status changed from New to WIP

Created task branch 10455a.

#7 Updated by Teodor Gorghe 11 months ago

% Done changed from 0 to 100
Status changed from WIP to Review
reviewer Constantin Asofiei added

Committed revision 16124 on task branch 10455a:
- Commit changes to DirtyShareContext on open block scope when batch mode is active.

#8 Updated by Teodor Gorghe 11 months ago

ChUI regression testing passed with the current changes.

#9 Updated by Alexandru Lungu 11 months ago

reviewer Alexandru Lungu added

#10 Updated by Alexandru Lungu 11 months ago

% Done changed from 100 to 80
Status changed from Review to WIP

I made some experiments my own and reached the same conclusion as yours. Using function invocations inside the ASSIGN batch triggers something. You considered that the something is cross-buffer intra-session visibility. I think it is more that that ... it is actually validateMaybeFlush! If you would create a adaptivept9 record with values 2/2, then a validation exception will occur before the function is called. This means that 4GL is basically validating the assign batch at that point. Please try:

Even though after the first two statements there is an unique violation, the 4th statement cures it. OE shows that this is an actual full batch.
```
   assign adaptivept9.f1 = 2
          adaptivept9.f2 = 2
          i = 1         
          adaptivept9.f1 = 1.
```

First two makes the record violate an unique index and the thrid statement actually triggers it. The 4th statement isn't reached. You can have getValue empty, hinting that 4GL does that when calling a function regardless its body.
```
   assign adaptivept9.f1 = 2
          adaptivept9.f2 = 2
          i = getValue()         
          adaptivept9.f1 = 1.
```

This example is also the most interesting as it also throws:

Cannot execute user-defined function, method or property accessor 'getValue' in an ASSIGN statement after an indexing error. (13917)

The "curing" can happen after getValue. The 4th statement may conflict the unique index, but 5th statement is curing it.

   assign adaptivept9.f1 = 3
          adaptivept9.f2 = 2
          i = getValue()         
          adaptivept9.f1 = 2
          adaptivept9.f1 = 3.

Lastly, this behavior seems to also apply to method invocation, dynamic-function, etc.

I would personally stay away from record nursery / dirty-share in this example, because it proves that validation momentum is the culprit. If we can match the right validation time, then Validation.validateMaybeFlush will automatically validate and honor the record nursery.

Extra: You should also account for ASSIGN triggers that are set per-column. If you set an ASSIGN on f1, this trigger is called at the end of the batch. From my experiments:

even if the validation happens before the function call, the ASSIGN triggers are still at the very end (which looks weird to me TBH).
you can FIND the record affected by the batch update and RELEASE it causing a WRITE trigger. So this means you may end up flushing the record to the physical database in the middle of batch updating. This is just a fun fact :)

With this being said, I think 10455a is really close to a final solution, but I think that the dirty batch buffers should be rather simply: buffer.validateMaybeFlush(dmo, false, false); (flush to false to avoid WRITE trigger). This is not only for CHANGED - but for any arbitrary dirty buffer. You can use RB.endBatch as inspiration, but do not drag code related to endBatchAssignMode, getTriggerTracker or similar. I think the validation is enough. Also:

please make sure you don't validate the same buffer all over again. In getDirtyInfo there may be multiple nested blocks and each such block will open a scope that will make crtBatchScope validate everything again and again.

#11 Updated by Teodor Gorghe 11 months ago

Thanks for code review!

I have started working on this task branch, but I think that adding the validation into the BufferManager.scopeStart gives some other issues:
- When the validation fails, a DeferredLegacyErrorException is thrown by recordOrThrowError, the error is deferred by deferError from processScopeNotifications, since it is a Scopeable.
- For every Scopeable, the deferred error is actually thrown lately after block execution (Scopeable.scopeFinished is also called).

#12 Updated by Teodor Gorghe 11 months ago

Status changed from WIP to Review
% Done changed from 80 to 100

Committed revision 16125 on task branch 10455a:
- Added validation when a function is called in batch assignment mode.

Alexandru or Constantin, please review the changes, I am not quite sure if this is the right way to handle the validation error.

#13 Updated by Alexandru Lungu 11 months ago

Review of 10455a:

I would like to defer the review of exception handling to Constantin. To me it looks OK, but I am aware that recently there is work on how to properly handle these (including CATCH blocks) and I am not fully aware of the details.
From my POV, the attempt looks right. Please still assess:
- I am a bit concerned that indexingErrors is never reset to false.
- There is some code in endBatch with SYNCHRONIZE and AUTO-SYNCHRONIZE that I do not know if they should take effect for validation (if it passes).

More testing:

I think a bit more analysis should be done to check what is the state of the execution after-wards (are the changes reverted, is a catch block honored, etc.)
Please also test other ways to escape the "batch", except for function calling: dynamic function, property getter/setter handlers, methods, etc.
Is NO-ERROR affecting this scenario or ON BLOCK-LEVEL/ROUTINE-LEVEL?

#14 Updated by Teodor Gorghe 11 months ago

The changes are reverted to the state that was before BATCH mode.
The synchronization does not happen when the validation occurs before function call.
The error gets into the catch block.

I have notices 2 things that are not OK:

on DYNAMIC-FUNCTION test case, I will look on it tomorrow, this does not test my changes.
When accessing property blocks, it does not get into BlockManager.scopeStart.
When NO-ERROR is used, after execution, the ERROR-STATUS:NUM-MESSAGES is 2, but on FWD is 0 (sometimes).

I have attached all the tests that I have executed: ABL Code ABL Code

USING OpenEdge.Core.Assert FROM PROPATH.
USING support.test.AssertExt FROM PROPATH.

BLOCK-LEVEL ON ERROR UNDO, THROW.

CLASS tests.TestMethodCanFind:
    DEFINE VARIABLE i2 AS INTEGER NO-UNDO INITIAL 0.
    def temp-table tt1 field f1 as int
                       field f2 as int.

    def temp-table tt2 field f2 as int
                       field f3 as int.

    def temp-table tt3 field f1 as int
                       field f2 as int.

    define dataset dtt1 for tt1, tt2 
                   data-relation rel1 for tt1, tt2 relation-fields (f2, f2).

    DEFINE PROPERTY propGetterNoBlock AS INTEGER GET. SET.

    DEFINE PROPERTY propGetterBlock AS INTEGER GET():
    END.
    SET.

    METHOD PUBLIC VOID PopulateBuffers():
        /* POPULATE TT1 TEMP-TABLE */
       create tt1.
       tt1.f1 = 3.
       tt1.f2 = 3.
       release tt1.

       create tt1.
       tt1.f1 = 5.
       tt1.f2 = 5.
       release tt1.
       create tt1.
       tt1.f1 = 7.
       tt1.f2 = 7.
       release tt1.

       create tt1.
       tt1.f1 = 8.
       tt1.f2 = 8.
       release tt1.

       create tt1.
       tt1.f1 = 9.
       tt1.f2 = 9.
       release tt1.

       create tt1.
       tt1.f1 = 4.
       tt1.f2 = 4.
       release tt1.

       /* POPULATE TT2 TEMP-TABLE */
       create tt2.
       tt2.f2 = 4.
       tt2.f3 = 4.
       release tt2.

       create tt2.
       tt2.f2 = 6.
       tt2.f3 = 6.
       release tt2.

       create tt2.
       tt2.f2 = 0.
       tt2.f3 = 0.
       release tt2.

       /* SETS THE AUTO-SYNCHRONIZE ATTRIBUTE ON TT1. */
       buffer tt1:AUTO-SYNCHRONIZE = yes.
    END METHOD.

    METHOD PUBLIC INTEGER CheckBuffer():
        Assert:IsFalse(AVAILABLE (tt2)).
        RETURN tt1.f2.
    END METHOD.

    @Setup.
    METHOD PUBLIC VOID Setup():
        DeleteAllRecords().
        CREATE adaptivept9.
        adaptivept9.k1 = 1.
        adaptivept9.k2 = 2.
        RELEASE adaptivept9.
    END METHOD.

    @Cleanup.
    METHOD PUBLIC VOID Cleanup():
        DeleteAllRecords().
    END METHOD.

    METHOD PUBLIC VOID DeleteAllRecords():
        FOR EACH adaptivept9:
            DELETE adaptivept9.
        END.
    END METHOD.

    METHOD PUBLIC INTEGER GetValue():
        DEFINE BUFFER bpv FOR adaptivept9.
        IF CAN-FIND(FIRST bpv WHERE bpv.k1 = 1) THEN
            RETURN 1.
        RETURN 100.
    END METHOD.

    METHOD PUBLIC INTEGER SetValue():
        i2 = 3.
        RETURN i2.
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssign():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = GetValue().
        Assert:Equals(100, iVal).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationError():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = GetValue().

        Assert:IsTrue(FALSE).

        CATCH aError AS Progress.Lang.Error :
            Assert:Equals(2, aError:NumMessages).
            Assert:Equals(132, aError:GetMessageNum(1)).
            Assert:Equals("** adaptivept9 already exists with k1 2 k2 2. (132)", aError:GetMessage(1)).
            Assert:Equals(13917, aError:GetMessageNum(2)).
            Assert:Equals("Cannot execute user-defined function, method or property accessor 'GetValue' in an ASSIGN statement after an indexing error. (13917)", aError:GetMessage(2)).
            Assert:Equals(1, i).
            Assert:Equals(0, iVal).
        END CATCH.
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationNoError():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.
        DEFINE VARIABLE errCode AS INTEGER NO-UNDO EXTENT 2.
        DEFINE VARIABLE errMessage AS CHARACTER NO-UNDO EXTENT 2.

        ASSIGN
            errCode[1] = 132
            errCode[2] = 13917
            errMessage[1] = "** adaptivept9 already exists with k1 2 k2 2. (132)" 
            errMessage[2] = "Cannot execute user-defined function, method or property accessor 'GetValue' in an ASSIGN statement after an indexing error. (13917)".

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = GetValue() NO-ERROR.
        AssertExt:Errors(errCode, errMessage).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationNoErrorChangesAreReverted():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.
        DEFINE VARIABLE errCode AS INTEGER NO-UNDO EXTENT 2.
        DEFINE VARIABLE errMessage AS CHARACTER NO-UNDO EXTENT 2.

        ASSIGN
            errCode[1] = 132
            errCode[2] = 13917
            errMessage[1] = "** adaptivept9 already exists with k1 2 k2 2. (132)" 
            errMessage[2] = "Cannot execute user-defined function, method or property accessor 'GetValue' in an ASSIGN statement after an indexing error. (13917)".

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = GetValue() NO-ERROR.
        AssertExt:Errors(errCode, errMessage).
        Assert:Equals(1, adaptivept9.k1).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationNoErrorDynamicFunction():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.
        DEFINE VARIABLE errCode AS INTEGER NO-UNDO EXTENT 2.
        DEFINE VARIABLE errMessage AS CHARACTER NO-UNDO EXTENT 2.

        ASSIGN
            errCode[1] = 13664
            errCode[2] = 132
            errMessage[1] = "Cannot invoke method 'GetValue' in object of type 'tests.TestMethodCanFind' using DYNAMIC-FUNCTION(). (13664)" 
            errMessage[2] = "** adaptivept9 already ex ists with k1 2 k2 2. (132)".

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = DYNAMIC-FUNCTION ("GetValue") NO-ERROR.
        AssertExt:Errors(errCode, errMessage).
        Assert:Equals(1, adaptivept9.k1).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationNoErrorPropertyGetterNoBlock():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.
        DEFINE VARIABLE errCode AS INTEGER NO-UNDO EXTENT 1.
        DEFINE VARIABLE errMessage AS CHARACTER NO-UNDO EXTENT 1.

        ASSIGN
            errCode[1] = 132
            errMessage[1] = "** adaptivept9 already exists with k1 2 k2 2. (132)".

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = propGetterNoBlock NO-ERROR.
        AssertExt:Errors(errCode, errMessage).
        Assert:Equals(1, adaptivept9.k1).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationNoErrorPropertyGetterBlock():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.
        DEFINE VARIABLE errCode AS INTEGER NO-UNDO EXTENT 2.
        DEFINE VARIABLE errMessage AS CHARACTER NO-UNDO EXTENT 2.

        ASSIGN
            errCode[1] = 132
            errCode[2] = 13917
            errMessage[1] = "** adaptivept9 already exists with k1 2 k2 2. (132)" 
            errMessage[2] = "Cannot execute user-defined function, method or property accessor 'propGetterBlock' in an ASSIGN statement after an indexing error. (13917)".

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.
        ASSIGN
            adaptivept9.k2 = 2
            adaptivept9.k1 = 2
            iVal = propGetterBlock NO-ERROR.
        AssertExt:Errors(errCode, errMessage).
        Assert:Equals(1, adaptivept9.k1).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationErrorDoLeave():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.

        DO ON ERROR UNDO, LEAVE:
            ASSIGN
                adaptivept9.k2 = 2
                adaptivept9.k1 = 2
                iVal = GetValue()
                i = 2.
        END.
        Assert:Equals(1, i).
        Assert:Equals(0, iVal).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedParentMethodAssignValidationErrorDoLeave2():
        DEFINE VARIABLE iVal AS INTEGER NO-UNDO.
        DEFINE VARIABLE i AS INTEGER NO-UNDO INITIAL 1.
        i2 = 0.

        CREATE adaptivept9.
        adaptivept9.k1 = 2.
        adaptivept9.k2 = 2.

        FIND FIRST adaptivept9 WHERE adaptivept9.k1 = 1.

        DO ON ERROR UNDO, LEAVE:
            ASSIGN
                adaptivept9.k2 = 2
                adaptivept9.k1 = 2
                iVal = GetValue()
                i = SetValue()
                i = 2.
        END.
        Assert:Equals(1, i).
        Assert:Equals(0, i2).
        Assert:Equals(0, iVal).
    END METHOD.

    @Test.
    METHOD PUBLIC VOID TestBufferModifiedAutoSynchronize():
        PopulateBuffers().
        FIND FIRST tt1 WHERE tt1.f1 = 3.
        ASSIGN
            tt1.f2 = 11.
            tt1.f2 = CheckBuffer().
        Assert:Equals(11, tt1.f2).
        Assert:IsFalse(AVAILABLE (tt2)).
    END METHOD.
END CLASS.

#15 Updated by Teodor Gorghe 11 months ago

File TestMethodCanFind.cls added

The property block case works just fine, but the problem is that when there is no buffer usage into the the called function/property getter block, then the BufferManager does not get into the Scopeable list. The BufferManager.scopeStart does not run, hence the validation does not occur. On 4GL, the validation on function call happens in all cases, except when there is a property value getter (no getter block).
The NO-ERROR case is not working quite well and I need to figure it out why.

#16 Updated by Teodor Gorghe 11 months ago

Committed revision 16126 on task branch 10455a:

Moved validation from BufferManager into BlockManager.functionBlock.

#17 Updated by Alexandru Lungu 11 months ago

% Done changed from 100 to 90
Status changed from Review to WIP

Review of 10455a:

I am very OK with the logic being moved to BlockManager.functionBlock.
I am still not confident that validateBatchModeInvocation will happen only for the current batch and not other upstream ones. Consider:
- function 1 runs a batch. The batch runs function 2. Function 2 runs function 3. Won't the validateBatchModeInvocation revalidate the buffers from the OG batch?
- I would expect to check whether the crtBatchScope.batchDepth actually equals the current scope depth.

#18 Updated by Teodor Gorghe 11 months ago

Committed revision 16127 on task branch 10455a:
- Avoid calling validation multiple times in the nested function blocks.

#19 Updated by Teodor Gorghe 11 months ago

Status changed from WIP to Review
% Done changed from 90 to 100

#20 Updated by Constantin Asofiei 10 months ago

Alexandru, please review.

Teodor:

please don't call BufferManager.getInstance() - get it from WorkArea.tm.
in validateBatchModeInvocation, please return null instead of an empty lambda.

#21 Updated by Teodor Gorghe 10 months ago

Committed revision 16128 on task branch 10455a:
- Addressed code review #10455-20.

#22 Updated by Alexandru Lungu 10 months ago

Status changed from Review to Internal Test

I am OK with the changes in 10455a.

#23 Updated by Constantin Asofiei 9 months ago

Status changed from Internal Test to Merge Pending

This can be merged after 10336b.

#24 Updated by Teodor Gorghe 9 months ago

Status changed from Merge Pending to Test

Branch 10455a was merged to trunk rev. 16231 and archived.

#25 Updated by Teodor Gorghe 8 months ago

Status changed from Test to WIP
% Done changed from 100 to 50

There is a regression regarding this task.
When the BUFFER-COPY ASSIGN on a buffer on the same table with unique fields (having the unique fields copied) and on the first instruction, there is a function which is being called, 4GL allows this, but FWD raises the validation error.

Code:

FUNCTION func RETURNS INTEGER:
    MESSAGE "Reached!".
    RETURN 2.
END FUNCTION.

FOR EACH adaptivept4:
    DELETE adaptivept4.
END.

DEFINE BUFFER b1 FOR adaptivept4.

CREATE adaptivept4.
adaptivept4.k1 = 1.
RELEASE adaptivept4.

FIND FIRST adaptivept4 WHERE adaptivept4.k1 = 1 NO-ERROR.
CREATE b1.
BUFFER-COPY adaptivept4 TO b1 ASSIGN b1.k1 = DYNAMIC-FUNCTION ("func").

I think I need to make the validate flag to be false and when there is an assign for a buffer, set this flag to true.

#26 Updated by Teodor Gorghe 8 months ago

Created task branch 10455b.

#27 Updated by Constantin Asofiei 8 months ago

Teodor, please check with 12.x this test:

def temp-table tt1 field f1 as int index i1 is unique f1.

function func0 returns int.
message "here".
 return 2.
 end.

function func1 returns int.
   message "func1".
   return 1234.
end.

create tt1.
tt1.f1 = 1.
release tt1.
def var i as int.
def buffer btt1 for tt1.
find first tt1.
create btt1.
buffer-copy tt1 to btt1  assign tt1.f1 = dynamic-function("func0") i = func1().
message i. // 1234

With 11.7 this shows these messages:

** btt1 already exists with  1. (132) 
Cannot execute user-defined function, method or property accessor 'func0' in an ASSIGN statement after an indexing error. (13917)

but it does not prevent the further assignments to be executed - just this func0() call fails.

#28 Updated by Constantin Asofiei 8 months ago

Note that ErrorManager.recordOrThrowError(newErrorCodes, newErrorMessages, false); is called in RecordBuffer.reportBatchValidationException - this doesn't seem right considering the above.

#29 Updated by Teodor Gorghe 8 months ago

Please try this:

def temp-table tt1 field f1 as int index i1 is unique f1.

function func0 returns int.
message "here".
 return 2.
 end.

function func1 returns int.
   message "func1".
   return 1234.
end.

create tt1.
tt1.f1 = 1.
release tt1.
def var i as int.
def buffer btt1 for tt1.
find first tt1.
create btt1.
buffer-copy tt1 to btt1  assign btt1.f1 = dynamic-function("func0") i = func1().
message i. // 1234

The only difference is that btt1 is used instead of tt1 on the first instruction from assign.

#30 Updated by Constantin Asofiei 8 months ago

Hm... you are right, this works. But, how can we differentiate that we are in an assignment for a target buffer field? Because RVALUE is evaluated first and only after that we assign it.

We may need conversion changes for this?

Also, what I mentioned related to throwError vs showError needs also to be fixed (if is not working in FWD).

#31 Updated by Teodor Gorghe 8 months ago

I have also tried this:

def temp-table tt1 field f1 as int index i1 is unique f1.

function func0 returns int.
message "here".
 return 2.
 end.

function func1 returns int.
   message "func1".
   return 1234.
end.

create tt1.
tt1.f1 = 1.
release tt1.
def var i as int.
def buffer btt1 for tt1.
find first tt1.
create btt1.
buffer-copy tt1 to btt1  assign tt1.f1 = dynamic-function("func0") btt1.f1 = dynamic-function("func0") i = func1().
message i. // 1234

The return message:

here
here
** btt1 already exists with  1. (132) 
Cannot execute user-defined function, method or property accessor 'func1' an ASSIGN statement after an indexing error.
** btt1 already exists with 2. (132)
** btt1 already exists with 2. (132)
** btt1 already exists with 2. (132)

#32 Updated by Teodor Gorghe 8 months ago

Status changed from WIP to Review
% Done changed from 50 to 80
reviewer deleted (~~Alexandru Lungu~~)

Committed revision 16296 on task branch 10455b:
- Fixed BUFFER-COPY BATCH ASSIGN when a function call is the first instruction.

Constantin, please review the changes.
These are some runtime changes, which keeps most of the 10455a and also fixes the regression.
The conversion part, which solves the problem described on #10455-29 and #10455-30, I need to know if this should be fixed in this moment, but the current changes fixes the issue from customer application.

#33 Updated by Constantin Asofiei 8 months ago

For show vs throwError I think there are two cases:

if validation fails and LVALUE is the buffer (or field?) which fails validation, then it does throw

buffer-copy tt1 to btt1 assign  btt1.f1 = dynamic-function("func0") i = func1(). // validation is done on 'func1' call...

if validation fails and LVALUE is anything else, then it does show (the #10455-27 scenario, validation is done on 'func0' call)

Also, does the original test scenario still work?

#34 Updated by Teodor Gorghe 8 months ago

Constantin Asofiei wrote:

Also, does the original test scenario still work?

Yes, they still work, I have tested yesterday before committing the changes.
I will do some additional tests on 4GL and FWD, because is not quite clear for me.

#35 Updated by Teodor Gorghe 8 months ago

I am thinking on something else:

when the assigned buffer is the destination buffer, or something else, a variable, and the validation fails on the function call, it will throw the error.
when the assigned buffer is the source buffer, before calling the function, it will do the validation on btt1 (buffer-copy tt1 to btt1 assign tt1.f1 = 1), and then on the function call. What I have noticed is that when the assignation to a constant is used, it does THROW and when a function call is used, it does SHOW and assigns value ? to the source buffer. If the field is mandatory, it will throw that constraint violation message.

I think for now, I shall change the error back to be thrown and think about a solution to actually have this information when evaluating the RVALUE.

#36 Updated by Teodor Gorghe 8 months ago

Committed revision 16297 on task branch 10455b:
- Changed reportBatchValidationException to throw the error.

#37 Updated by Teodor Gorghe 8 months ago

Committed revision 16298 on task branch 10455b:
- Set the batch mode validation to false when it is in bulk copy.

#38 Updated by Constantin Asofiei 7 months ago

Please see the unit test in #9457-163, 10455b does not solve it.

#39 Updated by Teodor Gorghe 6 months ago

I am back to office. I am checking this right now, with my setup.

#40 Updated by Teodor Gorghe 6 months ago

I can confirm that the issue still occurs, I am working on a fix right now (maybe the last commit is not right).

#41 Updated by Teodor Gorghe 6 months ago

Please double check the patch. I was using an old patch. Now the case from #9457-163 is working.

#42 Updated by Teodor Gorghe 6 months ago

Rebased to trunk rev 16358

#43 Updated by Constantin Asofiei 6 months ago

Merge to trunk after 10203a

#44 Updated by Teodor Gorghe 6 months ago

Status changed from Review to Test
% Done changed from 80 to 100

Branch 10455b was merged to trunk rev. 16392 and archived.

#45 Updated by Constantin Asofiei 6 months ago

Status changed from Test to Closed

Also available in: Atom PDF

Project

General

Profile

FWD » Core Development » Database

Issues

Custom queries