FWD » Core Development » Database

The following need implementation in QueryWrapper:

public character indexInformation()
public character indexInformation(int n)
public character indexInformation(NumberType n)
public logical createResultListEntry()
public logical deleteResultListEntry()

Stanislav, if I'm putting your example into a OPEN QUERY case, the code can not compile in 4GL - it says that f2 can be resolved to multiple fields in tt2. I guess this is because it assumes f2 is an abreviation for either f21 or f22. Please test your scenario by either disambigating each field used in the query-prepare clause or renaming the fields so there are no abreviation collisions. Also, I think you should check how OPEN QUERY behaves in this case, too - you will need an associated DEFINE QUERY for INDEX-INFORMATION to work.

Constantin, yes, for static query it for some reason requires explicit fields specifications while works fine for dynamic query. Result is the same: 4GL picks idx21.

Guys, about deleteResultListEntry - how it should be implemented? My idea is to make query-specific list of recids that should be skipped on query iteration.
createResultListEntry doesn't present in the customer project. Should I implement it?

About deleteResultListEntry - it can be applied to
1. queries with preselected result set
2. scrolling queries or queries linked to browse.
While in preselected queries a deleted entry can be referenced thru record ids, for adaptive queries its not the case - we can meet this record later in the result set. As it seems to me, for adaptive queries progress has some preselected cache of ids, from which it removes an entry, so it's not clear how should I reference a deleted entry and it which cases we will be able to meet it again. I'll look deeper into that, but considering that we do not completely reproduce adaptive behavior, that can be a problem to implement it accurately. Do you have any ideas about a simple solution?

Stanislav Lomany wrote:

Eric, some issues about indexInformation:
3. For static queries I see three ways of solution:
- analyze where/sort clause at runtime and select an index using documented index selection criteria. Cons: code duplication with index_selection.rules.
- analyze where/sort clause, build, basically, original 4GL AST and pass it to index_selection.rules. Cons: seems complicated.
- my favorite: select indexes at conversion stage and, say, pass to query constructors. Cons: less pretty code.

I would like to avoid passing additional information into the constructor for the index in use, because we essentially already do this in the form of the sort clause. For every table/buffer involved in a static query, we have chosen an index to use during conversion, based on the index selection rules we have determined. This is represented in converted code as the sort clause applied to that query (or query component). We can (in fact, we already do, for FindQuery/RandomAccessQuery) map this clause back to an index (see the IndexHelper class). The missing piece is to map the selected index back to its legacy name, which we now can do based on the index annotations we have in the DMO classes. Ideally, we would only do this work on demand, in response to an indexInformation call, and cache it, rather than for every query.

Also you should take into consideration that this function provides 1. debug information 2. which usefulness is questionable since the tables are SQL-mapped.

This is true, but we can't know what decisions any given application will make based on the output of this attribute.

Stanislav Lomany wrote:

createResultListEntry doesn't present in the customer project. Should I implement it?

No. I cannot find a reference to it in the GUI project, either.

Stanislav Lomany wrote:

About deleteResultListEntry - it can be applied to
1. queries with preselected result set
2. scrolling queries or queries linked to browse.
While in preselected queries a deleted entry can be referenced thru record ids, for adaptive queries its not the case - we can meet this record later in the result set. As it seems to me, for adaptive queries progress has some preselected cache of ids, from which it removes an entry, so it's not clear how should I reference a deleted entry and it which cases we will be able to meet it again. I'll look deeper into that, but considering that we do not completely reproduce adaptive behavior, that can be a problem to implement it accurately. Do you have any ideas about a simple solution?

First, please explain what you mean by the adaptive behavior we are not completely reproducing. I guess you are referring to OPEN QUERY FOR ... cases (as opposed to OPEN QUERY PRESELECT ...), but we have taken great lengths to reproduce the same, dynamic behavior of Progress in this regard (in terms of changes to an index by the current session or by a separate session which trigger changes to the results an active query sees).

What behavior are you seeing in Progress that we are not performing, such that it is causing concern about how to implement this method?

I would like to avoid passing additional information into the constructor for the index in use, because we essentially already do this in the form of the sort clause. For every table/buffer involved in a static query, we have chosen an index to use during conversion, based on the index selection rules we have determined. This is represented in converted code as the sort clause applied to that query (or query component). We can (in fact, we already do, for FindQuery/RandomAccessQuery) map this clause back to an index (see the IndexHelper class).

Eric, I thought about it, but it seems unreliable to me. E.g.:

def temp-table tt
     field f1 as integer
     field f2 as integer
     field f3 as integer
     index idx1 f1
     index idx2 f2.

def query q for tt.
open query q for each tt where f1 > 0 by f2.

message query q:handle:index-information.

Yes, I see. The conversion seems correct, but the sort clause is lossy in this case, with respect to the selected index.

Go with your favored approach, then. Note that you will need to change a lot of constructors. Since this will be a mandatory parameter, you will want to put it near the front of the list of parameters, since there are so many variants when all the optional parameters are taken into account. Are there any concrete implementations of P2JQuery for which this parameter will not apply (i.e., with which indexInformation cannot be used)?

Are there any concrete implementations of P2JQuery for which this parameter will not apply (i.e., with which indexInformation cannot be used)?

AFAIK, no.

One more issue - if the index do not have bracketing, the first entry in the list is the CHARACTER string “WHOLE-INDEX,” and the second entry in the list is name of the index. E.g.:

def temp-table tt
     field f1 as integer
     field f2 as integer
     field f3 as integer
     index idx1 f1
     index idx2 f2.

def query q for tt.
open query q for each tt.

message query q:handle:index-information.

Stanislav Lomany wrote:

Are there any concrete implementations of P2JQuery for which this parameter will not apply (i.e., with which indexInformation cannot be used)?

AFAIK, no.

I think the 4GL query blocks (i.e. FOR EACH) do not allow access to INDEX-INFORMATION - currently, P2JQuery is the root class for all legacy-style queries (including blocks, FIND, etc). In #2120 (notes 71/72) there was some talk about enhancing the query infrastructure so that the functionalities accessed only via query resources are removed from P2JQuery and brought into another interface - we should have a task for this, if it's still in play.

Right. I thought we are talking about only about queries which can be used in OPEN QUERY and forgot that P2JQuery has a broader scope.

I guess you are referring to OPEN QUERY FOR ... cases (as opposed to OPEN QUERY PRESELECT ...),

By preselect queries I mean ones with use preselected result set rather than index walking (i.e. OPEN QUERY PRESELECT + subset of OPEN QUERY FOR).

What behavior are you seeing in Progress that we are not performing, such that it is causing concern about how to implement this method?

def buffer xbook for book.

do transaction:
   for each book:
     delete book.
   end.

  create book. book.book-id = 1. book.isbn = "isbn1". 
  create book. book.book-id = 2. book.isbn = "isbn2". 
  create book. book.book-id = 3. book.isbn = "isbn3".
  create book. book.book-id = 5. book.isbn = "isbn5". 
  create book. book.book-id = 6. book.isbn = "isbn6".  
end.

def query q for book scrolling.

open query q for each book.
def var i as integer init 1.
repeat:
  get next q no-lock.
  if not avail(book) then leave.

  if i = 2 then do transaction:
     find xbook where xbook.book-id = 1. 
     xbook.book-id = 4.
  end.

  i = i + 1.
end.

do transaction:
   find xbook where xbook.book-id = 4. 
   xbook.book-id = 1.
end.

get last q no-lock.
repeat:
  message string(book.book-id) + " " + string(book.isbn).

  get prev q no-lock.
  if not avail(book) then leave.
end.

6 isbn6
5 isbn5
1 isbn1
3 isbn3
2 isbn2
1 isbn1

6 isbn6
5 isbn5
3 isbn3
2 isbn2
1 isbn1

Do the results differ if the query is not set to be scrolling?

Yes, they differ and match P2J.

I am reviewing the 0604a update now (sorry it took so long for me to get to this). Could you please post some test case output, so I can see what the effect on the code is?

Code review 0604a:

