[ 
https://issues.apache.org/jira/browse/KYLIN-3634?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16665967#comment-16665967
 ] 

WangBo commented on KYLIN-3634:
-------------------------------

Hi,shaofeng,thank you for your test and verification work.I will append some 
test about how hive and presto dealing filter value with null value.So if there 
is no more question after your review,I will close the jira.
h1. Test Process

I do the test on my laptop,hive version hive-1.2.2,hadoop version 2.7.6,presto 
version,presto version is 0.193-SNAPSHOT

 
{code:java}
// data.txt
20180103,\N,11
20180102,beijing,20
20180102,shanghai,10
//do test in hive
hive> create table null_test(day int,city string,price double)ROW FORMAT 
DELIMITED

    >   FIELDS TERMINATED BY ','

    >   LINES TERMINATED BY '\n' ;

OK

Time taken: 0.191 seconds

hive> LOAD DATA LOCAL INPATH '/xxxxxx/data.txt' OVERWRITE INTO TABLE null_test;

Loading data to table test.null_test

Table test.null_test stats: [numFiles=1, numRows=0, totalSize=56, rawDataSize=0]

OK

Time taken: 0.816 seconds

hive> select * from null_test;

OK

20180103 NULL 11.0

20180102 beijing 20.0

20180102 shanghai 10.0

Time taken: 0.271 seconds, Fetched: 3 row(s)

hive> select * from null_test where city != 'abc';

OK

20180102 beijing 20.0

20180102 shanghai 10.0

Time taken: 0.167 seconds, Fetched: 2 row(s)
hive> select * from null_test where city > 'abc';

OK

20180102 beijing 20.0

20180102 shanghai 10.0

Time taken: 0.125 seconds, Fetched: 2 row(s)
hive> select * from null_test where city < 'abc';

OK

Time taken: 0.114 seconds

 
//query in presto
presto:test> select * from null_test where city != 'abc';

   day    |   city   | price

----------+----------+-------

20180102 | beijing  |  20.0

20180102 | shanghai |  10.0

(2 rows)




Query 20181027_045059_00005_5qhgb, FINISHED, 1 node

Splits: 17 total, 17 done (100.00%)

0:02 [3 rows, 56B] [1 rows/s, 34B/s]




Query aborted by user

presto:test> select * from null_test;

   day    |   city   | price

----------+----------+-------

20180103 | NULL     |  11.0

20180102 | beijing  |  20.0

20180102 | shanghai |  10.0

(3 rows)
presto:test> select * from null_test where city > 'abc1d';

   day    |   city   | price

----------+----------+-------

20180102 | beijing  |  20.0

20180102 | shanghai |  10.0

(2 rows)

Query 20181027_051805_00012_5qhgb, FINISHED, 1 node

Splits: 17 total, 17 done (100.00%)

0:02 [3 rows, 56B] [1 rows/s, 24B/s]



Query 20181027_045109_00006_5qhgb, FINISHED, 1 node

Splits: 17 total, 17 done (100.00%)

0:00 [3 rows, 56B] [10 rows/s, 187B/s]
presto:test> select * from null_test where city < 'abc1d';

day | city | price

-----+------+-------

(0 rows)

Query 20181027_051843_00013_5qhgb, FINISHED, 1 node

Splits: 17 total, 17 done (100.00%)

0:00 [3 rows, 56B] [16 rows/s, 303B/s]

{code}
 

>From the test result,hive and presto actually excluded the rows when the 
>filter(including !=,>,<) column value is null,
h2. Presto Source Code

