FWD » Core Development » Database

Summary¶

A presort query took a fixed, preselected snapshot of its results, so records created or changed after the query opened were never seen. This makes PresortQuery change-sensitive by inheriting AdaptiveQuery instead of PreselectQuery, so it can switch to dynamic retrieval when the session holds pending changes.

Three files, +91/-10.

The change¶

PresortQuery now extends AdaptiveQuery. Its createResults override became createHighLevelResults (see below), assembleOrderByClause was split so the body keeps its checked exception while the override matches AdaptiveQuery's signature, and executeQuery now asks for a preselected result set explicitly on the template-rowid path.

AdaptiveQuery gained one hook, createHighLevelResults, called from createResults in place of the inline super.createResults(adapter). The default implementation is exactly what it replaced, so AdaptiveFind and every other caller are unaffected; PresortQuery is the only override.

PreselectQuery gained executePreselected, split out of executeQuery, so a subclass whose parent executes lazily can still ask for a fully materialised result set. Java offers no way to reach a grandparent implementation directly.

Why the results wrapper moved¶

This is the part worth a close look. SortedResults materialises and sorts in its constructor and never re-reads its delegate. Building it per delegate change meant that once the query went dynamic, every next() rebuilt the presorted view over a cursor that had already moved, and rows were lost. Building it once per open, over the adapter rather than the delegate, is what AdaptiveQuery already does for its own results; the hook simply lets a subclass participate in that.

The wrapper is cached for the life of an open, and cleared on re-open through open() -> close() -> cleanup().

Testing¶

Patterns covered, in FWD and against an OpenEdge reference where the semantics were in question:

• an uncommitted CREATE that matches the predicate, read from another buffer
• an uncommitted UPDATE that moves a row into or out of the predicate
• a pending change to a field the query SORTS on, and to a field it BREAKS on, where the row still matches the predicate so only its position and group can be wrong
• re-opening a presort statement, which must rebuild the presorted set rather than replay the previous open
• an existing corpus of BY/WHERE index-selection queries in both presort and preselect shapes, as a regression guard

The one remaining failure in the first suite fails on trunk as well and is out of scope here. The BY/WHERE failing-method set is identical to trunk, method for method. One test was added for the re-open case and passes, bringing that suite to 46.

Alex, please review the first iteration of the implementation in 10806a.

Will do a few more rounds of self review and generate more tests.

The remaining failing tests are:

They also fail with trunk.
The branch is at rev 16767.

Review of 10806a / r16767

• Presortable.java
  • line 11: missing H entry and (C) year update. When here, please upgrade the H header format to 110 chars/line (-T- column should also be dropped);
• Presorter.java
  • lines 234-283: something does not seem fine here. First: at 238/240 it seems more natural to say: if (!criteriaMatchAt(criteria, offset)) return false; and return true;, at the end, if criteria from all offset passed. I could not fully understand the logic of criteriaMatchAt(): it iterates sortHelpers, and compare then with SortCriterion starting at offset?
  • line 1495: stray javadoc.
  • line 1525: method backingReplaced() is missing its javadoc. Is it the one from line 1495?
• AdaptivePresortQuery
  • line 6: Copyright (c) should be 2026 only, -JPRM-- column should be dropped.

Note:
This is a complex update, involving both conversion and runtime. It is difficult for me to understand the reasons for some changes without seeing the code in action. So this is the result of first passes through the branch. I need to look again at Presorter and especially to the new AdaptivePresortQuery for more accurate review. Maybe use the AI for suggestions. I'm keeping the task to review state, but feel free to address the few issues I have already found.

This was not intended. Sorry.

testPostOpenDeleteOfUnvisitedRow — fixed

Pattern. FOR EACH … BREAK BY si2 over two rows (BBB, CCC), converted to
AdaptivePresortQuery. BBB is consumed, then CCC — unvisited, and the last row — is
deleted and committed. OpenEdge does not advance past a run of deleted rows at the end
of the list: it hands back BBB once more (rows=2 seq=BBB,BBB, NUM-RESULTS 2) and
only the next move ends the walk. FWD advanced past the end and stopped (rows=1).

Measured across three shapes over the same fixture: a plain AdaptiveQuery and a frozen
PresortQuery both end the walk on OpenEdge, and FWD matched both. Only the
index-satisfiable presort diverged.

Fix. Not buffer lifetime — the record is re-fetched every move. It is the position.
In AdaptiveQuery.next's missing-record retry loop, when the skip exhausts the list,
restore the position to the row before it and deliver that row again:

  if (!avail && skipped && !deadEndDelivered && redeliverRowBeforeSkippedOffEnd() &&
      rowBefore != null && rowBefore >= 0 && results.setRowNumber(rowBefore))
  {
     avail = true;
     redelivered = true;
     deadEndDelivered = true;
  }

