Merge lp:~mmcm/akiban-server/skip-scan-loop into lp:~akiban-technologies/akiban-server/trunk

Proposed by Mike McMahon
Status: Merged
Approved by: Nathan Williams
Approved revision: 2731
Merged at revision: 2731
Proposed branch: lp:~mmcm/akiban-server/skip-scan-loop
Merge into: lp:~akiban-technologies/akiban-server/trunk
Diff against target: 327 lines (+124/-19)
4 files modified
src/main/java/com/akiban/qp/operator/Intersect_Ordered.java (+13/-6)
src/main/java/com/akiban/qp/operator/Union_Ordered.java (+13/-6)
src/main/java/com/akiban/qp/persistitadapter/indexcursor/IndexCursorUnidirectional.java (+10/-7)
src/test/java/com/akiban/server/test/it/qp/Intersect_OrderedSkipScanIT.java (+88/-0)
To merge this branch: bzr merge lp:~mmcm/akiban-server/skip-scan-loop
Reviewer Review Type Date Requested Status
Akiban Technologies Pending
Review via email: mp+179020@code.launchpad.net

Description of the change

Fix skip scan inside a loop.

The general problem is that open() must re-initialize the state of the cursor from the bindings and it is perfectly valid for this to happen more than once in the cursor's lifetime.

The specific problems were:
(1) The Intersect and Union operators only copied the fixed fields for skip once. Needs to be once per iteration.
(2) A unidirectional cursor initialized itself in the constructor and then clobbered this state on jump. The first part needs to happen in open so that it is reset for a new set of jumps.

Includes a test by running two of the other tests next to one another via an outer loop.

To post a comment you must log in.
Revision history for this message
Nathan Williams (nwilliams) wrote :

