FWD » Core Development » Database

Is the idea to just remove the ORDER BY at the SQL level (as opposed to the FQL level)?

If you are intending to remove it at the FQL level, please note that we do a reverse lookup on the ORDER BY clause early in RandomAccessQuery processing, and there is some RecordNursery processing that is dependent upon that. There may be other runtime dependencies on knowing the legacy index in use as well. Please keep these in mind, such that we don't introduce regressions with this optimization.

Eric Faulhaber wrote:

If you are intending to remove it at the FQL level, please note that we do a reverse lookup on the ORDER BY clause early in RandomAccessQuery processing, and there is some RecordNursery processing that is dependent upon that. There may be other runtime dependencies on knowing the legacy index in use as well. Please keep these in mind, such that we don't introduce regressions with this optimization.

I was thinking of SQL. Radu, please advice; are your changes in #7066 at SQL level?

Alexandru Lungu wrote:

Eric Faulhaber wrote:

If you are intending to remove it at the FQL level, please note that we do a reverse lookup on the ORDER BY clause early in RandomAccessQuery processing, and there is some RecordNursery processing that is dependent upon that. There may be other runtime dependencies on knowing the legacy index in use as well. Please keep these in mind, such that we don't introduce regressions with this optimization.

I was thinking of SQL. Radu, please advice; are your changes in #7066 at SQL level?

My changes are at FQL level.

I will write a more detailed post in #7066, but, long story short, the ORDER BY is removed from the FQL only when dealing with a multi-table non-invalidated (PreselectMode) AdaptiveQuery over temporary tables. In the other cases, it is exactly the same. I thought that by removing the ORDER BY from the FQL, then that component would not require to be parsed in order to convert it to SQL (so a small performance boost would be obtained).

On its way to being converted to SQL, is the FQL (of the previously described case) required in other parts of the program? If so, I can adapt the solution.

I've added in PreselectQuery a function that checks if the query will retrieve a unique finding. If so, then no ORDER BY clause should be required.
The changes are on 7026c, rev. 14527.

After navigating a large customer application for 10-15 minutes, the results showed ~40 queries that successfully decided to not include the ORDER BY clause in their FQL.
Although the changes are at FQL level, as far as I've tested, things don't interfere with RandomAccessQuery or RecordNursery, but I will wait for a review.

Radu, let the intended changes for #7194 reside in 7026c. I am thinking of merging 7026c independently from 7066a, as 7066a needs more testing. Please let me know if you have changes in 7066a that should be moved to 7026c first. Afterwards, I will start reviewing.

My initial thought when I created #7194 was to optimize UNIQUE FIND/CAN-FIND for persistent databases. For temp databases we already have direct access which doesn't take the order-by into account. My concern was to avoid sending an ORDER BY to PostgreSQL, for example, even though we know that the UNIQUE FIND/CAN-FIND is going to find at most 1 record.

However, it is interesting that you tackled the PreselectQuery this way and that you actually find unique where clauses. At a certain extent I think we can even resolve unique PreselectQuery to direct-access (for _temp), but I should put some thought on this.

Review of 7026c, rev. 14527
This change conditionally removes the ORDER BY from PreselectQuery and AdaptiveQuery if not needed (when each component is marked as isUniqueFind).

• The changes are clean enough.
• My regression testing passes.
• Even if this doesn't regress, I am not 100% certain that removing the ORDER BY will not break the AdaptiveQuery.
  • What if the uniquely found record is changed? The query should invalidate (or not?) and "refind" the unique record again in dynamic mode. Radu, please make an example with this - I hope at this point th invalidation will trigger so that ORDER BY won't matter anyway.
  • I don't expect the LAZY mode to interact with your ORDER BY in any way. Please double-check this.
  • Is the adaptive_scrolling/non-scrolling test-suite OK with your changes? For both persistent (concurrent environment) and _temp (sometimes using LAZY)?
• Profiling shows the following:

This is the time to actually append the ORDER BY clause into the FQL; it is not related to SQL execution time. With the changes, I expect to see 20% less ORDER BY generations (faster with ~17ms) and fortunately faster SQL parsing, planning, executions, caching, SQL conversion, etc. I will run my profiling now.