This was the last failing test, the final score is trunk 30/125 failures, 10806 2/125. The 2 remaining failures are also in trunk and are not in scope.

Alex or Ovidiu, please review 10806a/16769.

Indeed, the latests revision should fix the issue from review. I think you can start testing. Be sure it passes the full set of tests; due to nature of the changes, all projects are affected.

The %done is still at 50%. Is this accurate? If not, please update.

Thank you, Ovidiu.
Will start with multi session tests, since the new query must be sensitive to other users changes too, then move on to standard regression testing.

Multi session tests identified a few issues.
1. Regression: presort walk truncates at the first fetch bracket

• delete one row ahead of the cursor — 9 rows delivered of 402, NUM-RESULTS 11
• create one row behind the cursor — 10 rows of 403, NUM-RESULTS 11

A commit landing past the end of the set is unaffected. The boundary is ProgressiveResults.BASE@ = 10, the first fetch bracket. NUM-RESULTS = 11 is a correct report of what was resolved, not a separate bug.

Impact: a loop over a presort query silently processes ten rows instead of four hundred, with no error. Not reachable on trunk, where PresortQuery materialises at the open.

2. Pre-existing: dead-end re-delivery does not fire cross-session

When another session deletes every row ahead of the cursor, the row before them is not re-delivered. Fails on trunk and on the branch; AdaptiveQuery.redeliverRowBeforeSkippedOffEnd covers the single-session case only.

Working on fixing the issues and extending the multi session test suite.

Adding back Alex as reviewer. It was not in my intention to remove him from the list, but somehow, that just happened.

Issues the multi-session tests found and solved

1. The walk truncated at the first fetch bracket. Any committed change by another session at or behind the resolution point killed the walk at ten rows — 9 rows delivered of 402, no error. A loop over a presort query silently processed ten rows instead of four hundred.

2. The dead-end re-delivery didn't fire cross-session. When another session deleted every row ahead, the walk delivered nothing instead of re-delivering the row before the dead end. Cause: the rebuilt backing handed back rows whose records were gone, so the query reported a result the caller couldn't read.

Both were reachable only from the branch's incremental path, and only cross-session — single-session probes pass. Neither was visible to the existing suite, whose post-open tests all use 2–4 row fixtures and never cross a bracket boundary.

Also found, not fixed: PC06 — with an OUTER JOIN, a foreign delete of the joined row drops the driving row. Fails on trunk too, so it's pre-existing and outside this ticket.

The fix

Three files; every existing presort query keeps today's code path.

- Presortable.consumeBackingReplaced — new default method returning false. Nothing else implements it.
- AdaptivePresortQuery — raises the flag in resetResults, already on the invalidation path. Asking consumes it, bounding the retry to one per replacement.
- Presorter.IncrementalResults.loadRow — on next() == false, retry instead of tearing the backing down if the query says it was replaced; and skip a row none of whose components resolved to a record.

Please review rev 16771.
Meanwhile I will continue testing .

Thank you, Stefanel. I am on it.

Artur,

I think the code changes are fine. But I guess the full set of tests needs to be redone.

• in Presorter.java, the methods criteriaMatchAt() and sortsBefore() use two parallel iterators: enhanced for on sortHelpers and i. I think we discussed this and I understand that semantically these should be synchronized (for example, in sortsBefore(), values[i] will not raise offset exception because values was constructed using sortHelpers.size()), but this is not evident locally. I think a short note should be added to javadoc.
• there is one thing I missed in previous review, when I was focused on code changes rather than on comments: in the header of Presortable.java, the -T- column should be dropped, nut not the --JPRM--. Although that is deprecated now, it is still valuable information. As opposed to T, whose values were evident. Keep the max width to 110 chars.

Review from ai:

• [CRITICAL] functional AdaptiveQuery.retainResultsOnReset: The test highLevelResults instanceof SimpleResults also matches FilteredResults (class FilteredResults extends SimpleResults), which PreselectQuery.createResults returns for every plain AdaptiveQuery with a client-side where. Previously resetResults called FilteredResults.cleanup() (rows.clear() + reset()); now the cleanup is skipped, resultsNeedBacking re-executes into the adapter, createResults hands back the same FilteredResults and setResults skips cleanup because the object is identical. FilteredResults.first()/next() serve the stale cached rows before falling through to the new delegate. Trigger: CompoundQuery calls query.reset(!resolveOnce, false) on each inner component every loop pass (CompoundQuery.java:2381, 4242), so in FOR EACH a, EACH b WHERE b.k = a.k AND <client-side expr> the inner query for outer row N returns the rows cached for outer row N-1, then its own. Key the exemption on the presort layer (a flag/override in AdaptivePresortQuery), not on SimpleResults.