Makes sense.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/main/java/com/akiban/qp/operator/Intersect_Ordered.java'
2--- src/main/java/com/akiban/qp/operator/Intersect_Ordered.java 2013-07-15 22:01:49 +0000
3+++ src/main/java/com/akiban/qp/operator/Intersect_Ordered.java 2013-08-07 17:10:40 +0000
4@@ -275,6 +275,7 @@
5 nextLeftRow();
6 nextRightRow();
7 closed = leftRow.isEmpty() && rightRow.isEmpty();
8+ leftSkipRowFixed = rightSkipRowFixed = false; // Fixed fields are per iteration.
9 } finally {
10 TAP_OPEN.out();
11 }
12@@ -567,10 +568,11 @@
13
14 private ValuesHolderRow leftSkipRow()
15 {
16- if (leftSkipRow == null) {
17+ if (!leftSkipRowFixed) {
18+ boolean usingPValues = Intersect_Ordered.this.usePValues;
19+ if (leftSkipRow == null)
20+ leftSkipRow = new ValuesHolderRow(leftRowType, usingPValues);
21 assert leftRow.isHolding();
22- boolean usingPValues = Intersect_Ordered.this.usePValues;
23- leftSkipRow = new ValuesHolderRow(leftRowType, usingPValues);
24 int f = 0;
25 while (f < leftFixedFields) {
26 if (usingPValues)
27@@ -589,16 +591,18 @@
28 else
29 leftSkipRow.holderAt(f++).putNull();
30 }
31+ leftSkipRowFixed = true;
32 }
33 return leftSkipRow;
34 }
35
36 private ValuesHolderRow rightSkipRow()
37 {
38- if (rightSkipRow == null) {
39+ if (!rightSkipRowFixed) {
40+ boolean usingPValues = Intersect_Ordered.this.usePValues;
41+ if (rightSkipRow == null)
42+ rightSkipRow = new ValuesHolderRow(rightRowType, usingPValues);
43 assert rightRow.isHolding();
44- boolean usingPValues = Intersect_Ordered.this.usePValues;
45- rightSkipRow = new ValuesHolderRow(rightRowType, usingPValues);
46 int f = 0;
47 while (f < rightFixedFields) {
48 if (usingPValues)
49@@ -615,6 +619,7 @@
50 else
51 rightSkipRow.holderAt(f++).putNull();
52 }
53+ rightSkipRowFixed = true;
54 }
55 return rightSkipRow;
56 }
57@@ -632,5 +637,7 @@
58 private final ShareHolder<Row> rightRow = new ShareHolder<>();
59 private ValuesHolderRow leftSkipRow;
60 private ValuesHolderRow rightSkipRow;
61+ private boolean leftSkipRowFixed;
62+ private boolean rightSkipRowFixed;
63 }
64 }
65
66=== modified file 'src/main/java/com/akiban/qp/operator/Union_Ordered.java'
67--- src/main/java/com/akiban/qp/operator/Union_Ordered.java 2013-07-18 14:20:06 +0000
68+++ src/main/java/com/akiban/qp/operator/Union_Ordered.java 2013-08-07 17:10:40 +0000
69@@ -212,6 +212,7 @@
70 if (leftRow.isEmpty() && rightRow.isEmpty()) {
71 close();
72 }
73+ leftSkipRowFixed = rightSkipRowFixed = false; // Fixed fields are per iteration.
74 } finally {
75 TAP_OPEN.out();
76 }
77@@ -455,10 +456,11 @@
78
79 private ValuesHolderRow leftSkipRow()
80 {
81- boolean usingPValues = Union_Ordered.this.usePValues;
82- if (leftSkipRow == null) {
83+ if (!leftSkipRowFixed) {
84+ boolean usingPValues = Union_Ordered.this.usePValues;
85+ if (leftSkipRow == null)
86+ leftSkipRow = new ValuesHolderRow(rowType, usingPValues);
87 assert leftRow.isHolding();
88- leftSkipRow = new ValuesHolderRow(rowType, usingPValues);
89 int f = 0;
90 while (f < fixedFields) {
91 if (usingPValues)
92@@ -473,16 +475,18 @@
93 else
94 leftSkipRow.holderAt(f++).putNull();
95 }
96+ leftSkipRowFixed = true;
97 }
98 return leftSkipRow;
99 }
100
101 private ValuesHolderRow rightSkipRow()
102 {
103- boolean usingPValues = Union_Ordered.this.usePValues;
104- if (rightSkipRow == null) {
105+ if (!rightSkipRowFixed) {
106+ boolean usingPValues = Union_Ordered.this.usePValues;
107+ if (rightSkipRow == null)
108+ rightSkipRow = new ValuesHolderRow(rowType, usingPValues);
109 assert rightRow.isHolding();
110- rightSkipRow = new ValuesHolderRow(rowType, usingPValues);
111 int f = 0;
112 while (f < fixedFields) {
113 if (usingPValues)
114@@ -497,6 +501,7 @@
115 else
116 rightSkipRow.holderAt(f++).putNull();
117 }
118+ rightSkipRowFixed = true;
119 }
120 return rightSkipRow;
121 }
122@@ -514,5 +519,7 @@
123 private final ShareHolder<Row> rightRow = new ShareHolder<>();
124 private ValuesHolderRow leftSkipRow;
125 private ValuesHolderRow rightSkipRow;
126+ private boolean leftSkipRowFixed;
127+ private boolean rightSkipRowFixed;
128 }
129 }
130
131=== modified file 'src/main/java/com/akiban/qp/persistitadapter/indexcursor/IndexCursorUnidirectional.java'
132--- src/main/java/com/akiban/qp/persistitadapter/indexcursor/IndexCursorUnidirectional.java 2013-07-16 18:27:15 +0000
133+++ src/main/java/com/akiban/qp/persistitadapter/indexcursor/IndexCursorUnidirectional.java 2013-08-07 17:10:40 +0000
134@@ -49,6 +49,9 @@
135 public void open()
136 {
137 super.open();
138+ keyRange = initialKeyRange;
139+ if (keyRange != null)
140+ initializeCursor();
141 evaluateBoundaries(context, sortKeyAdapter);
142 initializeForOpen();
143 }
144@@ -131,7 +134,7 @@
145 direction == FORWARD
146 ? keyRange.resetLo(new IndexBound(row, columnSelector))
147 : keyRange.resetHi(new IndexBound(row, columnSelector));
148- initializeCursor(keyRange, ordering);
149+ initializeCursor();
150 reevaluateBoundaries(context, sortKeyAdapter);
151 initializeForOpen();
152 }
153@@ -159,11 +162,12 @@
154 SortKeyAdapter<S, ?> sortKeyAdapter)
155 {
156 super(context, iterationHelper);
157+ this.initialKeyRange = keyRange;
158+ this.ordering = ordering;
159 // end state never changes. start state can change on a jump, so it is set in initializeCursor.
160 this.endBoundColumns = keyRange.boundColumns();
161 this.endKey = endBoundColumns == 0 ? null : adapter.takeIndexRow(keyRange.indexRowType());
162 this.sortKeyAdapter = sortKeyAdapter;
163- initializeCursor(keyRange, ordering);
164 }
165
166 protected void evaluateBoundaries(QueryContext context, SortKeyAdapter<S, ?> keyAdapter)
167@@ -382,10 +386,8 @@
168
169 // For use by this class
170
171- private void initializeCursor(IndexKeyRange keyRange, API.Ordering ordering)
172+ private void initializeCursor()
173 {
174- this.keyRange = keyRange;
175- this.ordering = ordering;
176 this.lo = keyRange.lo();
177 this.hi = keyRange.hi();
178 if (ordering.allAscending()) {
179@@ -486,7 +488,7 @@
180 SortKeyAdapter<S, ?> sortKeyAdapter)
181 {
182 super(context, iterationHelper);
183- this.keyRange = null;
184+ this.initialKeyRange = null;
185 this.ordering = ordering;
186 if (ordering.allAscending()) {
187 this.startBoundary = Key.BEFORE;
188@@ -513,8 +515,9 @@
189
190 // Object state
191
192+ private final IndexKeyRange initialKeyRange;
193+ private final API.Ordering ordering;
194 private IndexKeyRange keyRange;
195- private API.Ordering ordering;
196 protected int direction; // +1 = ascending, -1 = descending
197 protected Key.Direction keyComparison;
198 protected Key.Direction initialKeyComparison;
199
200=== modified file 'src/test/java/com/akiban/server/test/it/qp/Intersect_OrderedSkipScanIT.java'
201--- src/test/java/com/akiban/server/test/it/qp/Intersect_OrderedSkipScanIT.java 2013-07-08 20:48:05 +0000
202+++ src/test/java/com/akiban/server/test/it/qp/Intersect_OrderedSkipScanIT.java 2013-08-07 17:10:40 +0000
203@@ -17,22 +17,37 @@
204
205 package com.akiban.server.test.it.qp;
206
207+import com.akiban.qp.expression.ExpressionRow;
208 import com.akiban.qp.expression.IndexBound;
209 import com.akiban.qp.expression.IndexKeyRange;
210+import com.akiban.qp.expression.RowBasedUnboundExpressions;
211 import com.akiban.qp.operator.API;
212 import com.akiban.qp.operator.ExpressionGenerator;
213 import com.akiban.qp.operator.Operator;
214+import com.akiban.qp.row.BindableRow;
215+import com.akiban.qp.row.Row;
216 import com.akiban.qp.row.RowBase;
217 import com.akiban.qp.rowtype.IndexRowType;
218 import com.akiban.qp.rowtype.RowType;
219 import com.akiban.qp.rowtype.Schema;
220 import com.akiban.server.api.dml.SetColumnSelector;
221+import com.akiban.server.api.dml.SetColumnSelector;
222 import com.akiban.server.api.dml.scan.NewRow;
223+import com.akiban.server.types3.TInstance;
224+import com.akiban.server.types3.mcompat.mtypes.MNumeric;
225+import com.akiban.server.types3.pvalue.PValue;
226+import com.akiban.server.types3.texpressions.TPreparedExpression;
227+import com.akiban.server.types3.texpressions.TPreparedLiteral;
228+
229 import org.junit.Test;
230
231+import java.util.ArrayList;
232+import java.util.Arrays;
233 import java.util.EnumSet;
234+import java.util.List;
235
236 import static com.akiban.qp.operator.API.*;
237+import static com.akiban.server.test.ExpressionGenerators.boundField;
238 import static com.akiban.server.test.ExpressionGenerators.field;
239
240 public class Intersect_OrderedSkipScanIT extends OperatorITBase
241@@ -388,6 +403,28 @@
242 compareRows(expectedY, cursor(intersectPxPy(13, false, false, true), queryContext, queryBindings));
243 }
244
245+ @Test
246+ public void test4x5x()
247+ {
248+ int[] keys = { 44, 55 };
249+ RowBase[] expectedX = new RowBase[]{
250+ row(parentXIndexRowType, 44L, 44L, 4001L),
251+ row(parentXIndexRowType, 44L, 44L, 4002L),
252+ row(parentXIndexRowType, 55L, 55L, 5001L),
253+ row(parentXIndexRowType, 55L, 55L, 5002L),
254+ };
255+ compareRows(expectedX, cursor(intersectPxPyMap(keys, true, true, false), queryContext, queryBindings));
256+ compareRows(expectedX, cursor(intersectPxPyMap(keys, true, true, true), queryContext, queryBindings));
257+ RowBase[] expectedY = new RowBase[]{
258+ row(parentYIndexRowType, 44L, 4001L),
259+ row(parentYIndexRowType, 44L, 4002L),
260+ row(parentYIndexRowType, 55L, 5001L),
261+ row(parentYIndexRowType, 55L, 5002L),
262+ };
263+ compareRows(expectedY, cursor(intersectPxPyMap(keys, false, true, false), queryContext, queryBindings));
264+ compareRows(expectedY, cursor(intersectPxPyMap(keys, false, true, true), queryContext, queryBindings));
265+ }
266+
267 private Operator intersectPxPy(int key, boolean leftOutput, boolean ascending, boolean skipScan)
268 {
269 Operator plan =
270@@ -443,6 +480,57 @@
271 return plan;
272 }
273
274+ private Operator intersectPxPyMap(int[] keys, boolean leftOutput, boolean ascending, boolean skipScan)
275+ {
276+ TInstance intType = MNumeric.INT.instance(false);
277+ RowType xyValueRowType = schema.newValuesType(intType);
278+ List<BindableRow> keyRows = new ArrayList<>(keys.length);
279+ for (int key : keys) {
280+ List<TPreparedExpression> pExpressions =
281+ Arrays.<TPreparedExpression>asList(new TPreparedLiteral(intType, new PValue(intType, key)));
282+ Row row = new ExpressionRow(xyValueRowType, queryContext, queryBindings, null, pExpressions);
283+ keyRows.add(BindableRow.of(row, true));
284+ }
285+ List<ExpressionGenerator> expressions = Arrays.asList(boundField(xyValueRowType, 0, 0));
286+ IndexBound xBound =
287+ new IndexBound(
288+ new RowBasedUnboundExpressions(parentXIndexRowType, expressions),
289+ new SetColumnSelector(0));
290+ IndexKeyRange xRange = IndexKeyRange.bounded(parentXIndexRowType, xBound, true, xBound, true);
291+ IndexBound yBound =
292+ new IndexBound(
293+ new RowBasedUnboundExpressions(parentYIndexRowType, expressions),
294+ new SetColumnSelector(0));
295+ IndexKeyRange yRange = IndexKeyRange.bounded(parentYIndexRowType, yBound, true, yBound, true);
296+ Operator plan =
297+ map_NestedLoops(
298+ valuesScan_Default(keyRows, xyValueRowType),
299+ intersect_Ordered(
300+ indexScan_Default(
301+ parentXIndexRowType,
302+ xRange,
303+ ordering(field(parentXIndexRowType, 1), ascending)),
304+ indexScan_Default(
305+ parentYIndexRowType,
306+ yRange,
307+ ordering(field(parentYIndexRowType, 1), ascending)),
308+ parentXIndexRowType,
309+ parentYIndexRowType,
310+ 1,
311+ 1,
312+ ascending(ascending),
313+ JoinType.INNER_JOIN,
314+ EnumSet.of(skipScan
315+ ? IntersectOption.SKIP_SCAN
316+ : IntersectOption.SEQUENTIAL_SCAN,
317+ leftOutput
318+ ? IntersectOption.OUTPUT_LEFT
319+ : IntersectOption.OUTPUT_RIGHT),
320+ null),
321+ 0, pipelineMap(), 1);
322+ return plan;
323+ }
324+
325 private IndexKeyRange parentXEq(long x)
326 {
327 IndexBound xBound = new IndexBound(row(parentXIndexRowType, x), new SetColumnSelector(0));

Subscribers

People subscribed via source and target branches