Looks good, though I would still like to see sample output. Note there is another version of IndexHelper in play right now (#2186, note 50). Please merge with that version (it just has a method removed), then regression test. Please re-convert the customer server project as well, and spot check the converted source diffs there.

Testcase:

def temp-table tt
     field f1 as integer
     field f2 as integer
     field f3 as integer
     index idx1 f1
     index idx2 f2.

def temp-table tt2
     field f4 as integer
     field f5 as integer
     field f6 as integer
     index idx4 f4
     index idx5 f5.

def var i as integer init 0.
def var b as logical.

def query q for tt.
def query q2 for tt, tt2.
open query q for each tt where f1 > 0 by f2.
message "1: " + query q:handle:index-information.

open query q for each tt by f2.
message "2: " + query q:handle:index-information.

open query q preselect each tt where f1 > 0.
message "3: " + query q:handle:index-information.

open query q2 preselect each tt where tt.f1 > 0, each tt2.
message "4: " + query q2:handle:index-information(1) + " (2) " + query q2:handle:index-information(2).

open query q2 preselect each tt where tt.f1 > 0, each tt2 where tt2.f4 > i.
message "5: " + query q2:handle:index-information(1) + " (2) " + query q2:handle:index-information(2).

open query q preselect each tt where f1 > 0 by if b then f2 else f2.
message "6: " + query q:handle:index-information.

open query q2 for each tt where tt.f1 > 0, each tt2 where tt2.f4 > 0.
message "7: " + query q2:handle:index-information(1) + " (2) " + query q2:handle:index-information(2).

open query q2 for each tt where tt.f1 > 0, each tt2 where tt2.f4 > 0 by if b then f2 else f2.
message "8: " + query q2:handle:index-information(1) + " (2) " + query q2:handle:index-information(2).

open query q2 for each tt where tt.f1 > 0, first tt2 where tt2.f4 > 0 by if b then f2 else f2.
message "9: " + query q2:handle:index-information(1) + " (2) " + query q2:handle:index-information(2).

Converted code:

query0.assign(new PreselectQuery(tt, "tt.f1 > 0", null, "tt.f2 asc, tt.f1 asc", "idx1"));
query0.open();
message(concat(new character("1: "), query0.indexInformation()));
query0.assign(new AdaptiveQuery(tt, (String) null, null, "tt.f2 asc", "WHOLE-INDEX,idx2"));
query0.open();
message(concat(new character("2: "), query0.indexInformation()));
query0.assign(new PreselectQuery(tt, "tt.f1 > 0", null, "tt.f1 asc", "idx1"));
query0.open();
message(concat(new character("3: "), query0.indexInformation()));
query1.assign(new PreselectQuery("tt.f1 asc, tt2.f4 asc"));
query1.addComponent(tt, "tt.f1 > 0", "idx1");
query1.addComponent(tt2, ((String) null), "WHOLE-INDEX,idx4");
query1.open();
message(concat("4: ", query1.indexInformation(1), " (2) ", query1.indexInformation(2)));
query1.assign(new PreselectQuery("tt.f1 asc, tt2.f4 asc"));
query1.addComponent(tt, "tt.f1 > 0", "idx1");
query1.addComponent(tt2, "tt2.f4 > ?", "idx4", new Object[]
{
    i
});
query1.open();
message(concat("5: ", query1.indexInformation(1), " (2) ", query1.indexInformation(2)));
query0.assign(new PresortQuery(tt, "tt.f1 > 0", null, "tt.f1 asc", "idx1"));
query0.addSortCriterion(byExpr0);
query0.open();
message(concat(new character("6: "), query0.indexInformation()));
query1.assign(new CompoundQuery(true, false));
query1.addComponent(new AdaptiveQuery(tt, "tt.f1 > 0", null, "tt.f1 asc", "idx1"));
query1.addComponent(new AdaptiveQuery(tt2, "tt2.f4 > 0", null, "tt2.f4 asc", "idx4"));
query1.open();
message(concat("7: ", query1.indexInformation(1), " (2) ", query1.indexInformation(2)));
query1.assign(new PresortQuery());
query1.addComponent(tt, "tt.f1 > 0", "idx1");
query1.addComponent(tt2, "tt2.f4 > 0", "idx4");
query1.addSortCriterion(byExpr0);
query1.open();
message(concat("8: ", query1.indexInformation(1), " (2) ", query1.indexInformation(2)));
query1.assign(new PresortCompoundQuery());
query1.addComponent(new PreselectQuery(tt, "tt.f1 > 0", null, "tt.f1 asc", "idx1"));
query1.addComponent(new RandomAccessQuery(tt2, "tt2.f4 > 0", null, "tt2.f4 asc", "idx4"), QueryConstants.FIRST);
query1.addSortCriterion(byExpr0);
query1.open();
message(concat("9: ", query1.indexInformation(1), " (2) ", query1.indexInformation(2)));

Looks good, though I would still like to see sample output. Note there is another version of IndexHelper in play right now (#2186, note 50). Please merge with that version (it just has a method removed), then regression test.

OK.

Please re-convert the customer's server project as well, and spot check the converted source diffs there.

I've done that before posting the update.

Sample looks good, thanks.

Regarding the question if scrolling adaptive queries are modified with DELETE-RESULT-LIST-ENTRY in the customer app: I cannot say definitely because the code is complicated, but it seem to be the case. So it looks like we have to properly implement adaptive scrolling queries.

About caching. So far I can state that:
1. Adaptive scrolling queries have recid-based cache which is populated as we iterate records.
2. Once populated:
  2.1 records do not change their position if indexed fields are updated;
  2.2 deleted records are not displayed;
  2.3 new records are not added.
3. DELETE-RESULT-LIST-ENTRY deletes only one entry even if then same record is represented by several entries in the result list.

BTW, there is an issue with dynamic queries you probably aware of. Creation of a record using a backing buffer of a dynamic query may lead to incorrect results.

def temp-table tt field f1 as integer
                  index idx1 f1.

def query q for tt.
def var i as integer.

create tt. tt.f1 = 1.
create tt. tt.f1 = 2.
create tt. tt.f1 = 3.

open query q for each tt.
get next q.

create tt. tt.f1 = 4.

get next q.
message string(tt.f1).

Strange case. Testcase 1:

def temp-table tt field f1 as integer
                  index idx1 f1.
define buffer xtt for tt.

def query q for tt.
def var i as integer.

create tt. tt.f1 = 1.
create tt. tt.f1 = 2.
create tt. tt.f1 = 3.

open query q for each tt.
create xtt. xtt.f1 = 0.

repeat:
  get prev q.
   if query-off-end("q") then do:
      message "N/A". leave.
   end.
   else message string(tt.f1).
end.

Testcase 2:

def temp-table tt field f1 as integer
                  index idx1 f1.
define buffer xtt for tt.

def query q for tt.
def var i as integer.

create tt. tt.f1 = 1.
create tt. tt.f1 = 2.
create tt. tt.f1 = 3.

open query q for each tt.

def var created as logical init false.

repeat:
   if not created then do:
      create xtt. xtt.f1 = 0.
      created = true.
   end.

   get prev q.
   if query-off-end("q") then do:
      message "N/A". leave.
   end.
   else message string(tt.f1).
end.

I think what's happening here is that nothing is releasing/flushing the newly created record from buffer xtt before or during the repeat loop in the first case, so it doesn't appear in the query results. I believe if you assign a non-default value to the indexed field f1 (say, -1), or change the default value for f1 to something non-0, that would trigger a flush on the assignment. Also, an explicit release or validate statement after the assignment should do it.

Stanislav Lomany wrote:

There is no difference in testcases output if the queries are iterated in the forward direction (i.e. get next q is used).

Are you saying they both include the 0 record or they both exclude it?

You are right - it is about flushing.

Are you saying they both include the 0 record or they both exclude it?

Include. But in the NEXT-case I also has value 4 assigned instead of 0. That's why it worked.

BTW, P2J includes records in both PREV cases.

Stanislav Lomany wrote:

BTW, P2J includes records in both PREV cases.

I am working on a flush/validate fix that I hope will make this more consistent with Progress. Is not having this now getting in your way? I don't want us both working on the same problem.

No problem about having this issue. I just need to consider it when making testcases.

This issue I have to fix. LAST just do not work for adaptive queries.

def temp-table tt field f1 as integer
                  index idx1 f1.
define buffer xtt for tt.

def query q for tt.
def var i as integer.

create tt. tt.f1 = 1.
create tt. tt.f1 = 2.
create tt. tt.f1 = 3.

open query q for each tt.
get last q.

message string(query-off-end("q")).
message string(avail(tt)).

OK, but that doesn't look like a flushing issue to me, so I guess we will stay out of each others' way on this one.

While working on the LAST issue, I've looked into invalidation logic for adaptive queries. I think it's a bit doubtful, e.g.:

def temp-table tt field f1 as integer
                  index idx1 f1.
define buffer xtt for tt.

def query q for tt scrolling.
def var i as integer.

create tt. tt.f1 = 1.
create tt. tt.f1 = 2.
create tt. tt.f1 = 3.

open query q for each tt.
get next q.
get next q.
get next q.

create xtt. xtt.f1 = 4.

get prev q.

if avail(tt) then message string(tt.f1).
else message "N/A".

Found this in AdaptiveQuery javadoc:

 * <b>WARNING:</b>  currently, the multi-table implementation is not
 * functional.
 * <p>
 * TODO:  support multiple table queries.  The support is partially in place
 * in the DynamicQuery inner class;  however, the instance of
 * CompoundQuery which is created to drive the continuation of
 * the query after invalidation currently does not have its state set
 * correctly.
 * <p>

Stanislav Lomany wrote:

Found this in AdaptiveQuery javadoc:
[...]

Yes, I wrote that. All such cases are converted to use CompoundQuery instead, so you won't see a multi-table AdaptiveQuery come out of conversion currently. The multi-table implementation would be an optimization, but it is quite a complicated solution. Since we convert multi-table cases to CompoundQuery, there is no functional gap here, just a missing optimization.

Code review 0812a:

I think it looks OK, but this whole area of the code is tricky to assess with just a review. I'll feel more comfortable after the more extensive testing you have planned (plus regression testing eventually) is successful.

1. Cache is advanced in forward (if the first iteration was FIRST) or reverse (the first iteration was LAST) direction as we move thru the result set using GET NEXT/PREV or REPOSITION.
2. LAST resets cache if it was advanced in forward direction and changes caching direction to opposite. FIRST - if in reverse.
3. Reaching the opposite off-end freezes cache: new records can't be added to the end of the result set, cache can't be reset using FIRST/LAST.
4. REPOSITION TO ROW resets cache if it was advanced in reverse direction and changes caching direction to forward.
5. REPOSITION FORWARD/BACKWARD/TO ROWID doesn't change caching direction.
6. For adaptive scrolling queries NUM-RESULTS doesn't make any iterations and returns the number of records in this cache.
7. Rules from note 26 apply.

1. In P2J REPOSITION FORWARD/BACKWARD/TO ROWID doesn't always correctly detect off-end.
2. In P2J NUM-RESULTS returns the total number of records and I can't easily change its behavior to correct because it is used by P2J browse and some other places.

Stanislav Lomany wrote:

I finally got caching rules for adaptive queries (single-table):

1. Cache is advanced in forward (if the first iteration was FIRST) or reverse (the first iteration was LAST) direction as we move thru the result set using GET NEXT/PREV or REPOSITION.
2. LAST resets cache if it was advanced in forward direction and changes caching direction to opposite. FIRST - if in reverse.
3. Reaching the opposite off-end freezes cache: new records can't be added to the end of the result set, cache can't be reset using FIRST/LAST.
4. REPOSITION TO ROW resets cache if it was advanced in reverse direction and changes caching direction to forward.
5. REPOSITION FORWARD/BACKWARD/TO ROWID doesn't change caching direction.
6. For adaptive scrolling queries NUM-RESULTS doesn't make any iterations and returns the number of records in this cache.
7. Rules from note 26 apply.

Nice work. Please report the test cases you used to determine these rules and check them into the testcases project if you haven't done so already.

How close is P2J to matching these rules, or better said, how much effort do you think is required to get it to this point?

Some P2J concerns:

1. In P2J REPOSITION FORWARD/BACKWARD/TO ROWID doesn't always correctly detect off-end.
2. In P2J NUM-RESULTS returns the total number of records and I can't easily change its behavior to correct because it is used by P2J browse and some other places.

If browse and other runtime components depend upon the current implementation of NUM-RESULTS, we can refactor that implementation to another method to preserve it for internal runtime purposes, but create a correct implementation of NUM-RESULTS for use by converted code.

Are these last two points incidental issues you found along the way, or are they blocking the implementation of deleteResultListEntry?

Please report the test cases you used to determine these rules and check them into the testcases project if you haven't done so already.

See uast/adaptive_scrolling directory.

How close is P2J to matching these rules, or better said, how much effort do you think is required to get it to this point?

Single table - about 2-3 days.

Are these last two points incidental issues you found along the way, or are they blocking the implementation of deleteResultListEntry?

I think they do not block neither cache implementation nor deleteResultListEntry.

Rules from note 41 apply to multi-table queries as well.

1. off-end is "yes" if the position is < -0.5 of > cacheSize - 1 + 0.5 (note that strict operators are used), except the following case:
2. off-end is "no" if we have the full cache and the current off-end state is "no".

However I have two concerns:
1. Some code in browse widget and queries use _isOffEnd() and isOffEnd() functions to detect if we have reached the last result (i.e. position is < 0 of > cacheSize - 1). My suggestion is to make _isOffEnd() return "useful" off-end and isOffEnd() - 4GL value. Of course we should decide if we want to implement off-end quirks an this point.
2. There is the case for which I cannot find a logical explanation (and I don't know how to implement it):

def temp-table tt field f1 as integer
          index idx1 f1.
def query q for tt scrolling.

create tt. tt.f1 = 1.
create tt. tt.f1 = 2.
create tt. tt.f1 = 3.

open query q for each tt. get first q. get prev q.
reposition q to row 0. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* yes */

open query q for each tt. get first q. get prev q.
reposition q to row 1. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* yes */

open query q for each tt. get first q. get prev q.
reposition q to row 2. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* yes */

open query q for each tt. get first q. get prev q.
reposition q to row 3. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* no */

open query q for each tt. get first q. get prev q.
reposition q to row 4. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* no */

open query q for each tt. get first q. get prev q.
reposition q to row 5. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* yes */

open query q for each tt. get first q. get prev q.
reposition q to row 100. 
put stream s string(query-off-end("q")) " " string(current-result-row("q")) skip. /* yes */

Eric, there is a quirk in 4GL DELETE-RESULT-LIST-ENTRY implementation: for adaptive non-scrolling queries if DELETE-RESULT-LIST-ENTRY has been called before, following DELETE-RESULT-LIST-ENTRY calls will return "yes" (once) if the query is off-end. Should I implement this quirk at this point?

If in your estimation it is not more than a few hours of effort, then yes -- it doesn't sound like it would be a lot of effort from your description. Best to get it right now than to trip over it later.

I've checked the presence of PresortCompoundQuery in the customer app. It presents only in FOR EACH form, so there is no need to implement DELETE-RESULT-LIST-ENTRY for this type of queries. And even if I needed to implement it, I don't think this would be easy - I have some concerns about the way PresortCompoundQuery is implemented.

OK, please thoroughly document the problem in a separate issue in this project, including why it is specific to PresortCompoundQuery, details of your concerns about the way PresortCompoundQuery is implemented, and your ideas about how an appropriate solution should be implemented. Provide enough detail that you or someone else can pick up this work later. Do not set an assignee, target, or start date. Link it as a related issue to this issue.

Other than this, is DELETE-RESULT-LIST-ENTRY fully implemented now? Is there anything else left to do on this task besides review and regression test?

Code review 0928a:

I think the changes are OK, but please see my questions and comments below.

In note 45 above, you wrote:

Note that my decision to remove Cursor.offEnd field probably was a mistake.

It is still removed in this update. Why was it a mistake, and how did you compensate/correct for it? Have we lost any functionality as a result?

Please list the test case names you used for your manual testing and be sure they are checked into the testcases project. The Cursor logic is too complicated to know whether the changes are correct just from a review, so I will have to rely on the results of your manual testing and regression testing. However, I noticed that your changes to the Cursor.reposition(Serializable[], boolean) method are not reflected in the javadoc, which still indicates that the methods throws IllegalStateException. Please review the other methods you have changed and update their javadoc as necessary (and that of the class, if needed) to reflect any changes you have made to the logic.

Why are you calling Collections.reverse(results) at Cursor:851?

Why did you make Cursor.next() public (from private)? If this needs to stay this way, please group it with the other public methods.

As with Cursor, the AdaptiveQuery logic is too complicated to confirm your changes from just a review, so I have to rely on your testing. However, I noticed your additions to AdaptiveQuery deal with composite rows (i.e., arrays of ids to represent multi-table row sets). However, AdaptiveQuery is currently limited to a single table implementation -- addComponent enforces this at runtime, and conversion will not create multi-table instances of AdaptiveQuery. What was the intention behind making your changes multi-table? In your estimation, how much effort is left to fully enabling multi-table support? If we could get this right, it should be much more performant than CompoundQuery in many cases.

Other than this, is DELETE-RESULT-LIST-ENTRY fully implemented now? Is there anything else left to do on this task besides review and regression test?

Adaptive non-scrolling queries remain.

Note that my decision to remove Cursor.offEnd field probably was a mistake.

It is still removed in this update. Why was it a mistake, and how did you compensate/correct for it?

The problem with off-end is described in note 46, i.e. in 4GL off-end state doesn't always match "logical" off-end. And placing this quirky logic into Cursor may be a solution.

Have we lost any functionality as a result?

Neither old nor new implementations doesn't fully conform 4GL because P2J just doesn't implement 4GL logic. If we want full conformity - we should just spent some time to implement it. Overall off-end behavior is better here and there, and sometimes it may be worse. But getOffEnd() is used internally by P2J, where it is meant for "logical" off-end, which it represents now, so from the practical point of view it became better.

Please list the test case names you used for your manual testing and be sure they are checked into the testcases project.

See uast/adaptive_scrolling and uast/delete_entry folders.

Why are you calling Collections.reverse(results) at Cursor:851?

That is the point at which cache built starting from the last element reaches the first element (i.e. we have the full set of rows). I found it easier for handling to change numeration from backward to forward. It is not an operation that I can't get rid of, just personal design preference.

I noticed your additions to AdaptiveQuery deal with composite rows (i.e., arrays of ids to represent multi-table row sets). However, AdaptiveQuery is currently limited to a single table implementation -- addComponent enforces this at runtime, and conversion will not create multi-table instances of AdaptiveQuery. What was the intention behind making your changes multi-table?

I had no intention of making AdaptiveQuery multi-table. I've connect it with Cursor which is multi-table, so it turned out that way.

In your estimation, how much effort is left to fully enabling multi-table support? If we could get this right, it should be much more performant than CompoundQuery in many cases.

Note that for the most part I've been modifying scrolling part of the AdaptiveQuery, which uses cache. And when we need to add a new cache row, we turn to core adaptive processing. I haven't looked into core processing, and cannot give estimates.

• An issue with the PresortCompoundQuery implementation.
• Fixing adaptive, non-scrolling queries.
• Off-end behavior still not fully compatible with 4GL.

Please create separate Redmine issues for each of these, in this project (no assignee, target version, or start date). You can relate them to this issue, but each one should have all the relevant details one would need to understand and work on the problem.

Other than that, please make the minor changes (javadoc, method ordering) requested in note 52, then commit and distribute your update. These minor changes should have no impact on functionality, so you don't need to re-run regression testing.

That is correct except for:

• Fixing adaptive, non-scrolling queries.

If you mean improving it for multi-table use, then yes, that is a separate task. But also I need to finish up DELETE-RESULT-LIST-ENTRY for this type of queries.

Other than that, please make the minor changes (javadoc, method ordering) requested in note 52, then commit and distribute your update. These minor changes should have no impact on functionality, so you don't need to re-run regression testing.

I made several fixes for the adaptive cache along the way, so I will have to re-run regression (unless you want me to check in the existing update).

Stanislav Lomany wrote:

That is correct except for:

• Fixing adaptive, non-scrolling queries.

If you mean improving it for multi-table use, then yes, that is a separate task. But also I need to finish up DELETE-RESULT-LIST-ENTRY for this type of queries.

No, I misunderstood that you were saying there was something else that needed to be fixed for adaptive, non-scrolling queries. Please go ahead and finish the DELETE-RESULT-LIST-ENTRY implementation.

Other than that, please make the minor changes (javadoc, method ordering) requested in note 52, then commit and distribute your update. These minor changes should have no impact on functionality, so you don't need to re-run regression testing.

I made several fixes for the adaptive cache along the way, so I will have to re-run regression (unless you want me to check in the existing update).

No, don't check it in. Please make the requested code changes, open the other two issues (for PresortCompoundQuery and the remaining, non-conforming, off-end behavior), finish with the DELETE-RESULT-LIST-ENTRY implementation, then repost an update for review.

Code review 1015a:

I found nothing problematic, though it is quite a large and complicated update, so it is hard to tell. Please regression test.

Code review 1020a:

Change look good.

Passed regression testing (except three tc_job_clock tests)

Do you have another run in which the failing tests have passed (except for tc_job_002, step 40)?

I have two runs with tc_job_clock_002 and tc_job_clock_004 failed. So I'll run regression one more time.