comments from review of the patch:
> - removed the call to procedure WAIT_FOR_POST_COMMIT as it didn't appear to
> be used anywhere.
sounds good, it is not called so remove all reference to it.
> - Was I misinterpreting the use of WAIT_FOR_POST_COMMIT? What would be the pur
pose of it?
It is not called, leftover cruft.
comments on TODO's:
+ // TODO: check on what the following comment from the original test
+ // means. I don't think this test is currently checking whether the
+ // optimizer is picking the index, might be worthwhile adding it.
At this point not sure history of this, I would just remove comment.
+ // as subset of columns, with qualifier not in list
+ //TODO: this comment is not matching the expected results. OK?
+ // should be 5,3,1 and 50,30,10
+ doQuery(st, "select e, c, a from foo where foo.b = 20",
comment looks bad, just fix the comment.
+ // TODO: how does this check that rows are reclaimed or not? How?
looks like all work was moved to the subroutine, so probably should move
comments to the subroutine from the caller. At least the part of the comments
that make sense.
The code just tries to exercise the post commit path, but agree that it does
not specifically verify the reclaim. This is hard to do from a user level
test, in a way that is reproducible across different hardware, jvms. The
values are just hand picked to exercise different paths through the code.
One could try to use space table to verify sizes of the tables, but this
has caused non-reproducible results in cases where timing of background threads
// TODO: change this call into a call to SYSCS_DIAG.SPACE_TABLE
I agree, it should use the current supported interface.
+ // TODO: original test had 1 pages visited.
+ // But now that it's converted, we get 0 pages, which also
+ // seems reasonable to me. Or not?
This seems like it might be a problem. Can you print out the full query
plan on failure of the assert. It is hard to know what is going on from
just this one piece of info.
The main point of this test is that is should use the index whether there
are zero or 1 rows. We choose to force this because if we use index we get
row level locking, but if we use table scan we are going to get table level
locking - so better concurrency in the edge case.
+ // TODO: in which way does this check qualifiers?
qualifiers are just things like a = 1 and b = 20. When test was written the
code in language and store to evaluate these were changing. The changes
especially had some new logic to handle "and" and "or". So the test just
has some of these cases and tests them by making sure the right results