Alexandru Lungu wrote:

• What if the uniquely found record is changed? The query should invalidate (or not?) and "refind" the unique record again in dynamic mode. Radu, please make an example with this - I hope at this point the invalidation will trigger so that ORDER BY won't matter anyway.

From the tests I've done, the program works as before, regarding the invalidation process. This is quite a tricky test though, having a query that retrieves at most one record, which should/can/might be "re-found" later. It's not like after a bunch of next s the record can be found again. By this I mean that the test is very specific and from my gatherings it works fine.

• Is the adaptive_scrolling/non-scrolling test-suite OK with your changes? For both persistent (concurrent environment) and _temp (sometimes using LAZY)?

Yes, the results were as expected for all the tests (both persistent and temporary). However, very few unique findings were encountered in the test-suite.

In #7194-5 I said that this feature is available only in a multi-temporary-table environment. This is not true. I think my ideas got mixed up when writing the post for #7066, sorry. At the moment of speaking, this is available for all PreselectQueries and AdaptiveQueries, regardless of persistent/temporary and single/multi-table environments.

• I don't expect the LAZY mode to interact with your ORDER BY in any way. Please double-check this.

Taking into account the previous point, LAZY can appear when dealing with a single-table query (which has a unique finding) over a temporary table. I don't think that this should be a problem, especially considering the fact that some tests included this exact case. If this happens to be wrong, I will adapt the solution accordingly.

I have modified RandomAccessQuery so that an ORDER BY clause will not be added to the FQL if a unique finding is encountered. This takes place when creating a bundle (in FQLHelper.createBundle), only in FIRST/LAST cases and only for persistent databases.
The changes are on 7026d, rev. 14562.
I've tested the changes with some examples and a large customer application. In the last case, ~28.74% of the FIRST/LAST RandomAccessQueries over persistent databases lead to a unique finding, thus orderBy became null.

The changes are tested and OK to me.

However, I can't isolate a performance increase for this change (it is either the same as before or slightly worse (+0.1%) on some profiling runs.
Can you profile some dummy testcases with/without the change? Maybe that order-by was helping the planning as a hint .... or not. (only RAQ changes).

Alexandru Lungu wrote:

Can you profile some dummy testcases with/without the change?

• If 10,000 queries or less were executed, one could not distinguish a difference between them.
• For 100,000 queries, a -1.9% improvement could be seen (10.4 seconds without the changes vs 10.2 seconds with the changes).

• FQLHelperCache was disabled for this round of testing which leads me to believe that in real scenarios, this improvement percentage is even smaller.
• All queries that were executed lead to a unique finding.
• That -1.9% improvement refers only to the total time of executing the queries, not the program itself.

Radu, please post your attempt on #7194-16 here (the test-case). AFAIK, you couldn't generate two records with the same value violating a unique constraint for the period of the transaction.

Alexandru Lungu wrote:

Radu, please post your attempt on #7194-16 here (the test-case). AFAIK, you couldn't generate two records with the same value violating a unique constraint for the period of the transaction.

Right. I tried to run the following test in 4GL, but I received an error saying b2 already exists with f1 1.

define temp-table tt field f1 as int field f2 as int index idx1 as unique primary f1.
define buffer b1 for tt.
define buffer b2 for tt.

do transaction:
    create b1. b1.f1 = 1. b1.f2 = 1.
end.

do transaction:
    create b2. b2.f1 = 1. b2.f2 = 2.

    define buffer b3 for tt.
    define buffer b4 for tt.

    find first b3 where b3.f1 = 1.
    find last b4 where b4.f1 = 1.

    message b3.f1 b3.f2 b4.f1 b4.f2.
    b2.f1 = 2.
end.

If there are any suggestions, I will be thankful.

Alexandru Lungu wrote:

I will move this to review/100% just because everything is done already - this last step are only some correctness checks.

What exactly does this mean? AFIACT, the changes have been integrated into FWD-H2, correct? So, what is the actual status of this task?