Akiban Server

Merge lp:~oontvoo/akiban-server/assertion-error-bug1084743 into lp:~akiban-technologies/akiban-server/trunk

  1. assertion-error-bug1084743
  2. Merge into trunk
Proposed by Vy Nguyen
Status: Merged
Approved by: Yuval Shavit
Approved revision: 2616
Merged at revision: 2616
Proposed branch: lp:~oontvoo/akiban-server/assertion-error-bug1084743
Merge into: lp:~akiban-technologies/akiban-server/trunk
Diff against target: 203 lines (+39/-9)
9 files modified
src/main/java/com/akiban/server/types3/common/funcs/BoolLogic.java (+12/-1)
src/main/java/com/akiban/server/types3/common/funcs/Coalesce.java (+2/-1)
src/main/java/com/akiban/server/types3/common/funcs/DescribeExpression.java (+2/-1)
src/main/java/com/akiban/server/types3/common/funcs/Elt.java (+2/-1)
src/main/java/com/akiban/server/types3/mcompat/mfuncs/MArithmetic.java (+1/-1)
src/main/java/com/akiban/server/types3/mcompat/mfuncs/MIfElse.java (+2/-1)
src/main/java/com/akiban/server/types3/texpressions/TPreptimeErrorScalar.java (+1/-1)
src/main/java/com/akiban/server/types3/texpressions/TScalarBase.java (+2/-2)
src/test/resources/com/akiban/sql/pg/yaml/functional/test-bool-logic.yaml (+15/-0)
To merge this branch: bzr merge lp:~oontvoo/akiban-server/assertion-error-bug1084743
Reviewer Review Type Date Requested Status
Yuval Shavit (community) Approve
Vy Nguyen (community) Needs Resubmitting
Review via email: mp+157504@code.launchpad.net

Description of the change

fix assertion error in boolean expression (bug 1084743)

The assertion error happened because TScalarBase.evaluateConstant assumes that if the top expression is said to be constant, then all of its operands must be constant, too, which is clearly not the case with BOOLEAN expressions.

The fix is to have BoolLogic.constantness() cache the return value if it decides that the expression is going to be CONST, so that doEvaluate() does not need to evaluate the inputs again (which would cause the assertion error to happen)

To post a comment you must log in.
lp:~oontvoo/akiban-server/assertion-error-bug1084743 updated
2615. By Vy Nguyen

more tests

Revision history for this message
Yuval Shavit (yshavit) wrote :

Looks good. Can you just remove the unused import in TScalar.java that you introduced (line 8 of the diff)?

review: Needs Fixing
lp:~oontvoo/akiban-server/assertion-error-bug1084743 updated
2616. By Vy Nguyen

remove import Constantness in TScalar.java

Revision history for this message
Vy Nguyen (oontvoo) wrote :

Remove unused imports

review: Needs Resubmitting
Revision history for this message
Yuval Shavit (yshavit) wrote :

Thanks, looks good now.