I warned about something similar in my previous requirements:

plenty of code has instanceof AdaptiveQuery or similar.

I should have been more specific, but AQ and PQ have plenty of "hard-coded" checks to identify certain query states. Checking if the results are dynamic using results instanceof DynamicResults, instead of checking whether the query is in preselect/dynamic mode. As the Presort feature is being ported to this infrastructure, most of the instanceof require second pass to see if they still fit the logic.

• [MAJOR] functional AdaptiveQuery.sessionEvent: The SESSION_CLOSING_NORMALLY guard became !resultsCached && !(highLevelResults instanceof SimpleResults), which (same root cause) matches FilteredResults. For a scrolling client-where AdaptiveQuery (OPEN QUERY ... SCROLLING / browse walked with GET NEXT across a committed transaction), TRANSACTION_COMMITTING already breaks on isScrolling(), and now session close no longer stores reference rows nor calls resetResults(). The adapter stays connected with its pre-close delegate (resultsNeedBacking() is false), so the next GET NEXT reads a ScrollingResults JDBC cursor over a session that Persistence closes or marks reclaimable, and the cursor is no longer released at session close. Apply the same presort-specific test as for retainResultsOnReset.

Same mention as above. Mentioning SESSION_CLOSING_NORMALLY may raise us a complete new perspective; are the presort query results invalidated on commit as they might be forward-only? If so, do we need caching BatchResults on top of the extracted FilteredResults?

• [MAJOR] functional AdaptivePresortQuery.finishInitialize: setScrolling() is always called and the conversion change withholds setNonScrolling for indexSatisfiable FOR EACH presorts. On UNDO, RETRY, TransactionManager calls PreselectQuery.retry() (sets betweenRows = true); the re-run body then calls next(), but AdaptiveQuery.next() honors betweenRows only when !isScrolling() (~line 2441) and neither AdaptiveQuery nor AdaptivePresortQuery overrides retry, so results.next() advances. Trigger: FOR EACH customer BREAK BY customer.name: UPDATE customer.credit-limit. IF ... THEN UNDO, RETRY. END. (index on name) skips the retried row and proceeds with the next one. Regression vs the previous PresortQuery path, which was non-scrolling via the grafted setNonScrolling and whose PreselectQuery.next() (line 3621) honors betweenRows regardless.

To be investigated.

• [MAJOR] functional Presorter.IncrementalResults.sessionEvent: On the first TRANSACTION_COMMITTING during the walk, all remaining rows of the backing (the AdaptiveQuery adapter) are drained into a SimpleResults, resultsCached = true is set and the listener deregisters; afterwards loadRow reads only the frozen snapshot, so later resetResults/consumeBackingReplaced handling never reaches it. The patch edits this drain block but adds no guard for the adaptive backing. Trigger: FOR EACH t EXCLUSIVE-LOCK BREAK BY t.name: (indexed name) whose body updates t (one transaction per iteration) and later creates a row name = "ZZZ" past the read-ahead band: OE delivers it (per the patch's own 12.8 note in behindReadAhead: "CCX, DDX and ZZZ all are"), FWD never does. Change sensitivity of AdaptivePresortQuery effectively ends at the first commit, and the rest of the table is read eagerly at that point.

Same comment as above regarding results caching. Please create some performance tests in this idea to ensure that the caching is done in a performant way for presort queries (if this is the case).

• [MAJOR] functional AdaptivePresortQuery.createHighLevelResults: The client-side where filter is applied after sorting (super.createHighLevelResults(presorted) → PreselectQuery.createResults wraps the presorted layer in FilteredResults when isClientWhere()), the reverse of PresortQuery.createResults (filter first, then createIncrementalResults/createSortedResults). breakRows are recorded against unfiltered row indexes while Presorter.isFirstOfGroup/isLastOfBreakGroup/isLast look up getSortedResults().getRowNumber() from the filtered layer. Example: sorted rows A(si2=1, filtered out), B(si2=1), C(si2=2) give breakRows = {0, 2}; after filtering B=0, C=1, so LAST-OF for B and FIRST-OF for C both return false. Trigger: FOR EACH t WHERE t.si1 = 1 AND <expr not executable server-side, e.g. a UDF call> BREAK BY t.si2 with index (si1, si2); index_selection runs before where_clause_prep2 (annotations.xml:676/681) so index_satisfiable_sort ignores client-side where. Apply the filter below the presort layer (mirroring PresortQuery) or exclude client-where queries from index_satisfiable_sort.

to be investigated.

• [MAJOR] performance AdaptivePresortQuery.finishInitialize: Single-table FOR EACH ... BREAK BY with an index-satisfiable sort previously converted to PresortQuery + setNonScrolling(), i.e. a non-scrolling IncrementalResults holding 3 rows with breakRows trimmed to the last 2 and no cursor. It now converts to AdaptivePresortQuery without the graft, so setScrolling() stands: IncrementalResults.addRow appends every row's Long[] ids, breakRows sets are never trimmed, and the CursorWrapper installed by AdaptiveQuery.setScrolling() records ids for every row. A forward-only report loop goes from O(1) to several O(n) structures. The rules comment acknowledges giving up the ForwardResults optimisation, but nothing bounds this for large reports; consider a forward-only mode for this shape.

to be investigated.

• [MINOR] functional Presorter.IncrementalResults.behindReadAhead: The read-ahead band holds only the BREAK/BY criteria values, and sortsBefore treats a full tie as "not behind". Trigger: index (name, seq), FOR EACH t BREAK BY t.name, cursor on ("BBB", x), read-ahead ("CCC", 5); the body creates ("CCC", 1), the query is invalidated, the replacement resumes after the cursor in (name, seq) order and yields ("CCC", 1), which is not in resupplied and is appended after the cached ("CCC", 5) — delivered out of index order, whereas the patch's own model ("passes over anything behind the furthest row it has read") implies OE skips it. Break flags stay correct. The OE 12.8 observation quoted only covers the strictly-between BBX case; compare against the read-ahead row's full index key, or add an OE test for the tie case.

to be investigated.

• [MINOR] functional AdaptivePresortQuery.resetResults: backingReplaced is raised unconditionally, including from PreselectQuery.open (which calls resetResults() before execute()) and reset, while neither cleanup nor createPresortedResults clears it (its only consumer is Presorter.java:1715). Every open therefore builds a new IncrementalResults with the flag already set, and the first genuine end of the backing (including an empty result) is taken for the "between delegates" case in IncrementalResults.loadRow, issuing one redundant next() on the exhausted ResultsAdapter/ProgressiveResults (extra moveTo/bracket lookup) per open. Results stay correct. Clear the flag in cleanup or after createPresortedResults.

to be investigated.

There are also plenty of style issues you can regenerate I suppose using the AI FWD review skill.

I will try to find testcases for the technical issues.

Changes in the branch:
16793 -- Keep a presorted result list alive across the resets beneath it

A presorted layer is built once per open and the caller walks it, so the parent must not empty or rebuild it when the backing underneath is replaced. Four places assumed otherwise.

• AdaptiveQuery asked whether its results were materialised by testing for SimpleResults, which also matches FilteredResults -- a filter over the delegate, not a materialised view, and one which must still be cleaned up. The question is now put to the subclass through retainResultsOnReset.
• AdaptivePresortQuery answers it, and clears the backing replacement flag when the open state is discarded.
• PreselectQuery no longer cleans up the previous results when they are materialised: a re-execution can reach that path from underneath a walk, and emptying the list leaves the caller walking nothing.
• Presorter drains the delegate the backing already holds when a transaction commits, rather than asking the adapter for rows, which re-entered provideNewResults and handed back a rewound delegate.

Two behaviours measured against OpenEdge 12.8 rather than inferred: a client-side where belongs below the presorted layer, since break rows are recorded against the layer's own row numbers while the break flags look the current row up in whatever the caller is walking; and re-delivery of the row before a dead end depends on how the walk is driven, not on the query -- a named query walked with GET NEXT hands that row back while an inline FOR EACH block ends the walk.

16794 -- Give a presorted walk a read position, and measure it in the whole sort phrase

A BREAK walk reads one row past its cursor to answer LAST-OF, and which side of that read position a changed row falls on decides whether the legacy system delivers it. Nothing represented that position: a replacement backing resumed at the cursor, so the rows between the two were offered again for the layer to recognise, which it could not do for a row whose key tied with one it was holding.

• AdaptiveQuery gained resumeAnchorIds and resumeAnchorByScan, both defaulting to the previous behaviour, plus rowMatches; the replacement may now be advanced to the anchor rather than only tested against its first row.
• AdaptivePresortQuery answers with the furthest row its layer has read, which Presorter exposes as furthestRowRead.
• Presorter.useSortPhraseForBand widens the read-position band from the BREAK criteria to the query's whole sort phrase, so rows tying on si1 are still ordered by si2. The band's resolvables take the DMO proxy the criteria already held; a RecordBuffer is also a DataModelObject and would compile, but resolves its getters against no record. The recid component SortCriterion.parse appends is skipped.

Testing:
Measured against OpenEdge 12.8 self-service, 226 comparable tests: trunk 170, this branch 218, no regressions. The branch fixes 48 of trunk's 56 failures; the 8 remaining fail identically on trunk.