FWD » Core Development » Database

There is an issue with the invalidation of AdaptiveQuery instances that are generated by the optimizer. It seems like CQ.serverJoinAdaptive is creating a new AdaptiveQuery that is initialized, but the initialization is not registering the query to the change broker because it does not have records buffers yet; the buffers are created later on in CQ.serverJoinAdaptive. So, these AQ are not registered properly to the ChangeBroker:

USING Progress.Lang.*.
using OpenEdge.Core.Assert.

BLOCK-LEVEL ON ERROR UNDO, THROW.

CLASS Test:

    DEFINE TEMP-TABLE tt FIELD f1 AS INT INDEX idx AS UNIQUE F1.
    DEFINE TEMP-TABLE tt2 FIELD f1 AS INT INDEX idx2 AS UNIQUE F1.

    @SetUp.
    METHOD VOID before():
        def var i as int.
        empty temp-table tt.
        empty temp-table tt2.
        repeat i = 1 to 10:
            create tt2.
            tt2.f1 = i.
            release tt2.

            if i = 3 or i = 8 then next.
            create tt.
            tt.f1 = i.
            release tt.
        end.
    END.

    @Test.
    METHOD VOID test():
        def query q for tt, tt2 scrolling.
        open query q for each tt, first tt2 of tt.
        get first q.
        Assert:Equals(1, tt.f1).
        Assert:Equals(1, tt2.f1).
        get next q.
        Assert:Equals(2, tt.f1).
        Assert:Equals(2, tt2.f1).

        create tt.
        tt.f1 = 3.
        release tt.

        get next q.
        Assert:Equals(3, tt.f1). // this is not found!!!!
        Assert:Equals(3, tt2.f1).
    END.
END.

I recall this passing when working on #9724, so I am quite sure this is a regression. Please let me know if it looks familiar.

My temporary fix was to refactor CompoundQuery.maybeOptimize:

if (deferOptimization && components.size() < originalComponents.size())
         {
            // register the new, optimized queries as change listeners; if a query was left
            // unoptimized, the change broker is tolerant of a listener being added multiple times
            registerRecordChangeListeners();
         }

to

registerRecordChangeListeners();

I would personally like to know if there was a change somewhere else that regressed this so we can assess this better.

It is not enough apparently. Even if I force the multi-table AQ to invalidate, it will get to the absolute first row. In other words, there is no AQ.setWorkerRefererenceRecord for AQ.createCompoundQuery.
Now I really think something was changed in the invalidation of multi-table AQ and it is severely broken.

+ Stefanel

FYI, there is CQ.stateChanged that sets useSnapshot, initReferenceRowSupport and SnapshotParameterFilter to be used to handle optimized CQ invalidation. Still, I am not certain they reflect well within the optimized query implementation.

Made an update on 10962a/rev. 16541:

• Fixed the blurred line between storeReferenceRow and overrideReferenceRow. AQ is now consistently relying on storeReferenceRow before constructing dynamic query after invalidation.
• I really wanted to have the same pattern for CQ, but didn't managed to have it like so. For AQ, there was a convenient "invalidateIfReferenceRowExists" that is called for each next/prev, etc. For CQ, this is not the case:
  • for scrolling cases, CQ relies that the underlying sub-queries have the right state at the tip of the cursor. So there is no need to "invalidate"; the subqueries got invalidated anyway.
  • for indexed reposition cases, CQ relies the the underlying sub-queries are at one end, but the iteration can happen towards the other. I this case, I am still stuck with the manual reposition to the other end so that results are consistent. So all the work around referenceRow I did was only for AQ (even multi-table), but I couldn't make it work for CQ as well :/
• Currently, there are still some tests that fail I need to investigate. I need to understand how my changes on cursor reference row destined for AQ are now influencing the CQ.
• I re-enabled revalidation after indexed reposition.
• I fixed a weird bug in which deleting a record from a CQ query buffer, then calling NEXT, would skip till the deleted record is no longer in the buffer (4GL). In FWD, the deleted record messed up the query and started from first again. This is unrelated to my changes, but I spot it now.

This is still WIP; trying to stabilize. I am clear on what I can and cannot do ATM. I will drop the effort of using reference-row (front/back version) to manage CQ uniformly as AQ.

Made an update on 10962a/rev. 16542:

• Stabilized 10962a. The biggest issue was the lack of support of indexed-reposition in conjucation with revalidation. After breakValue was set, certain invariants were broken for indexed-reposition (e.g. run first/previous on indexed-reposition query that has a break values).
• My unit tests are now sound.
• Large customer unit tests are sound.
• GUI testing was almost flowless. I still have a browse which, while it scrolls upwards, it repeats rows. All other use-cases are working completely fine.

To simplify the process, I plugged in the 10962a branch through an AI reviewer. I am assessing this now ATM.

• [MAJOR] style AdaptiveQuery.header: Existing history entry "172 CA 20260506 When a AdaptiveQuery gets invalidated and moved to RAQ..." was deleted from the file header, and a new entry with the same sequence ID (172) was added; existing history must not be removed and sequence IDs must remain unique.
• [MAJOR] style AbstractCursor.header: The history entry "AL2 20260223 Extracted loadReferenceRow to be used with overrideReferenceRow." is duplicated; it already appears under entry #001 and was re-added under entry #002.
• [MAJOR] style AbstractCursor.imports: Unused import java.util.function.Consumer added at line 77.
• [MAJOR] style AbstractCursor.imports: Self-import com.goldencode.p2j.persist.AbstractCursor.ReferenceRowAssigner at line 81 is redundant — importing a class's own inner type from within the class is unnecessary and should be removed.
• [MAJOR] style RandomAccessCursor.moveQueryAtBackEnd: Method body uses 4-space indentation instead of the required 3-space indent.
• [MAJOR] style AbstractCursor.BackReferenceRowAssigner: Extraneous semicolon after closing brace of inner class };; should be just }.
• [MAJOR] style RandomAccessCursor.FrontReferenceRowAssigner: Extraneous semicolon after closing brace of inner class };; should be just }.
• [MAJOR] performance RandomAccessCursor.storeReferenceRows: Unconditionally loads both back and front reference rows even when only one end has moved. The short-circuit (getTableCount() == 1 && backReferenceRow != null && frontReferenceRow != null) only fires for single-table queries, so multi-table queries always run two full loadReferenceRecord passes on every RecordChangeEvent. Per-slot isNullAt checks avoid redundant DB loads when slots remain populated, but the unconditional double-pass still wastes work; consider a per-end "dirty" flag.
• [MAJOR] functional AdaptiveQuery.setReferenceRecord: Method dereferences referenceRecord.primaryKey() without a null-check. The Joinable.setReferenceRecord(Record) contract makes no non-null guarantee — peer implementation RandomAccessQuery.setReferenceRecord tolerates null, and the underlying backReferenceRow is documented to allow null entries (see resetReferenceRow using Arrays.fill(..., null) and BackReferenceRowAssigner.isNullAt). With the new multi-index caller setWorkerRefererenceRecord(Joinable, int) using persistable[index] for index > 0, a partially-populated reference row will trigger an NPE. Add a null guard at method entry or at the caller before dispatching.
• [MAJOR] functional CompoundQuery.stateChanged: Replaces cursor.peekLastLoadedResult() with cursor.peekLastLoadedResult(true) (hardcoded back-end). The lastLoadedResultIsAtEnd flag is flipped to false for backward iterations and front-end loads, so this comparison checks event PKs against the back-end row instead of the actual last-loaded row, causing useSnapshot[index] to be set incorrectly and ultimately producing wrong parameter selection in chooseParameters. The author's own "TODO: why peek last loaded result as true?" comment confirms this is unverified.
• [MAJOR] functional AdaptiveQuery.createCompoundQuery: After subQuery.reset(true, true) + forceDynamicOperation(), calls setWorkerRefererenceRecord(subQuery, i). When the worker subQuery is a PreselectQuery, PreselectQuery.setReferenceRecord emits LOG.severe("Attempting to set a reference record for the preselect query") unconditionally. Since preselect-as-worker is a supported case (per change 243: "a CompoundQuery can join a preselect with a RandomAccessQuery"), this produces SEVERE log spam on every AQ-to-CQ degradation involving a PreselectQuery. Either downgrade the log level, gate it behind LOG.isLoggable, or skip the call in setWorkerRefererenceRecord for PreselectQuery workers.
• [MAJOR] functional CompoundQuery.loadByValues: The ConditionException catch sets activeIndex = -1 (when loadAll) before restore = true; throw exc;. The new finally block then unconditionally overwrites activeIndex with oldActiveIndex (because restore == true), silently erasing the -1 catastrophic-failure marker that the original code preserved and that other paths in this class (e.g., retrieveImpl) use to signal "compound query state is invalidated." Apply the -1 after the finally, or use a separate flag so the ConditionException path skips the restore.
• [MINOR] style AbstractCursor.ReferenceRowAssigner: Javadoc typo "tru" should be "true" at lines 730 and 796.
• [MINOR] style RandomAccessCursor.FrontReferenceRowAssigner: Javadoc typo "tru" should be "true" at line 1289.
• [MINOR] style AbstractCursor.ReferenceRowAssigner: Blank line before closing brace } of the interface body violates "No blank lines before }".
• [MINOR] style AbstractCursor.loadReferenceRecord: Method signature wraps parameters but leaves trailing whitespace after commas; parameters should be aligned properly without trailing whitespace.
• [MINOR] style AbstractCursor.BackReferenceRowAssigner: Class declaration places implements ReferenceRowAssigner on its own line but has trailing whitespace on both the class-name line and the implements line.
• [MINOR] style RandomAccessCursor.FrontReferenceRowAssigner: Class declaration places implements ReferenceRowAssigner on its own line but has trailing whitespace on both lines.
• [MINOR] style AdaptiveQuery.setReferenceRecord: Method overrides Joinable.setReferenceRecord but is missing the @Override annotation (inconsistent with RandomAccessQuery).
• [MINOR] style PreselectQuery.setReferenceRecord: Method overrides Joinable.setReferenceRecord but is missing the @Override annotation.
• [MINOR] style RandomAccessCursor.isRandomAccess: Missing @Override annotation; this method overrides the parent AbstractCursor/Cursor declaration.
• [MINOR] style AdaptiveQuery.buildCompoundFromAdaptive: Trailing whitespace after if (this.cursor != null) (extra space before {).
• [MINOR] style AdaptiveQuery.previous: Trailing whitespace blocks added (e.g. try { and { at multiple places).
• [MINOR] style AbstractCursor.loadReferenceRecord: Line if (assigner.isNull()) has trailing whitespace before {.
• [MINOR] style ScrollingCursor.storeReferenceRows: Many lines added by this diff have trailing spaces (100+ trailing-whitespace additions across files), violating clean formatting.
• [MINOR] style AbstractCursor.header / RandomAccessCursor.header / ScrollingCursor.header: Newly added history lines include trailing whitespace.
• [MINOR] style CompoundQuery.header: Continuation history entry " 20260430 Prior to retrieve, the load should not corrupt the counters when failing." is missing the author initials (AL2) where other continuation entries include them.
• [MINOR] performance AbstractCursor.loadReferenceRecord: The refactor replaces direct array reads/writes in the per-buffer loop with virtual ReferenceRowAssigner interface calls (isNullAt, setAt) plus a field dereference through the assigner's cursor reference. With two implementations (BackReferenceRowAssigner / FrontReferenceRowAssigner) the call site is bimorphic and HotSpot can usually inline, but the abstraction does add hot-path overhead compared to the previous direct array access. Consider whether the indirection is needed when both implementations differ only in which array field they index.
• [MINOR] performance CompoundQuery.skipReset: The new method initializes counters with a manual for loop setting each element to 1. Replace with Arrays.fill(this.counters, 1); for consistency with the Arrays.fill usage already present in this diff for the reference-row arrays.
• [MINOR] functional RandomAccessCursor.moveQueryAtFrontEnd / moveQueryAtBackEnd: For non-CompoundQuery (single AdaptiveQuery cursor), the methods replace the previous iterative tryMoveAtEnd retry loop (introduced by change 003 specifically to defend against boundary-record deletions, with comment "Move to end could end up in infinite loop if a record was deleted") with a single storeReferenceRows(null) that probes only the last loaded result. If that boundary row is evicted/deleted, the reference row remains null and there is no retry across other cached rows; the query then falls back to activateNext/activatePrevious from a possibly-stale buffer.getSnapshot() while lastLoadedResultIsAtEnd has already been flipped, leaving the query positioned inconsistently with the cursor's expected end. The fallback activateFirst/activateLast path mitigates the worst case only when the buffer snapshot is also null.