[
https://issues.apache.org/jira/browse/DRILL-6524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16788807#comment-16788807
]
Volodymyr Vysotskyi edited comment on DRILL-6524 at 3/12/19 4:38 PM:
---------------------------------------------------------------------
The problem here is connected with incorrect scalar replacement. It still
happens even for the case when a reference to the object outside of the
conditional block is changed inside the conditional block. It causes wrong
results for the case when code in this conditional block will not be executed.
The code below demonstrates this issue:
Original class:
{code:java}
package org.apache.drill;
import org.apache.drill.exec.expr.holders.NullableBigIntHolder;
public class CompileClassWithIfs {
public static void doSomething() {
int a = 2;
NullableBigIntHolder out0 = new NullableBigIntHolder();
out0.isSet = 1;
NullableBigIntHolder out4 = new NullableBigIntHolder();
out4.isSet = 0;
NullableBigIntHolder out5 = new NullableBigIntHolder();
if (a == 0) {
out0 = out4;
} else {
}
if (out4.isSet == 0) {
out0.isSet = 1;
out5.value = 3;
} else {
out0.isSet = 0;
assert false : "Incorrect class transformation. This code should never be
executed.";
}
}
}
{code}
Code after scalar replacement:
{code:java}
package org.apache.drill;
public class CompileClassWithIfs {
public static void doSomething() {
byte var0 = 2;
boolean var1 = false;
long var2 = 0L;
var1 = true;
boolean var4 = false;
long var5 = 0L;
var4 = false;
boolean var7 = false;
long var8 = 0L;
if (var0 == 0) {
var1 = var4;
}
if (!var1) {
var1 = true;
var8 = (long)3;
} else {
var1 = false;
if (true) {
throw new AssertionError("Incorrect class transformation. This code
should never be executed.");
}
}
}
public CompileClassWithIfs() {
}
}
{code}
Please note that in the original code, assertion error will not be thrown, but
after scalar replacement {{out0}} and {{out4}} have the same values, so they
were inlined using the same variables.
Code after the changes in https://github.com/apache/drill/pull/1686:
{code:java}
package org.apache.drill;
import org.apache.drill.exec.expr.holders.NullableBigIntHolder;
public class CompileClassWithIfs {
public static void doSomething() {
byte var0 = 2;
NullableBigIntHolder var1 = new NullableBigIntHolder();
var1.isSet = 1;
NullableBigIntHolder var2 = new NullableBigIntHolder();
var2.isSet = 0;
boolean var3 = false;
long var4 = 0L;
if (var0 == 0) {
var1 = var2;
}
if (var2.isSet == 0) {
var1.isSet = 1;
var4 = (long)3;
} else {
var1.isSet = 0;
if (true) {
throw new AssertionError("Incorrect class transformation. This code
should never be executed.");
}
}
}
public CompileClassWithIfs() {
}
}
{code}
Please note, that two {{NullableBigIntHolder}} objects whose references were
assigned to each other in if the block weren't replaced since it is impossible
to produce the correct replacement.
Bytecode of the last class:
{code}
// class version 50.0 (50)
// access flags 0x21
public class org/apache/drill/CompileClassWithIfs {
// access flags 0x9
public static doSomething()V
ICONST_2
ISTORE 0
NEW org/apache/drill/exec/expr/holders/NullableBigIntHolder
DUP
INVOKESPECIAL
org/apache/drill/exec/expr/holders/NullableBigIntHolder.<init> ()V
ASTORE 1
ALOAD 1
ICONST_1
PUTFIELD org/apache/drill/exec/expr/holders/NullableBigIntHolder.isSet : I
NEW org/apache/drill/exec/expr/holders/NullableBigIntHolder
DUP
INVOKESPECIAL
org/apache/drill/exec/expr/holders/NullableBigIntHolder.<init> ()V
ASTORE 2
ALOAD 2
ICONST_0
PUTFIELD org/apache/drill/exec/expr/holders/NullableBigIntHolder.isSet : I
ICONST_0
ISTORE 3
LCONST_0
LSTORE 4
ILOAD 0
ICONST_0
IF_ICMPNE L0
ALOAD 2
ASTORE 1
L0
ALOAD 2
GETFIELD org/apache/drill/exec/expr/holders/NullableBigIntHolder.isSet : I
ICONST_0
IF_ICMPNE L1
ALOAD 1
ICONST_1
PUTFIELD org/apache/drill/exec/expr/holders/NullableBigIntHolder.isSet : I
ICONST_3
I2L
LSTORE 4
GOTO L2
L1
ALOAD 1
ICONST_0
PUTFIELD org/apache/drill/exec/expr/holders/NullableBigIntHolder.isSet : I
ICONST_0
IFNE L2
NEW java/lang/AssertionError
DUP
LDC "Incorrect class transformation. This code should never be executed."
INVOKESPECIAL java/lang/AssertionError.<init> (Ljava/lang/Object;)V
ATHROW
L2
RETURN
MAXSTACK = 3
MAXLOCALS = 6
// access flags 0x1
public <init>()V
ALOAD 0
INVOKESPECIAL java/lang/Object.<init> ()V
RETURN
MAXSTACK = 1
MAXLOCALS = 1
}
{code}
was (Author: vvysotskyi):
The problem here is connected with incorrect scalar replacement. It still
happens even for the case when a reference to the object outside of the
conditional block is changed inside the conditional block. It causes wrong
results for the case when code in this conditional block will not be executed.
The code below demonstrates this issue:
Original class:
{code:java}
package org.apache.drill;
import org.apache.drill.exec.expr.holders.NullableBigIntHolder;
public class CompileClassWithIfs {
public static void doSomething() {
int a = 2;
NullableBigIntHolder out0 = new NullableBigIntHolder();
out0.isSet = 1;
NullableBigIntHolder out4 = new NullableBigIntHolder();
out4.isSet = 0;
NullableBigIntHolder out5 = new NullableBigIntHolder();
if (a == 0) {
out0 = out4;
} else {
}
if (out4.isSet == 0) {
out0.isSet = 1;
out5.value = 3;
} else {
out0.isSet = 0;
assert false : "Incorrect class transformation. This code should never be
executed.";
}
}
}
{code}
Code after scalar replacement:
{code:java}
package org.apache.drill;
public class CompileClassWithIfs {
public static void doSomething() {
byte var0 = 2;
boolean var1 = false;
long var2 = 0L;
var1 = true;
boolean var4 = false;
long var5 = 0L;
var4 = false;
boolean var7 = false;
long var8 = 0L;
if (var0 == 0) {
var1 = var4;
}
if (!var1) {
var1 = true;
var8 = (long)3;
} else {
var1 = false;
if (true) {
throw new AssertionError("Incorrect class transformation. This code
should never be executed.");
}
}
}
public CompileClassWithIfs() {
}
}
{code}
Please note that in the original code, assertion error will not be thrown, but
after scalar replacement {{out0}} and {{out4}} have the same values, so they
were inlined using the same variables.
Code after the changes in https://github.com/apache/drill/pull/1686:
{code:java}
package org.apache.drill;
import org.apache.drill.exec.expr.holders.NullableBigIntHolder;
public class CompileClassWithIfs {
public static void doSomething() {
byte var0 = 2;
NullableBigIntHolder var1 = new NullableBigIntHolder();
var1.isSet = 1;
NullableBigIntHolder var2 = new NullableBigIntHolder();
var2.isSet = 0;
boolean var3 = false;
long var4 = 0L;
if (var0 == 0) {
var1 = var2;
}
if (var2.isSet == 0) {
var1.isSet = 1;
var4 = (long)3;
} else {
var1.isSet = 0;
if (true) {
throw new AssertionError("Incorrect class transformation. This code
should never be executed.");
}
}
}
public CompileClassWithIfs() {
}
}
{code}
Please note, that two {{NullableBigIntHolder}} objects whose references were
assigned to each other in if the block weren't replaced since it is impossible
to produce the correct replacement.
> Two CASE statements in projection influence results of each other
> -----------------------------------------------------------------
>
> Key: DRILL-6524
> URL: https://issues.apache.org/jira/browse/DRILL-6524
> Project: Apache Drill
> Issue Type: Bug
> Components: Query Planning & Optimization
> Affects Versions: 1.11.0
> Environment: Linux 3.10.0-693.21.1.el7.x86_64 #1 SMP Wed Mar 7
> 19:03:37 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux,
> NAME="CentOS Linux"
> VERSION="7 (Core)"
> apache drill 1.11.0,
> openjdk version "1.8.0_171"
> OpenJDK Runtime Environment (build 1.8.0_171-b10)
> OpenJDK 64-Bit Server VM (build 25.171-b10, mixed mode)
> Reporter: Oleksandr Chornyi
> Assignee: Volodymyr Vysotskyi
> Priority: Major
> Fix For: 1.16.0
>
>
> h3. Steps to Reproduce
> Run the following query via {{sqlline}}:
> {code:sql}
> select
> case when expr$0 = 3 then expr$0 else expr$1 end,
> case when expr$0 = 1 then expr$0 else expr$1 end
> from (values(1, 2));
> {code}
> h4. Actual Results
> {noformat}
> +---------+---------+
> | EXPR$0 | EXPR$1 |
> +---------+---------+
> | 2 | 2 |
> +---------+---------+
> {noformat}
> h4. Expected Results
> {noformat}
> +---------+---------+
> | EXPR$0 | EXPR$1 |
> +---------+---------+
> | 2 | 1 |
> +---------+---------+
> {noformat}
> Note, that changing order of CASE statements fixes the issue. The following
> query yields correct results:
> {code:sql}
> select
> case when expr$0 = 1 then expr$0 else expr$1 end,
> case when expr$0 = 3 then expr$0 else expr$1 end
> from (values(1, 2));
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