Hive source code is not very graceful,So I find some source code of presto.

 
{code:java}
//code1 LocalExecutionPlanner.java
catch (RuntimeException e) {
    if (!interpreterEnabled) {
        throw new PrestoException(COMPILER_ERROR, "Compiler failed and 
interpreter is disabled", e);
    }

    // compilation failed, use interpreter
    log.error(e, "Compile failed for filter=%s projections=%s sourceTypes=%s 
error=%s", filterExpression, assignments, sourceTypes, e);
}
//code2 InterpretedCursorProcessor.java
@Override
public CursorProcessorOutput process(ConnectorSession session, 
DriverYieldSignal yieldSignal, RecordCursor cursor, PageBuilder pageBuilder)
{
    checkArgument(!pageBuilder.isFull(), "page builder can't be full");
    requireNonNull(yieldSignal, "yieldSignal is null");

    int position = 0;
    while (true) {
        if (pageBuilder.isFull() || yieldSignal.isSet()) {
            return new CursorProcessorOutput(position, false);
        }

        if (!cursor.advanceNextPosition()) {
            return new CursorProcessorOutput(position, true);
        }

        if (filter(cursor)) {
            pageBuilder.declarePosition();
            for (int channel = 0; channel < projections.size(); channel++) {
                project(cursor, channel, pageBuilder);
            }
        }
        position++;
    }
}

private boolean filter(RecordCursor cursor)
{
    return filter == null || TRUE.equals(filter.evaluate(cursor));
}

//code3 ExpressionInterpreter.java
@Override
protected Object visitComparisonExpression(ComparisonExpression node, Object 
context)
{
    ComparisonExpressionType type = node.getType();

    Object left = process(node.getLeft(), context);
    if (left == null && type != ComparisonExpressionType.IS_DISTINCT_FROM) {
        return null;
    }

    Object right = process(node.getRight(), context);
    if (type == ComparisonExpressionType.IS_DISTINCT_FROM) {
        if (left == null && right == null) {
            return false;
        }
        else if (left == null || right == null) {
            return true;
        }
    }
    else if (right == null) {
        return null;
    }

    if (hasUnresolvedValue(left, right)) {
        return new ComparisonExpression(type, toExpression(left, 
type(node.getLeft())), toExpression(right, type(node.getRight())));
    }

    return invokeOperator(OperatorType.valueOf(type.name()), 
types(node.getLeft(), node.getRight()), ImmutableList.of(left, right));
}{code}
 

 

code1:

presto exeuctes fragment plan using dynamic compilation.But when compiling code 
failed and set 

compiler.interpreter-enabled = true,it will execute fragment plan using 
interpreted way.

it's hard the get content of dynamically generated code,so we use the logic of  
interpreted code as reference.

code2:

CursorProcessor looks like iterator in kylin.It returns tuple from source data.

it did some filter work,only when filter returns true,it will return true,this 
means null != true

code3:

the key method is visitComparisonExpression in ExpressionInterpreter,if the 
result of evaluating child node of comparision return null,it will return null.

So when the CursorProcessor get the null,the row will be excluded.

> When filter column has null value may cause incorrect query result
> ------------------------------------------------------------------
>
>                 Key: KYLIN-3634
>                 URL: https://issues.apache.org/jira/browse/KYLIN-3634
>             Project: Kylin
>          Issue Type: Bug
>          Components: Query Engine
>    Affects Versions: v2.0.0
>            Reporter: WangBo
>            Assignee: WangBo
>            Priority: Major
>             Fix For: v2.4.2, v2.5.1
>
>         Attachments: 
> 0001-KYLIN-3634-when-filter-column-has-null-value-may-cau.patch, 
> image-2018-10-27-14-11-57-955.png
>
>
> h1. Question
> when a column has null value,and using it as a filter column when querying, 
> and the filter value is not exist in the table,this may cause incorrect result
> h1. An Example
> h2. Table A
> the table A has three rows,city column of one row has null value
>  
> ||day||...||city||price||
> |20180101| |null|10|
> |20180101| |beijing|20|
> |20180101| |shanghai|10|
> h2. Query SQL
> select day,sum(price) from a where city <> 'abc' group by day
> h2. Correct Result
> exclude the row contains null city value
> ||day||col||
> |20180101|30|
> h2. InCorrect Result
> resullt 0 rows
> this happens in our production environment,the kylin version is 2.0.0
> h1. Analysis process
> 1,city column dosen't have a value,so the CompareTupleFilter will turn into  
> ConstantTupleFilter(see GTUtil.java)
> 2,if dimensions in the sql dosen't match all the columns using in group 
> by,the  bytesComparator used in hbase aggregation map will only compare the 
> columns using in group by
> 3,when GTAggregateScanner constructs key of aggBufMap,the key may contains 
> null value,because the comparator of aggBufMap only compares group by 
> columns,so the tuple share same group by columns may also share the same keys 
> which contains null value;This may cause kylin server receives tuples 
> contains null value;
> 4,when the code which dynamically generated by calcilte deals tuples using 
> filter,it first judges whether the column is null.Because filter column in 
> the tuple contains null value,so it always return false, no tuples will 
> return.
> h1. Solution
> when the filter column value is a invalid means not in the table,turn the 
> CompareTupleFiter into IS_NOT_NULL filter,instead of ConstantTupleFilter.TURE
>  
> Now I have test the feature in our production environment ;
> test in “mvn test” had passed,but not test in sandbox
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to