Project

General

Profile

Bug #12106

Temp-table queries lose their index because the cached temp-table P2O lookup is built before P2O post-processing adds the indexes

Added by Octavian Adrian Gavril about 7 hours ago. Updated about 3 hours ago.

Status:
Review
Priority:
Normal
Assignee:
Octavian Adrian Gavril
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Constantin Asofiei, Dănuț Filimon
production:
No
env_name:
topics:
Incremental Conversion

History

#1 Updated by Octavian Adrian Gavril about 7 hours ago

When a conversion run converts a single file, the queries on its temp-tables can lose their index, even with USE-INDEX. The temp-table lookup cached by P2OLookup.loadTemporarySchema() for that file holds no indexes. The same problem was found in #7169-69.

The testcase is tt_useidx.p:

define temp-table tt no-undo
   field f1 as integer
   field f2 as character
   index idx-f1 is primary f1
   index idx-f2 f2.

for each tt use-index idx-f2:
   display tt.f1 tt.f2.
end.
To reproduce it on trunk 16822:
  1. Add tt_useidx.p to a project with a few other programs and run a full conversion. The query in TtUseidx.java is correct: query0.initialize(tt, ((String) null), null, "tt.f2 asc").
  2. Add one space to tt_useidx.p and run an incremental conversion. Only tt_useidx.p is converted again.
  3. The query in TtUseidx.java is now wrong: query0.initialize(tt, ((String) null), null, "tt.recid asc").

A full conversion of tt_useidx.p alone gives the same wrong query.

The lookup of tt_useidx.p is rebuilt when "P2O Post-processing (temp-table schema files)" visits it, as its .p2o AST exists for the first time. The INDEX nodes are added by schema/p2o_post.xml only after that visit. isCurrentLookupFor() checks only the artifact, the presence of the AST and the class definition, so all later phases reuse this lookup without indexes. When other files are converted in the same run, visiting them replaces the cached lookup, so tt_useidx.p is usually reloaded with its indexes.

The flaw is older than the reuse check added in 16737: the P2OLookup.java of 16736 on top of trunk 16822 gives the same wrong query. The reuse check must notice that the P2O AST changed after the lookup was built.

#2 Updated by Octavian Adrian Gavril about 7 hours ago

  • Assignee set to Octavian Adrian Gavril
  • Status changed from New to WIP

I'm looking into this one.

#3 Updated by Octavian Adrian Gavril about 3 hours ago

  • Status changed from WIP to Review
  • % Done changed from 0 to 100
  • reviewer Constantin Asofiei, Dănuț Filimon added

I've committed a fix for this in 12106a/16827.

The reason why the queries lose their index is the order in which the temp-table lookup is built and the P2O AST is completed. P2OLookup.loadTemporarySchema() keeps the last lookup it built and reuses it as long as isCurrentLookupFor() accepts it. When P2O Post-processing (temp-table schema files) visits a file, its .p2o AST exists for the first time, so the lookup is rebuilt. Only after that visit does the phase add the INDEX nodes and save the AST again. The check compares the artifact, the presence of the AST and the class definition, so it cannot see the second save, and every later phase reuses the lookup without indexes. The lookup is cached per conversion thread, so with several threads the problem appears at random, also in runs with many files.

The fix makes the reuse check notice every save of a P2O AST:
  • P2OLookup counts the P2O AST saves in an AtomicLong shared by all threads.
  • DataModelWorker.persistTarget() increments the counter right after saveTree().
  • A temp-table lookup records the counter first thing in its constructor, before it loads its AST and the ASTs of the parent classes.
  • isCurrentLookupFor() also rejects a lookup whose recorded value is no longer the current one.

Saves happen only in the P2O phases, so the lookup is still reused in all later phases. At runtime persistTarget() is not called, so the counter stays 0 and dynamic temp-tables behave as before.

The cost on a large project is what the testcases cannot show. Each save makes every thread reload its lookup during the two P2O phases. On the testcases the conversion times stay within the run-to-run noise.

Danut, Constantin, please review the changes.

Also available in: Atom PDF