review: Approve

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/server/types3/common/funcs/BoolLogic.java'
2--- src/main/java/com/akiban/server/types3/common/funcs/BoolLogic.java 2013-03-22 20:05:57 +0000
3+++ src/main/java/com/akiban/server/types3/common/funcs/BoolLogic.java 2013-04-08 15:41:20 +0000
4@@ -26,6 +26,7 @@
5 import com.akiban.server.types3.TInstance;
6 import com.akiban.server.types3.TScalar;
7 import com.akiban.server.types3.TOverloadResult;
8+import com.akiban.server.types3.TPreptimeContext;
9 import com.akiban.server.types3.TPreptimeValue;
10 import com.akiban.server.types3.aksql.aktypes.AkBool;
11 import com.akiban.server.types3.pvalue.PValueSource;
12@@ -41,6 +42,7 @@
13 public class BoolLogic extends TScalarBase
14 {
15 public static final TScalar BINARIES[] = new TScalar[Op.values().length];
16+ private static final int OUT_VAL = 0;
17 static
18 {
19 Op op[] = Op.values();
20@@ -118,13 +120,16 @@
21 }
22
23 @Override
24- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
25+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
26 // The expression is const iff either argument is a const whose value is equal to op.contaminant.
27 // The first argument can never make the expression non-const (though it can make it const), and the second
28 // argument can never leave the constness unknown.
29 PValueSource preptimeValue = constSource(values, inputIndex);
30 if ((preptimeValue != null) && Objects.equal(op.contaminant, getBoolean(preptimeValue)))
31+ {
32+ context.set(OUT_VAL, op.contaminant);
33 return Constantness.CONST;
34+ }
35 return (inputIndex == 0) ? Constantness.UNKNOWN : Constantness.NOT_CONST;
36 }
37
38@@ -136,6 +141,12 @@
39 @Override
40 protected void doEvaluate(TExecutionContext context, LazyList<? extends PValueSource> inputs, PValueTarget output)
41 {
42+ Object outVal = context.preptimeObjectAt(OUT_VAL);
43+ if (outVal != null)
44+ {
45+ output.putBool((Boolean)outVal);
46+ return;
47+ }
48 Boolean firstArg = getBoolean(inputs, 0);
49 final Boolean result;
50 if (Objects.equal(op.contaminant, firstArg)) {
51
52=== modified file 'src/main/java/com/akiban/server/types3/common/funcs/Coalesce.java'
53--- src/main/java/com/akiban/server/types3/common/funcs/Coalesce.java 2013-03-22 20:05:57 +0000
54+++ src/main/java/com/akiban/server/types3/common/funcs/Coalesce.java 2013-04-08 15:41:20 +0000
55@@ -21,6 +21,7 @@
56 import com.akiban.server.types3.TExecutionContext;
57 import com.akiban.server.types3.TScalar;
58 import com.akiban.server.types3.TOverloadResult;
59+import com.akiban.server.types3.TPreptimeContext;
60 import com.akiban.server.types3.TPreptimeValue;
61 import com.akiban.server.types3.pvalue.PValueSource;
62 import com.akiban.server.types3.pvalue.PValueTarget;
63@@ -67,7 +68,7 @@
64 }
65
66 @Override
67- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
68+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
69 PValueSource preptimeValue = constSource(values, inputIndex);
70 if (preptimeValue == null)
71 return Constantness.NOT_CONST;
72
73=== modified file 'src/main/java/com/akiban/server/types3/common/funcs/DescribeExpression.java'
74--- src/main/java/com/akiban/server/types3/common/funcs/DescribeExpression.java 2013-03-22 20:05:57 +0000
75+++ src/main/java/com/akiban/server/types3/common/funcs/DescribeExpression.java 2013-04-08 15:41:20 +0000
76@@ -20,6 +20,7 @@
77 import com.akiban.server.types3.LazyList;
78 import com.akiban.server.types3.TExecutionContext;
79 import com.akiban.server.types3.TOverloadResult;
80+import com.akiban.server.types3.TPreptimeContext;
81 import com.akiban.server.types3.TPreptimeValue;
82 import com.akiban.server.types3.TScalar;
83 import com.akiban.server.types3.mcompat.mtypes.MString;
84@@ -60,7 +61,7 @@
85 }
86
87 @Override
88- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
89+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
90 return Constantness.CONST;
91 }
92
93
94=== modified file 'src/main/java/com/akiban/server/types3/common/funcs/Elt.java'
95--- src/main/java/com/akiban/server/types3/common/funcs/Elt.java 2013-03-22 20:05:57 +0000
96+++ src/main/java/com/akiban/server/types3/common/funcs/Elt.java 2013-04-08 15:41:20 +0000
97@@ -21,6 +21,7 @@
98 import com.akiban.server.types3.TClass;
99 import com.akiban.server.types3.TExecutionContext;
100 import com.akiban.server.types3.TOverloadResult;
101+import com.akiban.server.types3.TPreptimeContext;
102 import com.akiban.server.types3.TPreptimeValue;
103 import com.akiban.server.types3.pvalue.PValueSource;
104 import com.akiban.server.types3.pvalue.PValueTarget;
105@@ -67,7 +68,7 @@
106 }
107
108 @Override
109- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
110+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
111 assert inputIndex == 0 : inputIndex + " for " + values; // 0 should be enough to fully answer the question
112 PValueSource indexVal = constSource(values, 0);
113 if (indexVal == null)
114
115=== modified file 'src/main/java/com/akiban/server/types3/mcompat/mfuncs/MArithmetic.java'
116--- src/main/java/com/akiban/server/types3/mcompat/mfuncs/MArithmetic.java 2013-03-22 20:05:57 +0000
117+++ src/main/java/com/akiban/server/types3/mcompat/mfuncs/MArithmetic.java 2013-04-08 15:41:20 +0000
118@@ -799,7 +799,7 @@
119 }
120
121 @Override
122- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
123+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
124 return Constantness.CONST;
125 }
126
127
128=== modified file 'src/main/java/com/akiban/server/types3/mcompat/mfuncs/MIfElse.java'
129--- src/main/java/com/akiban/server/types3/mcompat/mfuncs/MIfElse.java 2013-03-22 20:05:57 +0000
130+++ src/main/java/com/akiban/server/types3/mcompat/mfuncs/MIfElse.java 2013-04-08 15:41:20 +0000
131@@ -21,6 +21,7 @@
132 import com.akiban.server.types3.TClass;
133 import com.akiban.server.types3.TExecutionContext;
134 import com.akiban.server.types3.TOverloadResult;
135+import com.akiban.server.types3.TPreptimeContext;
136 import com.akiban.server.types3.TPreptimeValue;
137 import com.akiban.server.types3.TScalar;
138 import com.akiban.server.types3.aksql.aktypes.AkBool;
139@@ -125,7 +126,7 @@
140 }
141
142 @Override
143- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
144+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
145 assert inputIndex == 0 : inputIndex; // should be fully resolved after the first call
146 PValueSource condition = values.get(0).value();
147 if (condition == null)
148
149=== modified file 'src/main/java/com/akiban/server/types3/texpressions/TPreptimeErrorScalar.java'
150--- src/main/java/com/akiban/server/types3/texpressions/TPreptimeErrorScalar.java 2013-03-22 20:05:57 +0000
151+++ src/main/java/com/akiban/server/types3/texpressions/TPreptimeErrorScalar.java 2013-04-08 15:41:20 +0000
152@@ -54,7 +54,7 @@
153 }
154
155 @Override
156- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
157+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
158 return Constantness.NOT_CONST;
159 }
160
161
162=== modified file 'src/main/java/com/akiban/server/types3/texpressions/TScalarBase.java'
163--- src/main/java/com/akiban/server/types3/texpressions/TScalarBase.java 2013-03-22 20:05:57 +0000
164+++ src/main/java/com/akiban/server/types3/texpressions/TScalarBase.java 2013-04-08 15:41:20 +0000
165@@ -128,7 +128,7 @@
166 }
167 for (int i = 0; i < inputs.size(); ++i) {
168 if (constnessMatters(i)) {
169- Constantness constness = constness(i, inputs);
170+ Constantness constness = constness(context, i, inputs);
171 if (constness == Constantness.NOT_CONST)
172 return null;
173 if (constness == Constantness.CONST)
174@@ -200,7 +200,7 @@
175 * @param values
176 * @return what is known about this expression's constness due to this input
177 */
178- protected Constantness constness(int inputIndex, LazyList<? extends TPreptimeValue> values) {
179+ protected Constantness constness(TPreptimeContext context, int inputIndex, LazyList<? extends TPreptimeValue> values) {
180 return constSource(values, inputIndex) == null ? Constantness.NOT_CONST : Constantness.UNKNOWN;
181 }
182
183
184=== added file 'src/test/resources/com/akiban/sql/pg/yaml/functional/test-bool-logic.yaml'
185--- src/test/resources/com/akiban/sql/pg/yaml/functional/test-bool-logic.yaml 1970-01-01 00:00:00 +0000
186+++ src/test/resources/com/akiban/sql/pg/yaml/functional/test-bool-logic.yaml 2013-04-08 15:41:20 +0000
187@@ -0,0 +1,15 @@
188+# bug 1084743
189+---
190+- CreateTable: t(id int);
191+---
192+- Statement: INSERT INTO t VALUES (1), (2), (3);
193+---
194+- Statement: SELECT id > 1 AND 0 FROM t;
195+- output: [['false'], ['false'], ['false']]
196+---
197+- Statement: SELECT 1 < id AND 0 FROM t;
198+- output: [['false'], ['false'], ['false']]
199+---
200+- Statement: SELECT 1 > id OR 1 FROM t;
201+- output: [['true'], ['true'], ['true']]
202+...
203\ No newline at end of file

Subscribers

People subscribed via source and target branches

to all changes: