You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ab...@apache.org on 2016/11/09 17:03:20 UTC
[1/5] incubator-impala git commit: IMPALA-1430,
IMPALA-4108: codegen all builtin aggregate functions
Repository: incubator-impala
Updated Branches:
refs/heads/master 677589389 -> 0aeb68050
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/java-udf.test b/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
index c473fdf..c4e2f1c 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
@@ -1,20 +1,20 @@
====
---- QUERY
-select udf_test.hive_pi()
+select hive_pi()
---- RESULTS
3.141592653589793
---- TYPES
DOUBLE
====
---- QUERY
-select udf_test.hive_bin(100)
+select hive_bin(100)
---- RESULTS
'1100100'
---- TYPES
STRING
====
---- QUERY
-select min(udf_test.hive_pi()) from functional.alltypesagg
+select min(hive_pi()) from functional.alltypesagg
---- RESULTS
3.141592653589793
---- TYPES
@@ -22,49 +22,49 @@ DOUBLE
====
---- QUERY
# Test identity functions
-select udf_test.identity(true);
+select identity(true);
---- TYPES
boolean
---- RESULTS
true
====
---- QUERY
-select udf_test.identity(cast(10 as tinyint));
+select identity(cast(10 as tinyint));
---- TYPES
tinyint
---- RESULTS
10
====
---- QUERY
-select udf_test.identity(cast(10 as smallint));
+select identity(cast(10 as smallint));
---- TYPES
smallint
---- RESULTS
10
====
---- QUERY
-select udf_test.identity(cast(10 as int));
+select identity(cast(10 as int));
---- TYPES
int
---- RESULTS
10
====
---- QUERY
-select udf_test.identity(cast(10 as bigint));
+select identity(cast(10 as bigint));
---- TYPES
bigint
---- RESULTS
10
====
---- QUERY
-select udf_test.identity(cast(10.0 as float));
+select identity(cast(10.0 as float));
---- TYPES
float
---- RESULTS
10
====
---- QUERY
-select udf_test.identity(cast(10.0 as double));
+select identity(cast(10.0 as double));
---- TYPES
double
---- RESULTS
@@ -73,16 +73,16 @@ double
---- QUERY
# IMPALA-1456. Each "identity" call below tests a different type (BytesWritable, Text,
# and String).
-select udf_test.identity("why hello there"),
- udf_test.identity("why", " hello there"),
- udf_test.identity("why", " hello", " there");
+select identity("why hello there"),
+ identity("why", " hello there"),
+ identity("why", " hello", " there");
---- TYPES
string, string, string
---- RESULTS
'why hello there','why hello there','why hello there'
====
---- QUERY
-select udf_test.identity(NULL);
+select identity(NULL);
---- TYPES
boolean
---- RESULTS
@@ -91,9 +91,9 @@ NULL
---- QUERY
# IMPALA-1134. Each "identity" call below tests a different type (BytesWritable, Text,
# and String). The different types are handled slightly differently.
-select length(udf_test.identity("0123456789")),
- length(udf_test.identity("0123456789", "0123456789")),
- length(udf_test.identity("0123456789", "0123456789", "0123456789"));
+select length(identity("0123456789")),
+ length(identity("0123456789", "0123456789")),
+ length(identity("0123456789", "0123456789", "0123456789"));
---- TYPES
int, int, int
---- RESULTS
@@ -101,14 +101,14 @@ int, int, int
====
---- QUERY
# IMPALA-1392: Hive UDFs that throw exceptions should return NULL
-select udf_test.throws_exception();
+select throws_exception();
---- TYPES
boolean
---- RESULTS
NULL
====
---- QUERY
-select udf_test.throws_exception() from functional.alltypestiny;
+select throws_exception() from functional.alltypestiny;
---- TYPES
boolean
---- RESULTS
@@ -122,49 +122,49 @@ NULL
NULL
====
---- QUERY
-select udf_test.hive_add(cast(1 as int), cast(2 as int));
+select hive_add(cast(1 as int), cast(2 as int));
---- TYPES
int
---- RESULTS
3
====
---- QUERY
-select udf_test.hive_add(udf_test.hive_add(cast(1 as int), cast(2 as int)), cast(2 as int));
+select hive_add(hive_add(cast(1 as int), cast(2 as int)), cast(2 as int));
---- TYPES
int
---- RESULTS
5
====
---- QUERY
-select udf_test.hive_add(cast(udf_test.hive_add(cast(1 as int), cast(2 as int)) - udf_test.hive_add(cast(2 as int), cast(1 as int)) as int), cast(2 as int));
+select hive_add(cast(hive_add(cast(1 as int), cast(2 as int)) - hive_add(cast(2 as int), cast(1 as int)) as int), cast(2 as int));
---- TYPES
int
---- RESULTS
2
====
---- QUERY
-select udf_test.hive_add(cast(1 as smallint), cast(2 as smallint));
+select hive_add(cast(1 as smallint), cast(2 as smallint));
---- TYPES
smallint
---- RESULTS
3
====
---- QUERY
-select udf_test.hive_add(cast(1.0 as float), cast(2.0 as float));
+select hive_add(cast(1.0 as float), cast(2.0 as float));
---- TYPES
float
---- RESULTS
3.0
====
---- QUERY
-select udf_test.hive_add(cast(1.0 as double), cast(2.0 as double));
+select hive_add(cast(1.0 as double), cast(2.0 as double));
---- TYPES
double
---- RESULTS
3.0
====
---- QUERY
-select udf_test.hive_add(cast(1 as boolean), cast(0 as boolean));
+select hive_add(cast(1 as boolean), cast(0 as boolean));
---- TYPES
boolean
---- RESULTS
@@ -172,63 +172,63 @@ false
====
---- QUERY
# Testing whether all of persistent Java udfs are accessible.
-select java_udfs_test.identity(true);
+select identity_anytype(true);
---- TYPES
boolean
---- RESULTS
true
====
---- QUERY
-select java_udfs_test.identity(cast(10 as tinyint));
+select identity_anytype(cast(10 as tinyint));
---- TYPES
tinyint
---- RESULTS
10
====
---- QUERY
-select java_udfs_test.identity(cast(10 as smallint));
+select identity_anytype(cast(10 as smallint));
---- TYPES
smallint
---- RESULTS
10
====
---- QUERY
-select java_udfs_test.identity(cast(10 as int));
+select identity_anytype(cast(10 as int));
---- TYPES
int
---- RESULTS
10
====
---- QUERY
-select java_udfs_test.identity(cast(10 as bigint));
+select identity_anytype(cast(10 as bigint));
---- TYPES
bigint
---- RESULTS
10
====
---- QUERY
-select java_udfs_test.identity(cast(10.0 as float));
+select identity_anytype(cast(10.0 as float));
---- TYPES
float
---- RESULTS
10
====
---- QUERY
-select java_udfs_test.identity(cast(10.0 as double));
+select identity_anytype(cast(10.0 as double));
---- TYPES
double
---- RESULTS
10
====
---- QUERY
-select java_udfs_test.identity("a", "b");
+select identity_anytype("a", "b");
---- TYPES
string
---- RESULTS
'ab'
====
---- QUERY
-select java_udfs_test.identity("a", "b", "c");
+select identity_anytype("a", "b", "c");
---- TYPES
string
---- RESULTS
@@ -238,37 +238,37 @@ string
# IMPALA-3378: test many Java UDFs being opened and run concurrently
select * from
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(bool_col) union all
+ where identity(bool_col) union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(tinyint_col) > 1 union all
+ where identity(tinyint_col) > 1 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(smallint_col) > 1 union all
+ where identity(smallint_col) > 1 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(int_col) > 1 union all
+ where identity(int_col) > 1 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(bigint_col) > 1 union all
+ where identity(bigint_col) > 1 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(float_col) > 1.0 union all
+ where identity(float_col) > 1.0 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(double_col) > 1.0 union all
+ where identity(double_col) > 1.0 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(string_col) > '1' union all
+ where identity(string_col) > '1' union all
(select max(int_col) from functional.alltypesagg
- where not udf_test.identity(bool_col) union all
+ where not identity(bool_col) union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(tinyint_col) > 2 union all
+ where identity(tinyint_col) > 2 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(smallint_col) > 2 union all
+ where identity(smallint_col) > 2 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(int_col) > 2 union all
+ where identity(int_col) > 2 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(bigint_col) > 2 union all
+ where identity(bigint_col) > 2 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(float_col) > 2.0 union all
+ where identity(float_col) > 2.0 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(double_col) > 2.0 union all
+ where identity(double_col) > 2.0 union all
(select max(int_col) from functional.alltypesagg
- where udf_test.identity(string_col) > '2'
+ where identity(string_col) > '2'
)))))))))))))))) v
---- TYPES
INT
@@ -301,7 +301,7 @@ values('toast'), ('scone'), ('stuff'), ('sssss'), ('yes'), ('scone'), ('stuff');
# Regression test for IMPALA-4266: memory management bugs with output strings from
# Java UDFS, exposed by using the UDF as a grouping key in an aggregation.
# The UDF replaces "s" with "ss" in the strings.
-select distinct udf_test.replace_string(_c0) as es
+select distinct replace_string(_c0) as es
from replace_string_input
order by 1;
---- TYPES
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/libs_with_same_filenames.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/libs_with_same_filenames.test b/testdata/workloads/functional-query/queries/QueryTest/libs_with_same_filenames.test
index cb46d9c..64fdced 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/libs_with_same_filenames.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/libs_with_same_filenames.test
@@ -1,24 +1,21 @@
====
---- QUERY
-drop function if exists same_lib_filename_udf_test.no_args();
-drop function if exists same_lib_filename_udf_test.no_args2();
-
-create function same_lib_filename_udf_test.no_args() returns string
+create function no_args() returns string
location '$FILESYSTEM_PREFIX/test-warehouse/libTestUdfs.so' symbol='NoArgs';
-create function same_lib_filename_udf_test.no_args2() returns string
+create function no_args2() returns string
location '$FILESYSTEM_PREFIX/test-warehouse/udf_test/libTestUdfs.so' symbol='NoArgs';
---- RESULTS
====
---- QUERY
-select same_lib_filename_udf_test.no_args();
+select no_args();
---- TYPES
string
---- RESULTS
'string'
====
---- QUERY
-select same_lib_filename_udf_test.no_args2();
+select no_args2();
---- TYPES
string
---- RESULTS
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/load-java-udfs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/load-java-udfs.test b/testdata/workloads/functional-query/queries/QueryTest/load-java-udfs.test
index ab0c8bb..5ff4488 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/load-java-udfs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/load-java-udfs.test
@@ -1,130 +1,99 @@
====
---- QUERY
-drop function if exists udf_test.hive_pi();
-drop function if exists udf_test.hive_round(double);
-drop function if exists udf_test.hive_floor(double);
-drop function if exists udf_test.hive_mod(int, int);
-drop function if exists udf_test.hive_bin(bigint);
-drop function if exists udf_test.hive_lower(string);
-
-drop function if exists udf_test.identity(boolean);
-drop function if exists udf_test.identity(tinyint);
-drop function if exists udf_test.identity(smallint);
-drop function if exists udf_test.identity(int);
-drop function if exists udf_test.identity(bigint);
-drop function if exists udf_test.identity(float);
-drop function if exists udf_test.identity(double);
-drop function if exists udf_test.identity(string);
-drop function if exists udf_test.identity(string, string);
-drop function if exists udf_test.identity(string, string, string);
-drop function if exists udf_test.identity(timestamp);
-
-drop function if exists udf_test.hive_add(int, int);
-drop function if exists udf_test.hive_add(float, float);
-drop function if exists udf_test.hive_add(double, double);
-drop function if exists udf_test.hive_add(smallint, smallint);
-drop function if exists udf_test.hive_add(boolean, boolean);
-
-drop function if exists udf_test.throws_exception();
-
-drop function if exists java_udfs_test.identity;
-
-drop function if exists udf_test.replace_string(string);
-
-create function udf_test.hive_pi() returns double
+create function hive_pi() returns double
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFPI';
-create function udf_test.hive_round(double) returns double
+create function hive_round(double) returns double
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFRound';
-create function udf_test.hive_floor(double) returns bigint
+create function hive_floor(double) returns bigint
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFFloor';
-create function udf_test.hive_mod(int, int) returns int
+create function hive_mod(int, int) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFPosMod';
-create function udf_test.hive_bin(bigint) returns string
+create function hive_bin(bigint) returns string
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFBin';
-create function udf_test.hive_lower(string) returns string
+create function hive_lower(string) returns string
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFLower';
# Used to test persistent java functions
-create function java_udfs_test.identity
+create function identity_anytype
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(boolean) returns boolean
+create function identity(boolean) returns boolean
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(tinyint) returns tinyint
+create function identity(tinyint) returns tinyint
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(smallint) returns smallint
+create function identity(smallint) returns smallint
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(int) returns int
+create function identity(int) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(bigint) returns bigint
+create function identity(bigint) returns bigint
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(float) returns float
+create function identity(float) returns float
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(double) returns double
+create function identity(double) returns double
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(string) returns string
+create function identity(string) returns string
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(string, string) returns string
+create function identity(string, string) returns string
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.identity(string, string, string) returns string
+create function identity(string, string, string) returns string
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.hive_add(int, int) returns int
+create function hive_add(int, int) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.hive_add(smallint, smallint) returns smallint
+create function hive_add(smallint, smallint) returns smallint
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.hive_add(float, float) returns float
+create function hive_add(float, float) returns float
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.hive_add(double, double) returns double
+create function hive_add(double, double) returns double
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.hive_add(boolean, boolean) returns boolean
+create function hive_add(boolean, boolean) returns boolean
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdf';
-create function udf_test.throws_exception() returns boolean
+create function throws_exception() returns boolean
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestUdfException';
-create function udf_test.replace_string(string) returns string
+create function replace_string(string) returns string
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.ReplaceStringUdf';
====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/uda-mem-limit.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/uda-mem-limit.test b/testdata/workloads/functional-query/queries/QueryTest/uda-mem-limit.test
index 3a21274..b7c2f5f 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/uda-mem-limit.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/uda-mem-limit.test
@@ -1,8 +1,5 @@
====
---- QUERY
-create database if not exists native_function_test;
-use native_function_test;
-
drop function if exists agg_memtest(bigint);
create aggregate function agg_memtest(bigint) returns bigint
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/uda.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/uda.test b/testdata/workloads/functional-query/queries/QueryTest/uda.test
index 21877cf..05e24e7 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/uda.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/uda.test
@@ -41,3 +41,31 @@ true
---- TYPES
boolean
====
+---- QUERY
+# Test with even number of input rows.
+select toggle_null(id), count(*)
+from functional_parquet.alltypesagg
+---- RESULTS
+NULL,11000
+---- TYPES
+int,bigint
+====
+---- QUERY
+# Test with odd number of input rows.
+select toggle_null(id), count(*)
+from functional_parquet.alltypesagg
+where id <= 9998
+---- RESULTS
+1,10999
+---- TYPES
+int,bigint
+====
+---- QUERY
+# Test that input NULLs are passed to aggregate functions ok.
+select count_nulls(tinyint_col), count(*)
+from functional.alltypesagg
+---- RESULTS
+2000,11000
+---- TYPES
+bigint,bigint
+====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/udf-codegen-required.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/udf-codegen-required.test b/testdata/workloads/functional-query/queries/QueryTest/udf-codegen-required.test
new file mode 100644
index 0000000..07c93c3
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/udf-codegen-required.test
@@ -0,0 +1,10 @@
+====
+---- QUERY
+# Codegen is required for > 20 args.
+select twenty_one_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21);
+---- TYPES
+INT
+---- RESULTS
+231
+====
+
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test b/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
index 948d584..73aee7e 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
@@ -1,23 +1,19 @@
====
---- QUERY
-create database if not exists udf_test_errors;
----- RESULTS
-====
----- QUERY
-create function if not exists udf_test_errors.hive_pi() returns double
+create function if not exists hive_pi() returns double
location '$FILESYSTEM_PREFIX/test-warehouse/hive-exec.jar'
symbol='org.apache.hadoop.hive.ql.udf.UDFPI';
---- RESULTS
====
---- QUERY
-create function if not exists udf_test_errors.foo() returns double
+create function if not exists foo() returns double
location '$FILESYSTEM_PREFIX/test-warehouse/not-a-real-file.so'
symbol='FnDoesNotExist';
---- CATCH
Could not load binary: $FILESYSTEM_PREFIX/test-warehouse/not-a-real-file.so
====
---- QUERY
-create function if not exists udf_test_errors.foo() returns double
+create function if not exists foo() returns double
location '$FILESYSTEM_PREFIX/test-warehouse/not-a-real-file.so'
symbol='FnDoesNotExist';
---- CATCH
@@ -25,7 +21,7 @@ Could not load binary: $FILESYSTEM_PREFIX/test-warehouse/not-a-real-file.so
====
---- QUERY
# This test is run with codegen disabled. Interpretation only handles up to 20 arguments.
-create function if not exists udf_test_errors.twenty_args(int, int, int, int, int, int,
+create function if not exists twenty_args(int, int, int, int, int, int,
int, int, int, int, int, int, int, int, int, int, int, int, int, int) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/libTestUdfs.so'
symbol='TwentyArgs';
@@ -33,7 +29,7 @@ symbol='TwentyArgs';
====
---- QUERY
# Verifies that interpretation can support up to 20 arguments
-select udf_test_errors.twenty_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20);
+select twenty_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20);
---- TYPES
INT
---- RESULTS
@@ -41,7 +37,7 @@ INT
====
---- QUERY
# This test is run with codegen disabled. Interpretation only handles up to 20 arguments.
-create function if not exists udf_test_errors.twenty_one_args(int, int, int, int, int, int,
+create function if not exists twenty_one_args(int, int, int, int, int, int,
int, int, int, int, int, int, int, int, int, int, int, int, int, int, int) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/libTestUdfs.so'
symbol='TwentyOneArgs';
@@ -49,35 +45,36 @@ symbol='TwentyOneArgs';
====
---- QUERY
# Verifies that interpretation fails with more than 20 arguments.
-select udf_test_errors.twenty_one_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21);
+select twenty_one_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21);
---- CATCH
Cannot interpret native UDF 'twenty_one_args': number of arguments is more than 20. Codegen is needed. Please set DISABLE_CODEGEN to false.
====
---- QUERY
# This test is run with codegen disabled. IR UDF will fail.
-create function if not exists udf_test_errors.nine_args_ir(int, int, int, int, int, int,
+create function if not exists nine_args_ir(int, int, int, int, int, int,
int, int, int) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/test-udfs.ll'
symbol='NineArgs';
---- RESULTS
====
---- QUERY
-select udf_test_errors.nine_args_ir(1,2,3,4,5,6,7,8,9);
+select nine_args_ir(1,2,3,4,5,6,7,8,9);
---- CATCH
Cannot interpret LLVM IR UDF 'nine_args_ir': Codegen is needed. Please set DISABLE_CODEGEN to false.
====
---- QUERY
-drop database udf_test_errors;
+use default;
+drop database $DATABASE;
---- CATCH
-Cannot drop non-empty database: udf_test_errors
+Cannot drop non-empty database:
====
---- QUERY
-drop function udf_test_errors.hive_pi();
-drop function udf_test_errors.twenty_args(int, int, int, int, int, int, int, int,
+use $DATABASE;
+drop function hive_pi();
+drop function twenty_args(int, int, int, int, int, int, int, int,
int, int, int, int, int, int, int, int, int, int, int, int);
-drop function udf_test_errors.twenty_one_args(int, int, int, int, int, int, int, int,
+drop function twenty_one_args(int, int, int, int, int, int, int, int,
int, int, int, int, int, int, int, int, int, int, int, int, int);
-drop function udf_test_errors.nine_args_ir(int, int, int, int, int, int, int, int, int);
-drop database udf_test_errors;
+drop function nine_args_ir(int, int, int, int, int, int, int, int, int);
---- RESULTS
====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/udf-mem-limit.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/udf-mem-limit.test b/testdata/workloads/functional-query/queries/QueryTest/udf-mem-limit.test
index f9b8d4b..3f23aeb 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/udf-mem-limit.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/udf-mem-limit.test
@@ -1,10 +1,5 @@
====
---- QUERY
-create database if not exists native_function_test;
-use native_function_test;
-
-drop function if exists memtest(bigint);
-
create function memtest(bigint) returns bigint
location '$FILESYSTEM_PREFIX/test-warehouse/libTestUdfs.so' symbol='MemTest'
prepare_fn='MemTestPrepare' close_fn='MemTestClose';
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/testdata/workloads/functional-query/queries/QueryTest/udf.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/udf.test b/testdata/workloads/functional-query/queries/QueryTest/udf.test
index 5cbbecb..ef777a1 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/udf.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/udf.test
@@ -536,10 +536,3 @@ INT
---- RESULTS
210
====
----- QUERY
-select twenty_one_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21);
----- TYPES
-INT
----- RESULTS
-231
-====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/tests/common/test_result_verifier.py
----------------------------------------------------------------------
diff --git a/tests/common/test_result_verifier.py b/tests/common/test_result_verifier.py
index 6bf991e..80daf09 100644
--- a/tests/common/test_result_verifier.py
+++ b/tests/common/test_result_verifier.py
@@ -482,3 +482,25 @@ def verify_runtime_profile(expected, actual):
assert len(unmatched_lines) == 0, ("Did not find matches for lines in runtime profile:"
"\nEXPECTED LINES:\n%s\n\nACTUAL PROFILE:\n%s" % ('\n'.join(unmatched_lines),
actual))
+
+def get_node_exec_options(profile_string, exec_node_id):
+ """ Return a list with all of the ExecOption strings for the given exec node id. """
+ results = []
+ matched_node = False
+ id_string = "(id={0})".format(exec_node_id)
+ for line in profile_string.splitlines():
+ if matched_node and line.strip().startswith("ExecOption:"):
+ results.append(line.strip())
+ matched_node = False
+ if id_string in line:
+ # Check for the ExecOption string on the next line.
+ matched_node = True
+ return results
+
+def assert_codegen_enabled(profile_string, exec_node_ids):
+ """ Check that codegen is enabled for the given exec node ids by parsing the text
+ runtime profile in 'profile_string'"""
+ for exec_node_id in exec_node_ids:
+ for exec_options in get_node_exec_options(profile_string, exec_node_id):
+ assert 'Codegen Enabled' in exec_options
+ assert not 'Codegen Disabled' in exec_options
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/tests/query_test/test_aggregation.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_aggregation.py b/tests/query_test/test_aggregation.py
index 82d50c8..c44c784 100644
--- a/tests/query_test/test_aggregation.py
+++ b/tests/query_test/test_aggregation.py
@@ -19,28 +19,64 @@
#
import pytest
+from tests.common.environ import USING_OLD_AGGS_JOINS
from tests.common.impala_test_suite import ImpalaTestSuite
+from tests.common.skip import SkipIfOldAggsJoins
from tests.common.test_dimensions import (
create_exec_option_dimension,
create_uncompressed_text_dimension)
+from tests.common.test_result_verifier import assert_codegen_enabled
from tests.common.test_vector import TestDimension
-from tests.common.skip import SkipIfOldAggsJoins
-
-agg_functions = ['sum', 'count', 'min', 'max', 'avg']
-data_types = ['int', 'bool', 'double', 'bigint', 'tinyint',
- 'smallint', 'float', 'timestamp']
+# Test dimensions for TestAggregation.
+AGG_FUNCTIONS = ['sum', 'count', 'min', 'max', 'avg', 'ndv']
+DATA_TYPES = ['int', 'bool', 'double', 'bigint', 'tinyint',
+ 'smallint', 'float', 'timestamp', 'string']
+# Lookup table for TestAggregation results.
result_lut = {
- # TODO: Add verification for other types
'sum-tinyint': 45000, 'avg-tinyint': 5, 'count-tinyint': 9000,
- 'min-tinyint': 1, 'max-tinyint': 9,
+ 'min-tinyint': 1, 'max-tinyint': 9, 'ndv-tinyint': 9,
'sum-smallint': 495000, 'avg-smallint': 50, 'count-smallint': 9900,
- 'min-smallint': 1, 'max-smallint': 99,
+ 'min-smallint': 1, 'max-smallint': 99, 'ndv-smallint': 99,
'sum-int': 4995000, 'avg-int': 500, 'count-int': 9990,
- 'min-int': 1, 'max-int': 999,
+ 'min-int': 1, 'max-int': 999, 'ndv-int': 999,
'sum-bigint': 49950000, 'avg-bigint': 5000, 'count-bigint': 9990,
- 'min-bigint': 10, 'max-bigint': 9990,
+ 'min-bigint': 10, 'max-bigint' : 9990, 'ndv-bigint': 999,
+ 'sum-bool': 5000, 'count-bool': 10000, 'min-bool': 'false',
+ 'max-bool': 'true', 'avg-bool': 0.5, 'ndv-bool': 2,
+ 'sum-double': 50449500.0, 'count-double': 9990, 'min-double': 10.1,
+ 'max-double': 10089.9, 'avg-double': 5050.0, 'ndv-double': 999,
+ 'sum-float': 5494500.0, 'count-float': 9990, 'min-float': 1.10,
+ 'max-float': 1098.9, 'avg-float': 550.0, 'ndv-float': 999,
+ 'count-timestamp': 10000, 'min-timestamp': '2010-01-01 00:00:00',
+ 'max-timestamp': '2010-01-10 18:02:05.100000000',
+ 'avg-timestamp': '2010-01-05 20:47:11.705080000', 'ndv-timestamp': 10000,
+ 'count-string': 10000, 'min-string': '0', 'max-string': '999', 'ndv-string': 999,
+ 'sum-distinct-tinyint': 45, 'count-distinct-tinyint': 9, 'min-distinct-tinyint': 1,
+ 'max-distinct-tinyint': 9, 'avg-distinct-tinyint': 5, 'ndv-distinct-tinyint': 9,
+ 'sum-distinct-smallint': 4950, 'count-distinct-smallint': 99,
+ 'min-distinct-smallint': 1, 'max-distinct-smallint': 99,
+ 'avg-distinct-smallint': 50, 'ndv-distinct-smallint': 99,
+ 'sum-distinct-int': 499500, 'count-distinct-int': 999, 'min-distinct-int': 1,
+ 'max-distinct-int': 999, 'avg-distinct-int': 500, 'ndv-distinct-int': 999,
+ 'sum-distinct-bigint': 4995000, 'count-distinct-bigint': 999, 'min-distinct-bigint': 10,
+ 'max-distinct-bigint': 9990, 'avg-distinct-bigint': 5000,
+ 'ndv-distinct-bigint': 999,
+ 'sum-distinct-bool': 1, 'count-distinct-bool': 2, 'min-distinct-bool': 'false',
+ 'max-distinct-bool': 'true', 'avg-distinct-bool': 0.5, 'ndv-distinct-bool': 2,
+ 'sum-distinct-double': 5044950.0, 'count-distinct-double': 999,
+ 'min-distinct-double': 10.1, 'max-distinct-double': 10089.9,
+ 'avg-distinct-double': 5050.0, 'ndv-distinct-double': 999,
+ 'sum-distinct-float': 549450.0, 'count-distinct-float': 999, 'min-distinct-float': 1.1,
+ 'max-distinct-float': 1098.9, 'avg-distinct-float': 550.0,
+ 'ndv-distinct-float': 999,
+ 'count-distinct-timestamp': 10000, 'min-distinct-timestamp': '2010-01-01 00:00:00',
+ 'max-distinct-timestamp': '2010-01-10 18:02:05.100000000',
+ 'avg-distinct-timestamp': '2010-01-05 20:47:11.705080000',
+ 'ndv-distinct-timestamp': 10000,
+ 'count-distinct-string': 1000, 'min-distinct-string': '0',
+ 'max-distinct-string': '999', 'ndv-distinct-string': 999,
}
class TestAggregation(ImpalaTestSuite):
@@ -53,8 +89,8 @@ class TestAggregation(ImpalaTestSuite):
super(TestAggregation, cls).add_test_dimensions()
# Add two more dimensions
- cls.TestMatrix.add_dimension(TestDimension('agg_func', *agg_functions))
- cls.TestMatrix.add_dimension(TestDimension('data_type', *data_types))
+ cls.TestMatrix.add_dimension(TestDimension('agg_func', *AGG_FUNCTIONS))
+ cls.TestMatrix.add_dimension(TestDimension('data_type', *DATA_TYPES))
cls.TestMatrix.add_constraint(lambda v: cls.is_valid_vector(v))
@classmethod
@@ -68,30 +104,63 @@ class TestAggregation(ImpalaTestSuite):
if vector.get_value('exec_option')['batch_size'] != 0: return False
# Avro doesn't have timestamp type
+ non_numeric = data_type in ['bool', 'string']
if file_format == 'avro' and data_type == 'timestamp':
return False
- elif agg_func not in ['min', 'max', 'count'] and data_type == 'bool':
+ elif non_numeric and agg_func not in ['min', 'max', 'count', 'ndv']:
return False
elif agg_func == 'sum' and data_type == 'timestamp':
return False
return True
def test_aggregation(self, vector):
+ exec_option = vector.get_value('exec_option')
+ disable_codegen = exec_option['disable_codegen']
+ # The old aggregation node does not support codegen for all aggregate functions.
+ check_codegen_enabled = not disable_codegen and not USING_OLD_AGGS_JOINS
data_type, agg_func = (vector.get_value('data_type'), vector.get_value('agg_func'))
+
query = 'select %s(%s_col) from alltypesagg where day is not null' % (agg_func,
data_type)
- result = self.execute_scalar(query, vector.get_value('exec_option'),
- table_format=vector.get_value('table_format'))
- if 'int' in data_type:
- assert result_lut['%s-%s' % (agg_func, data_type)] == int(result)
-
- # AVG
- if vector.get_value('data_type') == 'timestamp' and\
- vector.get_value('agg_func') == 'avg':
- return
+ result = self.execute_query(query, exec_option,
+ table_format=vector.get_value('table_format'))
+ assert len(result.data) == 1
+ self.verify_agg_result(agg_func, data_type, False, result.data[0]);
+
+ if check_codegen_enabled:
+ # Verify codegen was enabled for both stages of the aggregation.
+ assert_codegen_enabled(result.runtime_profile, [1, 3])
+
query = 'select %s(DISTINCT(%s_col)) from alltypesagg where day is not null' % (
agg_func, data_type)
- result = self.execute_scalar(query, vector.get_value('exec_option'))
+ result = self.execute_query(query, vector.get_value('exec_option'))
+ assert len(result.data) == 1
+ self.verify_agg_result(agg_func, data_type, True, result.data[0]);
+
+ if check_codegen_enabled:
+ # Verify codegen was enabled for all stages of the aggregation.
+ assert_codegen_enabled(result.runtime_profile, [1, 2, 4, 6])
+
+ def verify_agg_result(self, agg_func, data_type, distinct, actual_string):
+ key = '%s-%s%s' % (agg_func, 'distinct-' if distinct else '', data_type)
+
+ if agg_func == 'ndv':
+ # NDV is inherently approximate. Compare with some tolerance.
+ err = abs(result_lut[key] - int(actual_string))
+ rel_err = err / float(result_lut[key])
+ print key, result_lut[key], actual_string,abs(result_lut[key] - int(actual_string))
+ assert err <= 1 or rel_err < 0.05
+ elif data_type in ('float', 'double') and agg_func != 'count':
+ # Compare with a margin of error.
+ delta = 1e6 if data_type == 'double' else 1e3
+ assert abs(result_lut[key] - float(actual_string)) < delta
+ elif data_type == 'timestamp' and agg_func != 'count':
+ # Strip off everything past 10s of microseconds.
+ ignore_digits = 4
+ assert result_lut[key][:-ignore_digits] == actual_string[:-ignore_digits]
+ else:
+ assert str(result_lut[key]) == actual_string
+
class TestAggregationQueries(ImpalaTestSuite):
"""Run the aggregation test suite, with codegen enabled and disabled, to exercise our
@@ -140,6 +209,7 @@ class TestAggregationQueries(ImpalaTestSuite):
first phase is running on multiple nodes). Need to pull the result apart and
compare the actual items)"""
exec_option = vector.get_value('exec_option')
+ disable_codegen = exec_option['disable_codegen']
table_format = vector.get_value('table_format')
# Test group_concat distinct with other aggregate function and groupings.
# expected result is the row: 2010,'1, 2, 3, 4','1-2-3-4','2|3|1|4',40,4
@@ -156,6 +226,10 @@ class TestAggregationQueries(ImpalaTestSuite):
assert(set(row[i].split(delimiter[i-1])) == set(['1', '2', '3', '4']))
assert(row[4] == '40')
assert(row[5] == '4')
+ check_codegen_enabled = not disable_codegen and not USING_OLD_AGGS_JOINS
+ if check_codegen_enabled:
+ # Verify codegen was enabled for all three stages of the aggregation.
+ assert_codegen_enabled(result.runtime_profile, [1, 2, 4])
# Test group_concat distinct with arrow delimiter, with multiple rows
query = """select day, group_concat(distinct string_col, "->")
@@ -185,6 +259,10 @@ class TestAggregationQueries(ImpalaTestSuite):
where int_col < 10"""
result = self.execute_query(query, exec_option, table_format=table_format)
assert(set((result.data)[0].split(" ")) == set(['1','2','3','4','5','6','7','8','9']))
+ if check_codegen_enabled:
+ # Verify codegen was enabled for all four stages of the aggregation.
+ assert_codegen_enabled(result.runtime_profile, [1, 2, 4, 6])
+
class TestTPCHAggregationQueries(ImpalaTestSuite):
# Uses the TPC-H dataset in order to have larger aggregations.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/tests/query_test/test_udfs.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_udfs.py b/tests/query_test/test_udfs.py
index 0497cbd..5e9fdd9 100644
--- a/tests/query_test/test_udfs.py
+++ b/tests/query_test/test_udfs.py
@@ -24,7 +24,7 @@ from tests.common.impala_cluster import ImpalaCluster
from tests.common.impala_test_suite import ImpalaTestSuite
from tests.common.skip import SkipIfLocal
from tests.common.test_dimensions import (
- create_single_exec_option_dimension,
+ create_exec_option_dimension,
create_uncompressed_text_dimension)
from tests.util.calculation_util import get_random_id
from tests.util.filesystem_utils import get_fs_path, IS_S3
@@ -37,48 +37,46 @@ class TestUdfs(ImpalaTestSuite):
@classmethod
def add_test_dimensions(cls):
super(TestUdfs, cls).add_test_dimensions()
- # Without limiting the test suite to a single exec option, the tests will fail
- # because the same test case may be executed in parallel with different exec option
- # values leading to conflicting DDL ops.
- cls.TestMatrix.add_dimension(create_single_exec_option_dimension())
+ cls.TestMatrix.add_dimension(
+ create_exec_option_dimension(disable_codegen_options=[False, True]))
# There is no reason to run these tests using all dimensions.
cls.TestMatrix.add_dimension(create_uncompressed_text_dimension(cls.get_workload()))
- def test_native_functions(self, vector):
- database = 'native_function_test'
-
+ def test_native_functions(self, vector, unique_database):
self.__load_functions(
- self.create_udfs_template, vector, database,
+ self.create_udfs_template, vector, unique_database,
get_fs_path('/test-warehouse/libTestUdfs.so'))
self.__load_functions(
- self.create_sample_udas_template, vector, database,
+ self.create_sample_udas_template, vector, unique_database,
get_fs_path('/test-warehouse/libudasample.so'))
self.__load_functions(
- self.create_test_udas_template, vector, database,
+ self.create_test_udas_template, vector, unique_database,
get_fs_path('/test-warehouse/libTestUdas.so'))
- self.run_test_case('QueryTest/udf', vector, use_db=database)
- if not IS_S3: # S3 doesn't support INSERT
- self.run_test_case('QueryTest/udf-init-close', vector, use_db=database)
- self.run_test_case('QueryTest/uda', vector, use_db=database)
+ self.run_test_case('QueryTest/udf', vector, use_db=unique_database)
+ if not vector.get_value('exec_option')['disable_codegen']:
+ self.run_test_case('QueryTest/udf-codegen-required', vector, use_db=unique_database)
+ self.run_test_case('QueryTest/udf-init-close', vector, use_db=unique_database)
+ self.run_test_case('QueryTest/uda', vector, use_db=unique_database)
- def test_ir_functions(self, vector):
- database = 'ir_function_test'
+ def test_ir_functions(self, vector, unique_database):
+ if vector.get_value('exec_option')['disable_codegen']:
+ # IR functions require codegen to be enabled.
+ return
self.__load_functions(
- self.create_udfs_template, vector, database,
+ self.create_udfs_template, vector, unique_database,
get_fs_path('/test-warehouse/test-udfs.ll'))
- self.run_test_case('QueryTest/udf', vector, use_db=database)
- if not IS_S3: # S3 doesn't support INSERT
- self.run_test_case('QueryTest/udf-init-close', vector, use_db=database)
+ self.run_test_case('QueryTest/udf', vector, use_db=unique_database)
+ self.run_test_case('QueryTest/udf-init-close', vector, use_db=unique_database)
- def test_udf_errors(self, vector):
+ def test_udf_errors(self, vector, unique_database):
# Disable codegen to force interpretation path to be taken.
# Aim to exercise two failure cases:
# 1. too many arguments
# 2. IR UDF
vector.get_value('exec_option')['disable_codegen'] = 1
- self.run_test_case('QueryTest/udf-errors', vector)
+ self.run_test_case('QueryTest/udf-errors', vector, use_db=unique_database)
def test_udf_invalid_symbol(self, vector):
""" IMPALA-1642: Impala crashes if the symbol for a Hive UDF doesn't exist
@@ -98,17 +96,9 @@ class TestUdfs(ImpalaTestSuite):
finally:
self.client.execute(drop_fn_stmt)
- def test_java_udfs(self, vector):
- self.client.execute("create database if not exists java_udfs_test "
- "location '%s'" % get_fs_path('/test-warehouse/java_udf_test.db'))
- self.client.execute("create database if not exists udf_test "
- "location '%s'" % get_fs_path('/test-warehouse/udf_test.db'))
- try:
- self.run_test_case('QueryTest/load-java-udfs', vector)
- self.run_test_case('QueryTest/java-udf', vector)
- finally:
- self.client.execute("drop database if exists java_udfs_test cascade")
- self.client.execute("drop database if exists udf_test cascade")
+ def test_java_udfs(self, vector, unique_database):
+ self.run_test_case('QueryTest/load-java-udfs', vector, use_db=unique_database)
+ self.run_test_case('QueryTest/java-udf', vector, use_db=unique_database)
@SkipIfLocal.multiple_impalad
def test_hive_udfs_missing_jar(self, vector, unique_database):
@@ -150,13 +140,8 @@ class TestUdfs(ImpalaTestSuite):
except ImpalaBeeswaxException, e:
assert "Failed to get file info" in str(e)
- def test_libs_with_same_filenames(self, vector):
- self.client.execute("create database if not exists same_lib_filename_udf_test "
- "location '%s'" % get_fs_path('/test-warehouse/same_lib_filename_udf_test.db'))
- try:
- self.run_test_case('QueryTest/libs_with_same_filenames', vector)
- finally:
- self.client.execute("drop database if exists same_lib_filename_udf_test cascade")
+ def test_libs_with_same_filenames(self, vector, unique_database):
+ self.run_test_case('QueryTest/libs_with_same_filenames', vector, use_db=unique_database)
def test_udf_update_via_drop(self, vector, unique_database):
"""Test updating the UDF binary without restarting Impala. Dropping
@@ -263,19 +248,19 @@ class TestUdfs(ImpalaTestSuite):
# Run serially because this will blow the process limit, potentially causing other
# queries to fail
@pytest.mark.execute_serially
- def test_mem_limits(self, vector):
+ def test_mem_limits(self, vector, unique_database):
# Set the mem limit high enough that a simple scan can run
mem_limit = 1024 * 1024
vector.get_value('exec_option')['mem_limit'] = mem_limit
try:
- self.run_test_case('QueryTest/udf-mem-limit', vector)
+ self.run_test_case('QueryTest/udf-mem-limit', vector, use_db=unique_database)
assert False, "Query was expected to fail"
except ImpalaBeeswaxException, e:
self.__check_exception(e)
try:
- self.run_test_case('QueryTest/uda-mem-limit', vector)
+ self.run_test_case('QueryTest/uda-mem-limit', vector, use_db=unique_database)
assert False, "Query was expected to fail"
except ImpalaBeeswaxException, e:
self.__check_exception(e)
@@ -330,6 +315,8 @@ returns decimal(9,2) location '{location}' update_fn='SumSmallDecimalUpdate';
create_test_udas_template = """
drop function if exists {database}.trunc_sum(double);
drop function if exists {database}.arg_is_const(int, int);
+drop function if exists {database}.toggle_null(int);
+drop function if exists {database}.count_nulls(bigint);
create database if not exists {database};
@@ -341,6 +328,14 @@ serialize_fn='TruncSumSerialize' finalize_fn='TruncSumFinalize';
create aggregate function {database}.arg_is_const(int, int)
returns boolean location '{location}'
init_fn='ArgIsConstInit' update_fn='ArgIsConstUpdate' merge_fn='ArgIsConstMerge';
+
+create aggregate function {database}.toggle_null(int)
+returns int location '{location}'
+update_fn='ToggleNullUpdate' merge_fn='ToggleNullMerge';
+
+create aggregate function {database}.count_nulls(bigint)
+returns bigint location '{location}'
+update_fn='CountNullsUpdate' merge_fn='CountNullsMerge';
"""
# Create test UDF functions in {database} from library {location}
[3/5] incubator-impala git commit: IMPALA-1430,
IMPALA-4108: codegen all builtin aggregate functions
Posted by ab...@apache.org.
IMPALA-1430,IMPALA-4108: codegen all builtin aggregate functions
This change enables codegen for all builtin aggregate functions,
e.g. timestamp functions and group_concat.
There are several parts to the change:
* Adding support for generic UDAs. Previous the codegen code did not
handle multiple input arguments or NULL return values.
* Defaulting to using the UDA interface when there is not a special
codegen path (we have implementations of all builtin aggregate
functions for the interpreted path).
* Remove all the logic to disable codegen for the special cases that now
are supported.
Also fix the generation of code to get/set NULL bits since I needed
to add functionality there anyway.
Testing:
Add tests that check that codegen was enabled for builtin aggregate
functions. Also fix some gaps in the preexisting tests.
Also add tests for UDAs that check input/output nulls are handled
correctly, in anticipation of enabling codegen for arbitrary UDAs.
The tests are run with both codegen enabled and disabled. To avoid
flaky tests, we switch the UDF tests to use "unique_database".
Perf:
Ran local TPC-H and targeted perf. Spent a lot of time on TPC-H Q1,
since my original approach regressed it ~5%. In the end the problem was
to do with the ordering of loads/stores to the slot and null bit in the
generated code: the previous version of the code exploited some
properties of the particular aggregate function. I ended up replicating
this behaviour to avoid regressing perf.
Change-Id: Id9dc21d1d676505d3617e1e4f37557397c4fb260
Reviewed-on: http://gerrit.cloudera.org:8080/4655
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/d7246d64
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d7246d64
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d7246d64
Branch: refs/heads/master
Commit: d7246d64c777384f29fe0f824ee0036b58e8aa2d
Parents: 6775893
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Sep 28 13:04:07 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Wed Nov 9 03:27:12 2016 +0000
----------------------------------------------------------------------
be/src/benchmarks/hash-benchmark.cc | 2 +-
be/src/codegen/codegen-anyval.cc | 63 +-
be/src/codegen/codegen-anyval.h | 55 +-
be/src/codegen/gen_ir_descriptions.py | 4 +-
be/src/codegen/llvm-codegen-test.cc | 16 +-
be/src/codegen/llvm-codegen.cc | 19 +-
be/src/codegen/llvm-codegen.h | 29 +-
be/src/exec/aggregation-node.cc | 43 +-
be/src/exec/exec-node.cc | 2 +-
be/src/exec/hash-join-node.cc | 12 +-
be/src/exec/hash-table.cc | 42 +-
be/src/exec/hdfs-avro-scanner.cc | 23 +-
be/src/exec/hdfs-scanner.cc | 15 +-
be/src/exec/old-hash-table.cc | 10 +-
be/src/exec/partitioned-aggregation-node-ir.cc | 2 +-
be/src/exec/partitioned-aggregation-node.cc | 611 ++++++++++---------
be/src/exec/partitioned-aggregation-node.h | 25 +-
be/src/exec/partitioned-hash-join-node.cc | 12 +-
be/src/exec/text-converter.cc | 12 +-
be/src/exprs/aggregate-functions-ir.cc | 24 +-
be/src/exprs/case-expr.cc | 2 +-
be/src/exprs/compound-predicates.cc | 7 +-
be/src/exprs/expr-codegen-test.cc | 2 +-
be/src/exprs/expr.cc | 4 +-
be/src/exprs/literal.cc | 4 +-
be/src/exprs/null-literal.cc | 2 +-
be/src/exprs/scalar-fn-call.cc | 12 +-
be/src/exprs/slot-ref.cc | 48 +-
be/src/runtime/descriptors.cc | 132 ++--
be/src/runtime/descriptors.h | 40 +-
be/src/runtime/tuple.cc | 5 +-
be/src/runtime/types.h | 19 +-
be/src/testutil/test-udas.cc | 36 ++
be/src/testutil/test-udfs.cc | 6 +-
be/src/util/tuple-row-compare.cc | 2 +-
.../queries/QueryTest/java-udf.test | 104 ++--
.../QueryTest/libs_with_same_filenames.test | 11 +-
.../queries/QueryTest/load-java-udfs.test | 79 +--
.../queries/QueryTest/uda-mem-limit.test | 3 -
.../functional-query/queries/QueryTest/uda.test | 28 +
.../queries/QueryTest/udf-codegen-required.test | 10 +
.../queries/QueryTest/udf-errors.test | 37 +-
.../queries/QueryTest/udf-mem-limit.test | 5 -
.../functional-query/queries/QueryTest/udf.test | 7 -
tests/common/test_result_verifier.py | 22 +
tests/query_test/test_aggregation.py | 124 +++-
tests/query_test/test_udfs.py | 83 ++-
47 files changed, 1046 insertions(+), 809 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/benchmarks/hash-benchmark.cc
----------------------------------------------------------------------
diff --git a/be/src/benchmarks/hash-benchmark.cc b/be/src/benchmarks/hash-benchmark.cc
index 5ab7f5c..4f6d7f1 100644
--- a/be/src/benchmarks/hash-benchmark.cc
+++ b/be/src/benchmarks/hash-benchmark.cc
@@ -343,7 +343,7 @@ Function* CodegenCrcHash(LlvmCodeGen* codegen, bool mixed) {
prototype.AddArgument(
LlvmCodeGen::NamedVariable("results", codegen->GetPtrType(TYPE_INT)));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[3];
Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/codegen/codegen-anyval.cc
----------------------------------------------------------------------
diff --git a/be/src/codegen/codegen-anyval.cc b/be/src/codegen/codegen-anyval.cc
index a1ca6d9..bd27409 100644
--- a/be/src/codegen/codegen-anyval.cc
+++ b/be/src/codegen/codegen-anyval.cc
@@ -120,8 +120,7 @@ Type* CodegenAnyVal::GetUnloweredPtrType(LlvmCodeGen* cg, const ColumnType& type
return GetUnloweredType(cg, type)->getPointerTo();
}
-Value* CodegenAnyVal::CreateCall(
- LlvmCodeGen* cg, LlvmCodeGen::LlvmBuilder* builder, Function* fn,
+Value* CodegenAnyVal::CreateCall(LlvmCodeGen* cg, LlvmBuilder* builder, Function* fn,
ArrayRef<Value*> args, const char* name, Value* result_ptr) {
if (fn->getReturnType()->isVoidTy()) {
// Void return type indicates that this function returns a DecimalVal via the first
@@ -152,20 +151,15 @@ Value* CodegenAnyVal::CreateCall(
}
}
-CodegenAnyVal CodegenAnyVal::CreateCallWrapped(
- LlvmCodeGen* cg, LlvmCodeGen::LlvmBuilder* builder, const ColumnType& type,
- Function* fn, ArrayRef<Value*> args, const char* name) {
+CodegenAnyVal CodegenAnyVal::CreateCallWrapped(LlvmCodeGen* cg, LlvmBuilder* builder,
+ const ColumnType& type, Function* fn, ArrayRef<Value*> args, const char* name) {
Value* v = CreateCall(cg, builder, fn, args, name);
return CodegenAnyVal(cg, builder, type, v, name);
}
-CodegenAnyVal::CodegenAnyVal(LlvmCodeGen* codegen, LlvmCodeGen::LlvmBuilder* builder,
- const ColumnType& type, Value* value, const char* name)
- : type_(type),
- value_(value),
- name_(name),
- codegen_(codegen),
- builder_(builder) {
+CodegenAnyVal::CodegenAnyVal(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const ColumnType& type, Value* value, const char* name)
+ : type_(type), value_(value), name_(name), codegen_(codegen), builder_(builder) {
Type* value_type = GetLoweredType(codegen, type);
if (value_ == NULL) {
// No Value* was specified, so allocate one on the stack and load it.
@@ -175,7 +169,7 @@ CodegenAnyVal::CodegenAnyVal(LlvmCodeGen* codegen, LlvmCodeGen::LlvmBuilder* bui
DCHECK_EQ(value_->getType(), value_type);
}
-Value* CodegenAnyVal::GetIsNull(const char* name) {
+Value* CodegenAnyVal::GetIsNull(const char* name) const {
switch (type_.type) {
case TYPE_BIGINT:
case TYPE_DOUBLE: {
@@ -448,23 +442,28 @@ void CodegenAnyVal::SetDate(Value* date) {
value_ = builder_->CreateInsertValue(value_, v, 0, name_);
}
-Value* CodegenAnyVal::GetUnloweredPtr() {
- Value* value_ptr = codegen_->CreateEntryBlockAlloca(*builder_, value_->getType());
- builder_->CreateStore(value_, value_ptr);
- return builder_->CreateBitCast(value_ptr, GetUnloweredPtrType(codegen_, type_));
+Value* CodegenAnyVal::GetLoweredPtr(const string& name) const {
+ Value* lowered_ptr =
+ codegen_->CreateEntryBlockAlloca(*builder_, value_->getType(), name.c_str());
+ builder_->CreateStore(GetLoweredValue(), lowered_ptr);
+ return lowered_ptr;
}
-void CodegenAnyVal::SetFromRawPtr(Value* raw_ptr) {
- Value* val_ptr =
- builder_->CreateBitCast(raw_ptr, codegen_->GetPtrType(type_), "val_ptr");
- Value* val = builder_->CreateLoad(val_ptr);
- SetFromRawValue(val);
+Value* CodegenAnyVal::GetUnloweredPtr(const string& name) const {
+ // Get an unlowered pointer by creating a lowered pointer then bitcasting it.
+ // TODO: if the original value was unlowered, this generates roundabout code that
+ // lowers the value and casts it back. Generally LLVM's optimiser can reason
+ // about what's going on and undo our shenanigans to generate sane code, but it
+ // would be nice to just emit reasonable code in the first place.
+ return builder_->CreateBitCast(
+ GetLoweredPtr(), GetUnloweredPtrType(codegen_, type_), name);
}
void CodegenAnyVal::SetFromRawValue(Value* raw_val) {
DCHECK_EQ(raw_val->getType(), codegen_->GetType(type_))
- << endl << LlvmCodeGen::Print(raw_val)
- << endl << type_ << " => " << LlvmCodeGen::Print(codegen_->GetType(type_));
+ << endl
+ << LlvmCodeGen::Print(raw_val) << endl
+ << type_ << " => " << LlvmCodeGen::Print(codegen_->GetType(type_));
switch (type_.type) {
case TYPE_STRING:
case TYPE_VARCHAR: {
@@ -614,9 +613,7 @@ void CodegenAnyVal::WriteToSlot(const SlotDescriptor& slot_desc, Value* tuple,
// Null block: set null bit
builder_->SetInsertPoint(null_block);
- Function* set_null_fn = slot_desc.GetUpdateNullFn(codegen_, true);
- DCHECK(set_null_fn != NULL);
- builder_->CreateCall(set_null_fn, tuple);
+ slot_desc.CodegenSetNullIndicator(codegen_, builder_, tuple, codegen_->true_value());
builder_->CreateBr(insert_before);
// Leave builder_ after conditional blocks
@@ -704,6 +701,14 @@ Value* CodegenAnyVal::Compare(CodegenAnyVal* other, const char* name) {
return builder_->CreateCall(compare_fn, args, name);
}
+void CodegenAnyVal::CodegenBranchIfNull(LlvmBuilder* builder, BasicBlock* null_block) {
+ Value* is_null = GetIsNull();
+ BasicBlock* not_null_block = BasicBlock::Create(
+ codegen_->context(), "not_null", builder->GetInsertBlock()->getParent());
+ builder->CreateCondBr(is_null, null_block, not_null_block);
+ builder->SetInsertPoint(not_null_block);
+}
+
Value* CodegenAnyVal::GetHighBits(int num_bits, Value* v, const char* name) {
DCHECK_EQ(v->getType()->getIntegerBitWidth(), num_bits * 2);
Value* shifted = builder_->CreateAShr(v, num_bits);
@@ -762,8 +767,8 @@ Value* CodegenAnyVal::GetNullVal(LlvmCodeGen* codegen, Type* val_type) {
return ConstantInt::get(val_type, 1);
}
-CodegenAnyVal CodegenAnyVal::GetNonNullVal(LlvmCodeGen* codegen,
- LlvmCodeGen::LlvmBuilder* builder, const ColumnType& type, const char* name) {
+CodegenAnyVal CodegenAnyVal::GetNonNullVal(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const ColumnType& type, const char* name) {
Type* val_type = GetLoweredType(codegen, type);
// All zeros => 'is_null' = false
Value* value = Constant::getNullValue(val_type);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/codegen/codegen-anyval.h
----------------------------------------------------------------------
diff --git a/be/src/codegen/codegen-anyval.h b/be/src/codegen/codegen-anyval.h
index 290ca03..c07f3eb 100644
--- a/be/src/codegen/codegen-anyval.h
+++ b/be/src/codegen/codegen-anyval.h
@@ -51,6 +51,8 @@ namespace impala {
/// TYPE_DOUBLE/DoubleVal: { i8, double }
/// TYPE_STRING/StringVal: { i64, i8* }
/// TYPE_TIMESTAMP/TimestampVal: { i64, i64 }
+/// TYPE_DECIMAL/DecimalVal (isn't lowered):
+/// %"struct.impala_udf::DecimalVal" { {i8}, [15 x i8], {i128} }
//
/// TODO:
/// - unit tests
@@ -78,14 +80,14 @@ class CodegenAnyVal {
/// result will not be a pointer in this case).
//
/// 'name' optionally specifies the name of the returned value.
- static llvm::Value* CreateCall(LlvmCodeGen* cg, LlvmCodeGen::LlvmBuilder* builder,
+ static llvm::Value* CreateCall(LlvmCodeGen* cg, LlvmBuilder* builder,
llvm::Function* fn, llvm::ArrayRef<llvm::Value*> args, const char* name = "",
llvm::Value* result_ptr = NULL);
/// Same as above but wraps the result in a CodegenAnyVal.
- static CodegenAnyVal CreateCallWrapped(LlvmCodeGen* cg,
- LlvmCodeGen::LlvmBuilder* builder, const ColumnType& type, llvm::Function* fn,
- llvm::ArrayRef<llvm::Value*> args, const char* name = "");
+ static CodegenAnyVal CreateCallWrapped(LlvmCodeGen* cg, LlvmBuilder* builder,
+ const ColumnType& type, llvm::Function* fn, llvm::ArrayRef<llvm::Value*> args,
+ const char* name = "");
/// Returns the lowered AnyVal type associated with 'type'.
/// E.g.: TYPE_BOOLEAN (which corresponds to a BooleanVal) => i16
@@ -116,8 +118,8 @@ class CodegenAnyVal {
/// E.g.: TYPE_DOUBLE (lowered DoubleVal: { i8, double }) => { 0, 0 }
/// This returns a CodegenAnyVal, rather than the unwrapped Value*, because the actual
/// value still needs to be set.
- static CodegenAnyVal GetNonNullVal(LlvmCodeGen* codegen,
- LlvmCodeGen::LlvmBuilder* builder, const ColumnType& type, const char* name = "");
+ static CodegenAnyVal GetNonNullVal(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const ColumnType& type, const char* name = "");
/// Creates a wrapper around a lowered *Val value.
//
@@ -132,14 +134,14 @@ class CodegenAnyVal {
/// must be of the correct lowered type.
//
/// If 'name' is specified, it will be used when generated instructions that set value_.
- CodegenAnyVal(LlvmCodeGen* codegen, LlvmCodeGen::LlvmBuilder* builder,
- const ColumnType& type, llvm::Value* value = NULL, const char* name = "");
+ CodegenAnyVal(LlvmCodeGen* codegen, LlvmBuilder* builder, const ColumnType& type,
+ llvm::Value* value = NULL, const char* name = "");
/// Returns the current type-lowered value.
- llvm::Value* value() { return value_; }
+ llvm::Value* GetLoweredValue() const { return value_; }
/// Gets the 'is_null' field of the *Val.
- llvm::Value* GetIsNull(const char* name = "is_null");
+ llvm::Value* GetIsNull(const char* name = "is_null") const;
/// Get the 'val' field of the *Val. Do not call if this represents a StringVal or
/// TimestampVal. If this represents a DecimalVal, returns 'val4', 'val8', or 'val16'
@@ -180,19 +182,18 @@ class CodegenAnyVal {
void SetDate(llvm::Value* date);
void SetTimeOfDay(llvm::Value* time_of_day);
- /// Allocas and stores this value in an unlowered pointer, and returns the pointer. This
- /// *Val should be non-null.
- llvm::Value* GetUnloweredPtr();
+ /// Stores this value in an alloca allocation, and returns the pointer, which has the
+ /// lowered type. This *Val should be non-null. The output variable is called 'name'.
+ llvm::Value* GetLoweredPtr(const std::string& name = "") const;
+
+ /// Stores this value in an alloca allocation, and returns the pointer, which has the
+ /// unlowered type. This *Val should be non-null. The output variable is called 'name'.
+ llvm::Value* GetUnloweredPtr(const std::string& name = "") const;
/// Set this *Val's value based on 'raw_val'. 'raw_val' should be a native type,
/// StringValue, or TimestampValue.
void SetFromRawValue(llvm::Value* raw_val);
- /// Set this *Val's value based on void* 'raw_ptr'. 'raw_ptr' should be a pointer to a
- /// native type, StringValue, or TimestampValue (i.e. the value returned by an
- /// interpreted compute fn).
- void SetFromRawPtr(llvm::Value* raw_ptr);
-
/// Converts this *Val's value to a native type, StringValue, TimestampValue, etc.
/// This should only be used if this *Val is not null.
///
@@ -235,9 +236,23 @@ class CodegenAnyVal {
/// this < 'other', 0 if this == 'other', > 0 if this > 'other'.
llvm::Value* Compare(CodegenAnyVal* other, const char* name = "result");
+ /// Generate code to branch to 'null_block' if this value is NULL. The branch terminates
+ /// the current BasicBlock, so a new BasicBlock for the non-NULL case is also created,
+ /// and builder's insert position is set to the start of the non-NULL block.
+ ///
+ /// This corresponds to the C++ code:
+ /// if (val.is_null) goto null_block;
+ ///
+ /// non_null_block:
+ /// <-- Builder insert position after this function returns.
+ /// ...
+ /// null_block:
+ /// ...
+ void CodegenBranchIfNull(LlvmBuilder* builder, llvm::BasicBlock* null_block);
+
/// Ctor for created an uninitialized CodegenAnYVal that can be assigned to later.
CodegenAnyVal()
- : type_(INVALID_TYPE), value_(NULL), name_(NULL), codegen_(NULL), builder_(NULL) { }
+ : type_(INVALID_TYPE), value_(NULL), name_(NULL), codegen_(NULL), builder_(NULL) {}
private:
ColumnType type_;
@@ -245,7 +260,7 @@ class CodegenAnyVal {
const char* name_;
LlvmCodeGen* codegen_;
- LlvmCodeGen::LlvmBuilder* builder_;
+ LlvmBuilder* builder_;
/// Helper function for getting the top (most significant) half of 'v'.
/// 'v' should have width = 'num_bits' * 2 and be an integer type.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/codegen/gen_ir_descriptions.py
----------------------------------------------------------------------
diff --git a/be/src/codegen/gen_ir_descriptions.py b/be/src/codegen/gen_ir_descriptions.py
index ddcd1db..09f3a20 100755
--- a/be/src/codegen/gen_ir_descriptions.py
+++ b/be/src/codegen/gen_ir_descriptions.py
@@ -59,8 +59,8 @@ ir_functions = [
"_ZN6impala26PartitionedAggregationNode22ProcessBatchNoGroupingEPNS_8RowBatchE"],
["PART_AGG_NODE_PROCESS_BATCH_STREAMING",
"_ZN6impala26PartitionedAggregationNode21ProcessBatchStreamingEbNS_13TPrefetchMode4typeEPNS_8RowBatchES4_PNS_12HashTableCtxEPi"],
- ["PART_AGG_NODE_GET_EXPR_CTX",
- "_ZNK6impala26PartitionedAggregationNode17GetAggExprContextEi"],
+ ["PART_AGG_NODE_GET_EXPR_CTXS",
+ "_ZNK6impala26PartitionedAggregationNode18GetAggExprContextsEi"],
["AVG_UPDATE_BIGINT",
"_ZN6impala18AggregateFunctions9AvgUpdateIN10impala_udf9BigIntValEEEvPNS2_15FunctionContextERKT_PNS2_9StringValE"],
["AVG_UPDATE_DOUBLE",
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/codegen/llvm-codegen-test.cc
----------------------------------------------------------------------
diff --git a/be/src/codegen/llvm-codegen-test.cc b/be/src/codegen/llvm-codegen-test.cc
index e6f2f79..178baa2 100644
--- a/be/src/codegen/llvm-codegen-test.cc
+++ b/be/src/codegen/llvm-codegen-test.cc
@@ -124,7 +124,7 @@ TEST_F(LlvmCodeGenTest, BadIRFile) {
// The random int in there is the address of jitted_counter
Function* CodegenInnerLoop(LlvmCodeGen* codegen, int64_t* jitted_counter, int delta) {
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
LlvmCodeGen::FnPrototype fn_prototype(codegen, "JittedInnerLoop", codegen->void_type());
Function* jitted_loop_call = fn_prototype.GeneratePrototype();
@@ -173,10 +173,10 @@ TEST_F(LlvmCodeGenTest, ReplaceFnCall) {
ASSERT_OK(LlvmCodeGenTest::CreateFromFile(&pool, module_file.c_str(), &codegen));
EXPECT_TRUE(codegen.get() != NULL);
- Function* loop_call = codegen->GetFunction(loop_call_name);
+ Function* loop_call = codegen->GetFunction(loop_call_name, false);
EXPECT_TRUE(loop_call != NULL);
EXPECT_TRUE(loop_call->arg_empty());
- Function* loop = codegen->GetFunction(loop_name);
+ Function* loop = codegen->GetFunction(loop_name, false);
EXPECT_TRUE(loop != NULL);
EXPECT_EQ(loop->arg_size(), 1);
@@ -261,7 +261,7 @@ Function* CodegenStringTest(LlvmCodeGen* codegen) {
LlvmCodeGen::FnPrototype prototype(codegen, "StringTest", codegen->GetType(TYPE_INT));
prototype.AddArgument(LlvmCodeGen::NamedVariable("str", string_val_ptr_type));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* str;
Function* interop_fn = prototype.GeneratePrototype(&builder, &str);
@@ -340,7 +340,7 @@ TEST_F(LlvmCodeGenTest, MemcpyTest) {
prototype.AddArgument(LlvmCodeGen::NamedVariable("src", codegen->ptr_type()));
prototype.AddArgument(LlvmCodeGen::NamedVariable("n", codegen->GetType(TYPE_INT)));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
char src[] = "abcd";
char dst[] = "aaaa";
@@ -396,9 +396,9 @@ TEST_F(LlvmCodeGenTest, HashTest) {
// Create a codegen'd function that hashes all the types and returns the results.
// The tuple/values to hash are baked into the codegen for simplicity.
- LlvmCodeGen::FnPrototype prototype(codegen.get(), "HashTest",
- codegen->GetType(TYPE_INT));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmCodeGen::FnPrototype prototype(
+ codegen.get(), "HashTest", codegen->GetType(TYPE_INT));
+ LlvmBuilder builder(codegen->context());
// Test both byte-size specific hash functions and the generic loop hash function
Function* fn_fixed = prototype.GeneratePrototype(&builder, NULL);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/codegen/llvm-codegen.cc
----------------------------------------------------------------------
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index f629d35..f6e7e97 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -671,7 +671,7 @@ Status LlvmCodeGen::MaterializeFunction(Function *fn) {
return MaterializeFunctionHelper(fn);
}
-Function* LlvmCodeGen::GetFunction(const string& symbol) {
+Function* LlvmCodeGen::GetFunction(const string& symbol, bool clone) {
Function* fn = module_->getFunction(symbol.c_str());
if (fn == NULL) {
LOG(ERROR) << "Unable to locate function " << symbol;
@@ -679,6 +679,7 @@ Function* LlvmCodeGen::GetFunction(const string& symbol) {
}
Status status = MaterializeFunction(fn);
if (UNLIKELY(!status.ok())) return NULL;
+ if (clone) return CloneFunction(fn);
return fn;
}
@@ -1335,15 +1336,15 @@ Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
Value* ptr = builder.CreateBitCast(data, GetPtrType(TYPE_BIGINT));
int i = 0;
while (num_bytes >= 8) {
- Value* index[] = { GetIntConstant(TYPE_INT, i++) };
- Value* d = builder.CreateLoad(builder.CreateGEP(ptr, index));
+ Value* index[] = {GetIntConstant(TYPE_INT, i++)};
+ Value* d = builder.CreateLoad(builder.CreateInBoundsGEP(ptr, index));
result_64 = builder.CreateCall(crc64_fn, ArrayRef<Value*>({result_64, d}));
num_bytes -= 8;
}
result = builder.CreateTrunc(result_64, GetType(TYPE_INT));
- Value* index[] = { GetIntConstant(TYPE_INT, i * 8) };
+ Value* index[] = {GetIntConstant(TYPE_INT, i * 8)};
// Update data to past the 8-byte chunks
- data = builder.CreateGEP(data, index);
+ data = builder.CreateInBoundsGEP(data, index);
}
if (num_bytes >= 4) {
@@ -1351,8 +1352,8 @@ Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
Value* ptr = builder.CreateBitCast(data, GetPtrType(TYPE_INT));
Value* d = builder.CreateLoad(ptr);
result = builder.CreateCall(crc32_fn, ArrayRef<Value*>({result, d}));
- Value* index[] = { GetIntConstant(TYPE_INT, 4) };
- data = builder.CreateGEP(data, index);
+ Value* index[] = {GetIntConstant(TYPE_INT, 4)};
+ data = builder.CreateInBoundsGEP(data, index);
num_bytes -= 4;
}
@@ -1361,8 +1362,8 @@ Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
Value* ptr = builder.CreateBitCast(data, GetPtrType(TYPE_SMALLINT));
Value* d = builder.CreateLoad(ptr);
result = builder.CreateCall(crc16_fn, ArrayRef<Value*>({result, d}));
- Value* index[] = { GetIntConstant(TYPE_INT, 2) };
- data = builder.CreateGEP(data, index);
+ Value* index[] = {GetIntConstant(TYPE_INT, 2)};
+ data = builder.CreateInBoundsGEP(data, index);
num_bytes -= 2;
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/codegen/llvm-codegen.h
----------------------------------------------------------------------
diff --git a/be/src/codegen/llvm-codegen.h b/be/src/codegen/llvm-codegen.h
index 095f939..21f9f71 100644
--- a/be/src/codegen/llvm-codegen.h
+++ b/be/src/codegen/llvm-codegen.h
@@ -71,11 +71,15 @@ namespace llvm {
namespace impala {
-class CodegenAnyVal;
class CodegenSymbolEmitter;
class SubExprElimination;
class TupleDescriptor;
+/// Define builder subclass in case we want to change the template arguments later
+class LlvmBuilder : public llvm::IRBuilder<> {
+ using llvm::IRBuilder<>::IRBuilder;
+};
+
/// LLVM code generator. This is the top level object to generate jitted code.
//
/// LLVM provides a c++ IR builder interface so IR does not need to be written
@@ -157,9 +161,6 @@ class LlvmCodeGen {
/// If false, only output IR for functions which were generated.
std::string GetIR(bool full_module) const;
- /// Typedef builder in case we want to change the template arguments later
- typedef llvm::IRBuilder<> LlvmBuilder;
-
/// Utility struct that wraps a variable name and llvm type.
struct NamedVariable {
std::string name;
@@ -330,18 +331,22 @@ class LlvmCodeGen {
/// recursively. Returns NULL if there is any error.
///
/// If 'clone' is true, a clone of the function will be returned. Clones should be used
- /// iff the caller will modify the returned function. This avoids clobbering the
- /// function in case other users need it, but we don't clone if we can avoid it to
- /// reduce compilation time.
+ /// iff the caller will modify the returned function so that the original unmodified
+ /// function remains available. Avoid cloning if possible to reduce compilation time.
///
/// TODO: Return Status instead.
llvm::Function* GetFunction(IRFunction::Type ir_type, bool clone);
/// Return the function with the symbol name 'symbol' from the module. The returned
- /// function and its callee will be recursively materialized. The returned function
- /// isn't cloned. Returns NULL if there is any error.
+ /// function and its callee will be recursively materialized. Returns NULL if there is
+ /// any error.
+ ///
+ /// If 'clone' is true, a clone of the function will be returned. Clones should be used
+ /// iff the caller will modify the returned function so that the original unmodified
+ /// function remains available. Avoid cloning if possible to reduce compilation time.
+ ///
/// TODO: Return Status instead.
- llvm::Function* GetFunction(const string& symbol);
+ llvm::Function* GetFunction(const string& symbol, bool clone);
/// Returns the hash function with signature:
/// int32_t Hash(int8_t* data, int len, int32_t seed);
@@ -442,8 +447,8 @@ class LlvmCodeGen {
/// Codegens IR to load array[idx] and returns the loaded value. 'array' should be a
/// C-style array (e.g. i32*) or an IR array (e.g. [10 x i32]). This function does not
/// do bounds checking.
- llvm::Value* CodegenArrayAt(LlvmBuilder*, llvm::Value* array, int idx,
- const char* name = "");
+ llvm::Value* CodegenArrayAt(
+ LlvmBuilder*, llvm::Value* array, int idx, const char* name = "");
/// Loads a module at 'file' and links it to the module associated with
/// this LlvmCodeGen object. The module must be on the local filesystem.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/aggregation-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/aggregation-node.cc b/be/src/exec/aggregation-node.cc
index 67928ca..0607aa3 100644
--- a/be/src/exec/aggregation-node.cc
+++ b/be/src/exec/aggregation-node.cc
@@ -550,6 +550,10 @@ llvm::Function* AggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
codegen->GetPtrType(FunctionContextImpl::LLVM_FUNCTIONCONTEXT_NAME);
PointerType* expr_ctx_ptr_type = codegen->GetPtrType(ExprContext::LLVM_CLASS_NAME);
StructType* tuple_struct = intermediate_tuple_desc_->GetLlvmStruct(codegen);
+ if (tuple_struct == NULL) {
+ VLOG_QUERY << "Could not codegen UpdateSlot(): could not generate tuple struct.";
+ return NULL;
+ }
PointerType* tuple_ptr_type = codegen->GetPtrType(tuple_struct);
PointerType* tuple_row_ptr_type = codegen->GetPtrType(TupleRow::LLVM_CLASS_NAME);
@@ -560,7 +564,7 @@ llvm::Function* AggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
prototype.AddArgument(LlvmCodeGen::NamedVariable("agg_tuple", tuple_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[4];
Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
Value* fn_ctx_arg = args[0];
@@ -588,8 +592,8 @@ llvm::Function* AggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
if (slot_desc->is_nullable()) {
// Dst is NULL, just update dst slot to src slot and clear null bit
- Function* clear_null_fn = slot_desc->GetUpdateNullFn(codegen, false);
- builder.CreateCall(clear_null_fn, ArrayRef<Value*>({agg_tuple_arg}));
+ slot_desc->CodegenSetNullIndicator(
+ codegen, &builder, agg_tuple_arg, codegen->false_value());
}
// Update the slot
@@ -630,32 +634,23 @@ llvm::Function* AggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
// Create pointer to src_anyval to pass to HllUpdate() function. We must use the
// unlowered type.
- Value* src_lowered_ptr = codegen->CreateEntryBlockAlloca(
- fn, LlvmCodeGen::NamedVariable("src_lowered_ptr", src.value()->getType()));
- builder.CreateStore(src.value(), src_lowered_ptr);
- Type* unlowered_ptr_type =
- CodegenAnyVal::GetUnloweredType(codegen, input_expr->type())->getPointerTo();
- Value* src_unlowered_ptr =
- builder.CreateBitCast(src_lowered_ptr, unlowered_ptr_type, "src_unlowered_ptr");
+ Value* src_unlowered_ptr = src.GetUnloweredPtr("src_unlowered_ptr");
// Create StringVal* intermediate argument from dst_value
- CodegenAnyVal dst_stringval = CodegenAnyVal::GetNonNullVal(
- codegen, &builder, TYPE_STRING, "dst_stringval");
+ CodegenAnyVal dst_stringval =
+ CodegenAnyVal::GetNonNullVal(codegen, &builder, TYPE_STRING, "dst_stringval");
dst_stringval.SetFromRawValue(dst_value);
// Create pointer to dst_stringval to pass to HllUpdate() function. We must use
// the unlowered type.
- Value* dst_lowered_ptr = codegen->CreateEntryBlockAlloca(
- fn, LlvmCodeGen::NamedVariable("dst_lowered_ptr",
- dst_stringval.value()->getType()));
- builder.CreateStore(dst_stringval.value(), dst_lowered_ptr);
- unlowered_ptr_type =
+ Value* dst_lowered_ptr = dst_stringval.GetLoweredPtr("dst_lowered_ptr");
+ Type* dst_unlowered_ptr_type =
codegen->GetPtrType(CodegenAnyVal::GetUnloweredType(codegen, TYPE_STRING));
- Value* dst_unlowered_ptr =
- builder.CreateBitCast(dst_lowered_ptr, unlowered_ptr_type, "dst_unlowered_ptr");
+ Value* dst_unlowered_ptr = builder.CreateBitCast(
+ dst_lowered_ptr, dst_unlowered_ptr_type, "dst_unlowered_ptr");
// Call 'hll_fn'
- builder.CreateCall(hll_fn,
- ArrayRef<Value*>({fn_ctx_arg, src_unlowered_ptr, dst_unlowered_ptr}));
+ builder.CreateCall(
+ hll_fn, ArrayRef<Value*>({fn_ctx_arg, src_unlowered_ptr, dst_unlowered_ptr}));
// Convert StringVal intermediate 'dst_arg' back to StringValue
Value* anyval_result = builder.CreateLoad(dst_lowered_ptr, "anyval_result");
@@ -765,13 +760,17 @@ Function* AggregationNode::CodegenUpdateTuple(LlvmCodeGen* codegen) {
// ExprContext** expr_ctx, Tuple* tuple, TupleRow* row)
// This signature needs to match the non-codegen'd signature exactly.
StructType* tuple_struct = intermediate_tuple_desc_->GetLlvmStruct(codegen);
+ if (tuple_struct == NULL) {
+ VLOG_QUERY << "Could not codegen UpdateSlot(): could not generate tuple struct.";
+ return NULL;
+ }
PointerType* tuple_ptr = PointerType::get(tuple_struct, 0);
LlvmCodeGen::FnPrototype prototype(codegen, "UpdateTuple", codegen->void_type());
prototype.AddArgument(LlvmCodeGen::NamedVariable("this_ptr", agg_node_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("agg_tuple", agg_tuple_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("tuple_row", tuple_row_ptr_type));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[3];
Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/exec-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/exec-node.cc b/be/src/exec/exec-node.cc
index 510a7d9..255217f 100644
--- a/be/src/exec/exec-node.cc
+++ b/be/src/exec/exec-node.cc
@@ -519,7 +519,7 @@ Status ExecNode::CodegenEvalConjuncts(LlvmCodeGen* codegen,
LlvmCodeGen::NamedVariable("num_ctxs", codegen->GetType(TYPE_INT)));
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[3];
*fn = prototype.GeneratePrototype(&builder, args);
Value* ctxs_arg = args[0];
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/hash-join-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hash-join-node.cc b/be/src/exec/hash-join-node.cc
index f2ac928..7c335be 100644
--- a/be/src/exec/hash-join-node.cc
+++ b/be/src/exec/hash-join-node.cc
@@ -548,7 +548,7 @@ Function* HashJoinNode::CodegenCreateOutputRow(LlvmCodeGen* codegen) {
prototype.AddArgument(LlvmCodeGen::NamedVariable("build_arg", tuple_row_ptr_type));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[4];
Function* fn = prototype.GeneratePrototype(&builder, args);
Value* out_row_arg = builder.CreateBitCast(args[1], tuple_row_working_type, "out");
@@ -560,8 +560,9 @@ Function* HashJoinNode::CodegenCreateOutputRow(LlvmCodeGen* codegen) {
// Copy probe row
codegen->CodegenMemcpy(&builder, out_row_arg, probe_row_arg, probe_tuple_row_size_);
- Value* build_row_idx[] = { codegen->GetIntConstant(TYPE_INT, num_probe_tuples) };
- Value* build_row_dst = builder.CreateGEP(out_row_arg, build_row_idx, "build_dst_ptr");
+ Value* build_row_idx[] = {codegen->GetIntConstant(TYPE_INT, num_probe_tuples)};
+ Value* build_row_dst =
+ builder.CreateInBoundsGEP(out_row_arg, build_row_idx, "build_dst_ptr");
// Copy build row.
BasicBlock* build_not_null_block = BasicBlock::Create(context, "build_not_null", fn);
@@ -578,9 +579,8 @@ Function* HashJoinNode::CodegenCreateOutputRow(LlvmCodeGen* codegen) {
// to work.
builder.SetInsertPoint(build_null_block);
for (int i = 0; i < num_build_tuples; ++i) {
- Value* array_idx[] =
- { codegen->GetIntConstant(TYPE_INT, i + num_probe_tuples) };
- Value* dst = builder.CreateGEP(out_row_arg, array_idx, "dst_tuple_ptr");
+ Value* array_idx[] = {codegen->GetIntConstant(TYPE_INT, i + num_probe_tuples)};
+ Value* dst = builder.CreateInBoundsGEP(out_row_arg, array_idx, "dst_tuple_ptr");
builder.CreateStore(codegen->null_ptr_value(), dst);
}
builder.CreateRetVoid();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/hash-table.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hash-table.cc b/be/src/exec/hash-table.cc
index 74fc534..8d45ec7 100644
--- a/be/src/exec/hash-table.cc
+++ b/be/src/exec/hash-table.cc
@@ -567,8 +567,8 @@ string HashTable::PrintStats() const {
// Helper function to store a value into the results buffer if the expr
// evaluated to NULL. We don't want (NULL, 1) to hash to the same as (0,1) so
// we'll pick a more random value.
-static void CodegenAssignNullValue(LlvmCodeGen* codegen,
- LlvmCodeGen::LlvmBuilder* builder, Value* dst, const ColumnType& type) {
+static void CodegenAssignNullValue(
+ LlvmCodeGen* codegen, LlvmBuilder* builder, Value* dst, const ColumnType& type) {
uint64_t fnv_seed = HashUtil::FNV_SEED;
if (type.type == TYPE_STRING || type.type == TYPE_VARCHAR) {
@@ -715,11 +715,11 @@ Status HashTableCtx::CodegenEvalRow(LlvmCodeGen* codegen, bool build, Function**
prototype.AddArgument(LlvmCodeGen::NamedVariable("this_ptr", this_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("expr_values", codegen->ptr_type()));
- prototype.AddArgument(LlvmCodeGen::NamedVariable("expr_values_null",
- codegen->ptr_type()));
+ prototype.AddArgument(
+ LlvmCodeGen::NamedVariable("expr_values_null", codegen->ptr_type()));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[4];
*fn = prototype.GeneratePrototype(&builder, args);
Value* this_ptr = args[0];
@@ -740,10 +740,10 @@ Status HashTableCtx::CodegenEvalRow(LlvmCodeGen* codegen, bool build, Function**
// vector of exprs
// Convert result buffer to llvm ptr type
int offset = expr_values_cache_.expr_values_offsets(i);
- Value* loc = builder.CreateGEP(
+ Value* loc = builder.CreateInBoundsGEP(
NULL, expr_values, codegen->GetIntConstant(TYPE_INT, offset), "loc_addr");
- Value* llvm_loc = builder.CreatePointerCast(loc,
- codegen->GetPtrType(ctxs[i]->root()->type()), "loc");
+ Value* llvm_loc = builder.CreatePointerCast(
+ loc, codegen->GetPtrType(ctxs[i]->root()->type()), "loc");
BasicBlock* null_block = BasicBlock::Create(context, "null", *fn);
BasicBlock* not_null_block = BasicBlock::Create(context, "not_null", *fn);
@@ -768,7 +768,7 @@ Status HashTableCtx::CodegenEvalRow(LlvmCodeGen* codegen, bool build, Function**
// Set null-byte result
Value* null_byte = builder.CreateZExt(is_null, codegen->GetType(TYPE_TINYINT));
- Value* llvm_null_byte_loc = builder.CreateGEP(
+ Value* llvm_null_byte_loc = builder.CreateInBoundsGEP(
NULL, expr_values_null, codegen->GetIntConstant(TYPE_INT, i), "null_byte_loc");
builder.CreateStore(null_byte, llvm_null_byte_loc);
builder.CreateCondBr(is_null, null_block, not_null_block);
@@ -863,7 +863,7 @@ Status HashTableCtx::CodegenHashRow(LlvmCodeGen* codegen, bool use_murmur, Funct
LlvmCodeGen::NamedVariable("expr_values_null", codegen->ptr_type()));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[3];
*fn = prototype.GeneratePrototype(&builder, args);
Value* this_arg = args[0];
@@ -910,7 +910,7 @@ Status HashTableCtx::CodegenHashRow(LlvmCodeGen* codegen, bool use_murmur, Funct
Value* str_null_result = NULL;
int offset = expr_values_cache_.expr_values_offsets(i);
- Value* llvm_loc = builder.CreateGEP(
+ Value* llvm_loc = builder.CreateInBoundsGEP(
NULL, expr_values, codegen->GetIntConstant(TYPE_INT, offset), "loc_addr");
// If the hash table stores nulls, we need to check if the stringval
@@ -920,11 +920,11 @@ Status HashTableCtx::CodegenHashRow(LlvmCodeGen* codegen, bool use_murmur, Funct
not_null_block = BasicBlock::Create(context, "not_null", *fn);
continue_block = BasicBlock::Create(context, "continue", *fn);
- Value* llvm_null_byte_loc = builder.CreateGEP(NULL, expr_values_null,
+ Value* llvm_null_byte_loc = builder.CreateInBoundsGEP(NULL, expr_values_null,
codegen->GetIntConstant(TYPE_INT, i), "null_byte_loc");
Value* null_byte = builder.CreateLoad(llvm_null_byte_loc, "null_byte");
- Value* is_null = builder.CreateICmpNE(null_byte,
- codegen->GetIntConstant(TYPE_TINYINT, 0), "is_null");
+ Value* is_null = builder.CreateICmpNE(
+ null_byte, codegen->GetIntConstant(TYPE_TINYINT, 0), "is_null");
builder.CreateCondBr(is_null, null_block, not_null_block);
// For null, we just want to call the hash function on the portion of
@@ -1071,7 +1071,7 @@ Status HashTableCtx::CodegenEquals(LlvmCodeGen* codegen, bool force_null_equalit
LlvmCodeGen::NamedVariable("expr_values_null", codegen->ptr_type()));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[4];
*fn = prototype.GeneratePrototype(&builder, args);
Value* this_ptr = args[0];
@@ -1116,19 +1116,19 @@ Status HashTableCtx::CodegenEquals(LlvmCodeGen* codegen, bool force_null_equalit
// We consider null values equal if we are comparing build rows or if the join
// predicate is <=>
if (force_null_equality || finds_nulls_[i]) {
- Value* llvm_null_byte_loc = builder.CreateGEP(
+ Value* llvm_null_byte_loc = builder.CreateInBoundsGEP(
NULL, expr_values_null, codegen->GetIntConstant(TYPE_INT, i), "null_byte_loc");
Value* null_byte = builder.CreateLoad(llvm_null_byte_loc);
- row_is_null = builder.CreateICmpNE(null_byte,
- codegen->GetIntConstant(TYPE_TINYINT, 0));
+ row_is_null =
+ builder.CreateICmpNE(null_byte, codegen->GetIntConstant(TYPE_TINYINT, 0));
}
// Get llvm value for row_val from 'expr_values'
int offset = expr_values_cache_.expr_values_offsets(i);
- Value* loc = builder.CreateGEP(
+ Value* loc = builder.CreateInBoundsGEP(
NULL, expr_values, codegen->GetIntConstant(TYPE_INT, offset), "loc");
- Value* row_val = builder.CreatePointerCast(loc,
- codegen->GetPtrType(build_expr_ctxs_[i]->root()->type()), "row_val");
+ Value* row_val = builder.CreatePointerCast(
+ loc, codegen->GetPtrType(build_expr_ctxs_[i]->root()->type()), "row_val");
// Branch for GetValue() returning NULL
builder.CreateCondBr(is_null, null_block, not_null_block);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/hdfs-avro-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-avro-scanner.cc b/be/src/exec/hdfs-avro-scanner.cc
index a073cef..c217b65 100644
--- a/be/src/exec/hdfs-avro-scanner.cc
+++ b/be/src/exec/hdfs-avro-scanner.cc
@@ -776,10 +776,10 @@ void HdfsAvroScanner::SetStatusValueOverflow(TErrorCode::type error_code, int64_
// bail_out: ; preds = %read_field11, %end_field3, %read_field2, %end_field,
// ret i1 false ; %read_field, %entry
// }
-Status HdfsAvroScanner::CodegenMaterializeTuple(HdfsScanNodeBase* node,
- LlvmCodeGen* codegen, Function** materialize_tuple_fn) {
+Status HdfsAvroScanner::CodegenMaterializeTuple(
+ HdfsScanNodeBase* node, LlvmCodeGen* codegen, Function** materialize_tuple_fn) {
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Type* this_type = codegen->GetType(HdfsAvroScanner::LLVM_CLASS_NAME);
DCHECK(this_type != NULL);
@@ -853,8 +853,7 @@ Status HdfsAvroScanner::CodegenReadRecord(
}
DCHECK_EQ(record.schema->type, AVRO_RECORD);
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder* builder =
- reinterpret_cast<LlvmCodeGen::LlvmBuilder*>(void_builder);
+ LlvmBuilder* builder = reinterpret_cast<LlvmBuilder*>(void_builder);
// Codegen logic for parsing each field and, if necessary, populating a slot with the
// result.
@@ -911,9 +910,8 @@ Status HdfsAvroScanner::CodegenReadRecord(
// Write null field IR
builder->SetInsertPoint(null_block);
if (slot_idx != HdfsScanNode::SKIP_COLUMN) {
- Function* set_null_fn = slot_desc->GetUpdateNullFn(codegen, true);
- DCHECK(set_null_fn != NULL);
- builder->CreateCall(set_null_fn, ArrayRef<Value*>({tuple_val}));
+ slot_desc->CodegenSetNullIndicator(
+ codegen, builder, tuple_val, codegen->true_value());
}
// LLVM requires all basic blocks to end with a terminating instruction
builder->CreateBr(end_field_block);
@@ -944,11 +942,10 @@ Status HdfsAvroScanner::CodegenReadRecord(
}
Status HdfsAvroScanner::CodegenReadScalar(const AvroSchemaElement& element,
- SlotDescriptor* slot_desc, LlvmCodeGen* codegen, void* void_builder,
- Value* this_val, Value* pool_val, Value* tuple_val, Value* data_val,
- Value* data_end_val, Value** ret_val) {
- LlvmCodeGen::LlvmBuilder* builder =
- reinterpret_cast<LlvmCodeGen::LlvmBuilder*>(void_builder);
+ SlotDescriptor* slot_desc, LlvmCodeGen* codegen, void* void_builder, Value* this_val,
+ Value* pool_val, Value* tuple_val, Value* data_val, Value* data_end_val,
+ Value** ret_val) {
+ LlvmBuilder* builder = reinterpret_cast<LlvmBuilder*>(void_builder);
Function* read_field_fn;
switch (element.schema->type) {
case AVRO_BOOLEAN:
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/hdfs-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scanner.cc b/be/src/exec/hdfs-scanner.cc
index 3885522..390085f 100644
--- a/be/src/exec/hdfs-scanner.cc
+++ b/be/src/exec/hdfs-scanner.cc
@@ -380,7 +380,7 @@ Status HdfsScanner::CodegenWriteCompleteTuple(HdfsScanNodeBase* node,
prototype.AddArgument(LlvmCodeGen::NamedVariable("error_in_row", uint8_ptr_type));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[8];
Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
@@ -411,8 +411,8 @@ Status HdfsScanner::CodegenWriteCompleteTuple(HdfsScanNodeBase* node,
// Put tuple in tuple_row
Value* tuple_row_typed =
builder.CreateBitCast(tuple_row_arg, PointerType::get(tuple_ptr_type, 0));
- Value* tuple_row_idxs[] = { codegen->GetIntConstant(TYPE_INT, node->tuple_idx()) };
- Value* tuple_in_row_addr = builder.CreateGEP(tuple_row_typed, tuple_row_idxs);
+ Value* tuple_row_idxs[] = {codegen->GetIntConstant(TYPE_INT, node->tuple_idx())};
+ Value* tuple_in_row_addr = builder.CreateInBoundsGEP(tuple_row_typed, tuple_row_idxs);
builder.CreateStore(tuple_arg, tuple_in_row_addr);
builder.CreateBr(parse_block);
@@ -447,11 +447,12 @@ Status HdfsScanner::CodegenWriteCompleteTuple(HdfsScanNodeBase* node,
codegen->GetIntConstant(TYPE_INT, 1),
};
Value* error_idxs[] = {
- codegen->GetIntConstant(TYPE_INT, slot_idx),
+ codegen->GetIntConstant(TYPE_INT, slot_idx),
};
- Value* data_ptr = builder.CreateGEP(fields_arg, data_idxs, "data_ptr");
- Value* len_ptr = builder.CreateGEP(fields_arg, len_idxs, "len_ptr");
- Value* error_ptr = builder.CreateGEP(errors_arg, error_idxs, "slot_error_ptr");
+ Value* data_ptr = builder.CreateInBoundsGEP(fields_arg, data_idxs, "data_ptr");
+ Value* len_ptr = builder.CreateInBoundsGEP(fields_arg, len_idxs, "len_ptr");
+ Value* error_ptr =
+ builder.CreateInBoundsGEP(errors_arg, error_idxs, "slot_error_ptr");
Value* data = builder.CreateLoad(data_ptr, "data");
Value* len = builder.CreateLoad(len_ptr, "len");
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/old-hash-table.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/old-hash-table.cc b/be/src/exec/old-hash-table.cc
index f51bc74..44bf7e4 100644
--- a/be/src/exec/old-hash-table.cc
+++ b/be/src/exec/old-hash-table.cc
@@ -182,8 +182,8 @@ int OldHashTable::AddBloomFilters() {
// Helper function to store a value into the results buffer if the expr
// evaluated to NULL. We don't want (NULL, 1) to hash to the same as (0,1) so
// we'll pick a more random value.
-static void CodegenAssignNullValue(LlvmCodeGen* codegen,
- LlvmCodeGen::LlvmBuilder* builder, Value* dst, const ColumnType& type) {
+static void CodegenAssignNullValue(
+ LlvmCodeGen* codegen, LlvmBuilder* builder, Value* dst, const ColumnType& type) {
uint64_t fnv_seed = HashUtil::FNV_SEED;
if (type.type == TYPE_STRING || type.type == TYPE_VARCHAR) {
@@ -288,7 +288,7 @@ Function* OldHashTable::CodegenEvalTupleRow(LlvmCodeGen* codegen, bool build) {
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[2];
Function* fn = prototype.GeneratePrototype(&builder, args);
@@ -429,7 +429,7 @@ Function* OldHashTable::CodegenHashCurrentRow(LlvmCodeGen* codegen) {
prototype.AddArgument(LlvmCodeGen::NamedVariable("this_ptr", this_ptr_type));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* this_arg;
Function* fn = prototype.GeneratePrototype(&builder, &this_arg);
@@ -612,7 +612,7 @@ Function* OldHashTable::CodegenEquals(LlvmCodeGen* codegen) {
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[2];
Function* fn = prototype.GeneratePrototype(&builder, args);
Value* row = args[1];
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/partitioned-aggregation-node-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/partitioned-aggregation-node-ir.cc b/be/src/exec/partitioned-aggregation-node-ir.cc
index 83362e7..b696131 100644
--- a/be/src/exec/partitioned-aggregation-node-ir.cc
+++ b/be/src/exec/partitioned-aggregation-node-ir.cc
@@ -26,7 +26,7 @@
using namespace impala;
-ExprContext* PartitionedAggregationNode::GetAggExprContext(int i) const {
+ExprContext* const* PartitionedAggregationNode::GetAggExprContexts(int i) const {
return agg_expr_ctxs_[i];
}
[2/5] incubator-impala git commit: IMPALA-1430,
IMPALA-4108: codegen all builtin aggregate functions
Posted by ab...@apache.org.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/partitioned-aggregation-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/partitioned-aggregation-node.cc b/be/src/exec/partitioned-aggregation-node.cc
index 17fbb6c..56db26f 100644
--- a/be/src/exec/partitioned-aggregation-node.cc
+++ b/be/src/exec/partitioned-aggregation-node.cc
@@ -17,21 +17,20 @@
#include "exec/partitioned-aggregation-node.h"
-#include <algorithm>
#include <math.h>
+#include <algorithm>
#include <set>
#include <sstream>
-#include <gutil/strings/substitute.h>
-#include <thrift/protocol/TDebugProtocol.h>
#include "codegen/codegen-anyval.h"
#include "codegen/llvm-codegen.h"
#include "exec/hash-table.inline.h"
#include "exprs/agg-fn-evaluator.h"
#include "exprs/anyval-util.h"
-#include "exprs/expr.h"
#include "exprs/expr-context.h"
+#include "exprs/expr.h"
#include "exprs/slot-ref.h"
+#include "gutil/strings/substitute.h"
#include "runtime/buffered-tuple-stream.inline.h"
#include "runtime/descriptors.h"
#include "runtime/mem-pool.h"
@@ -40,8 +39,8 @@
#include "runtime/row-batch.h"
#include "runtime/runtime-state.h"
#include "runtime/string-value.inline.h"
-#include "runtime/tuple.h"
#include "runtime/tuple-row.h"
+#include "runtime/tuple.h"
#include "udf/udf-internal.h"
#include "util/debug-util.h"
#include "util/runtime-profile-counters.h"
@@ -150,21 +149,20 @@ Status PartitionedAggregationNode::Init(const TPlanNode& tnode, RuntimeState* st
Expr::CreateExprTrees(pool_, tnode.agg_node.grouping_exprs, &grouping_expr_ctxs_));
for (int i = 0; i < tnode.agg_node.aggregate_functions.size(); ++i) {
AggFnEvaluator* evaluator;
- RETURN_IF_ERROR(AggFnEvaluator::Create(
- pool_, tnode.agg_node.aggregate_functions[i], &evaluator));
+ RETURN_IF_ERROR(
+ AggFnEvaluator::Create(pool_, tnode.agg_node.aggregate_functions[i], &evaluator));
aggregate_evaluators_.push_back(evaluator);
- ExprContext* agg_expr_ctx;
- if (evaluator->input_expr_ctxs().size() == 1) {
- agg_expr_ctx = evaluator->input_expr_ctxs()[0];
+ ExprContext* const* agg_expr_ctxs;
+ if (evaluator->input_expr_ctxs().size() > 0) {
+ agg_expr_ctxs = evaluator->input_expr_ctxs().data();
} else {
- // CodegenUpdateSlot() can only support aggregate operator with only one ExprContext
- // so it doesn't support operator such as group_concat. There are also aggregate
- // operators with no ExprContext (e.g. count(*)). In cases above, 'agg_expr_ctxs_'
- // will contain NULL for that entry.
+ // Some aggregate functions have no input expressions and therefore no ExprContext
+ // (e.g. count(*)). In those cases, 'agg_expr_ctxs_' will contain NULL for that
+ // entry.
DCHECK(evaluator->agg_op() == AggFnEvaluator::OTHER || evaluator->is_count_star());
- agg_expr_ctx = NULL;
+ agg_expr_ctxs = NULL;
}
- agg_expr_ctxs_.push_back(agg_expr_ctx);
+ agg_expr_ctxs_.push_back(agg_expr_ctxs);
}
return Status::OK();
}
@@ -1059,32 +1057,31 @@ void PartitionedAggregationNode::InitAggSlots(
intermediate_tuple_desc_->slots().begin() + grouping_expr_ctxs_.size();
for (int i = 0; i < aggregate_evaluators_.size(); ++i, ++slot_desc) {
AggFnEvaluator* evaluator = aggregate_evaluators_[i];
+ // To minimize branching on the UpdateTuple path, initialize the result value so that
+ // the Add() UDA function can ignore the NULL bit of its destination value. E.g. for
+ // SUM(), if we initialize the destination value to 0 (with the NULL bit set), we can
+ // just start adding to the destination value (rather than repeatedly checking the
+ // destination NULL bit. The codegen'd version of UpdateSlot() exploits this to
+ // eliminate a branch per value.
+ //
+ // For boolean and numeric types, the default values are false/0, so the nullable
+ // aggregate functions SUM() and AVG() produce the correct result. For MIN()/MAX(),
+ // initialize the value to max/min possible value for the same effect.
evaluator->Init(agg_fn_ctxs[i], intermediate_tuple);
- // Codegen specific path for min/max.
- // To minimize branching on the UpdateTuple path, initialize the result value
- // so that UpdateTuple doesn't have to check if the aggregation
- // dst slot is null.
- // TODO: remove when we don't use the irbuilder for codegen here. This optimization
- // will no longer be necessary when all aggregates are implemented with the UDA
- // interface.
- if ((*slot_desc)->type().type != TYPE_STRING &&
- (*slot_desc)->type().type != TYPE_VARCHAR &&
- (*slot_desc)->type().type != TYPE_TIMESTAMP &&
- (*slot_desc)->type().type != TYPE_CHAR) {
+
+ AggFnEvaluator::AggregationOp agg_op = evaluator->agg_op();
+ if ((agg_op == AggFnEvaluator::MIN || agg_op == AggFnEvaluator::MAX)
+ && !evaluator->intermediate_type().IsStringType()
+ && !evaluator->intermediate_type().IsTimestampType()) {
ExprValue default_value;
void* default_value_ptr = NULL;
- switch (evaluator->agg_op()) {
- case AggFnEvaluator::MIN:
- default_value_ptr = default_value.SetToMax((*slot_desc)->type());
- RawValue::Write(default_value_ptr, intermediate_tuple, *slot_desc, NULL);
- break;
- case AggFnEvaluator::MAX:
- default_value_ptr = default_value.SetToMin((*slot_desc)->type());
- RawValue::Write(default_value_ptr, intermediate_tuple, *slot_desc, NULL);
- break;
- default:
- break;
+ if (evaluator->agg_op() == AggFnEvaluator::MIN) {
+ default_value_ptr = default_value.SetToMax((*slot_desc)->type());
+ } else {
+ DCHECK_EQ(evaluator->agg_op(), AggFnEvaluator::MAX);
+ default_value_ptr = default_value.SetToMin((*slot_desc)->type());
}
+ RawValue::Write(default_value_ptr, intermediate_tuple, *slot_desc, NULL);
}
}
}
@@ -1450,111 +1447,123 @@ Status PartitionedAggregationNode::QueryMaintenance(RuntimeState* state) {
// void UpdateSlot(FunctionContext* agg_fn_ctx, ExprContext* agg_expr_ctx,
// AggTuple* agg_tuple, char** row)
//
-// The IR for sum(double_col) is:
+// The IR for sum(double_col), which is constructed directly with the IRBuilder, is:
//
// define void @UpdateSlot(%"class.impala_udf::FunctionContext"* %agg_fn_ctx,
-// %"class.impala::ExprContext"* %agg_expr_ctx,
-// { i8, [7 x i8], double }* %agg_tuple,
-// %"class.impala::TupleRow"* %row) #34 {
-//
+// %"class.impala::ExprContext"** %agg_expr_ctxs,
+// { i8, [7 x i8], double }* %agg_tuple, %"class.impala::TupleRow"* %row) #34 {
// entry:
-// %src = call { i8, double } @GetSlotRef(%"class.impala::ExprContext"* %agg_expr_ctx,
-// %"class.impala::TupleRow"* %row)
-// %0 = extractvalue { i8, double } %src, 0
-// %is_null = trunc i8 %0 to i1
-// br i1 %is_null, label %ret, label %src_not_null
-//
-// src_not_null: ; preds = %entry
+// %expr_ctx_ptr = getelementptr %"class.impala::ExprContext"*,
+// %"class.impala::ExprContext"** %agg_expr_ctxs, i32 0
+// %expr_ctx = load %"class.impala::ExprContext"*,
+// %"class.impala::ExprContext"** %expr_ctx_ptr
+// %input0 = call { i8, double } @GetSlotRef(%"class.impala::ExprContext"* %expr_ctx,
+// %"class.impala::TupleRow"* %row)
// %dst_slot_ptr = getelementptr inbounds { i8, [7 x i8], double },
// { i8, [7 x i8], double }* %agg_tuple, i32 0, i32 2
-// call void @SetNotNull({ i8, [7 x i8], double }* %agg_tuple)
// %dst_val = load double, double* %dst_slot_ptr
-// %val = extractvalue { i8, double } %src, 1
+// %0 = extractvalue { i8, double } %input0, 0
+// %is_null = trunc i8 %0 to i1
+// br i1 %is_null, label %ret, label %not_null
+//
+// ret: ; preds = %not_null, %entry
+// ret void
+//
+// not_null: ; preds = %entry
+// %val = extractvalue { i8, double } %input0, 1
// %1 = fadd double %dst_val, %val
+// %2 = bitcast { i8, [7 x i8], double }* %agg_tuple to i8*
+// %null_byte_ptr = getelementptr i8, i8* %2, i32 0
+// %null_byte = load i8, i8* %null_byte_ptr
+// %null_bit_cleared = and i8 %null_byte, -2
+// store i8 %null_bit_cleared, i8* %null_byte_ptr
// store double %1, double* %dst_slot_ptr
// br label %ret
-//
-// ret: ; preds = %src_not_null, %entry
-// ret void
// }
//
-// The IR for ndv(double_col) is:
+// The IR for min(timestamp_col), which uses the UDA interface, is:
//
// define void @UpdateSlot(%"class.impala_udf::FunctionContext"* %agg_fn_ctx,
-// %"class.impala::ExprContext"* %agg_expr_ctx,
-// { i8, [7 x i8], %"struct.impala::StringValue" }* %agg_tuple,
-// %"class.impala::TupleRow"* %row) #34 {
+// %"class.impala::ExprContext"** %agg_expr_ctxs,
+// { i8, [7 x i8], %"class.impala::TimestampValue" }* %agg_tuple,
+// %"class.impala::TupleRow"* %row) #34 {
// entry:
-// %dst_lowered_ptr = alloca { i64, i8* }
-// %src_lowered_ptr = alloca { i8, double }
-// %src = call { i8, double } @GetSlotRef(%"class.impala::ExprContext"* %agg_expr_ctx,
-// %"class.impala::TupleRow"* %row)
-// %0 = extractvalue { i8, double } %src, 0
-// %is_null = trunc i8 %0 to i1
-// br i1 %is_null, label %ret, label %src_not_null
-//
-// src_not_null: ; preds = %entry
-// %dst_slot_ptr = getelementptr inbounds { i8, [7 x i8], %"struct.impala::StringValue" },
-// { i8, [7 x i8], %"struct.impala::StringValue" }* %agg_tuple, i32 0, i32 2
-// call void @SetNotNull({ i8, [7 x i8], %"struct.impala::StringValue" }* %agg_tuple)
-// %dst_val =
-// load %"struct.impala::StringValue", %"struct.impala::StringValue"* %dst_slot_ptr
-// store { i8, double } %src, { i8, double }* %src_lowered_ptr
-// %src_unlowered_ptr =
-// bitcast { i8, double }* %src_lowered_ptr to %"struct.impala_udf::DoubleVal"*
-// %ptr = extractvalue %"struct.impala::StringValue" %dst_val, 0
-// %dst = insertvalue { i64, i8* } zeroinitializer, i8* %ptr, 1
-// %len = extractvalue %"struct.impala::StringValue" %dst_val, 1
-// %1 = extractvalue { i64, i8* } %dst, 0
-// %2 = zext i32 %len to i64
-// %3 = shl i64 %2, 32
-// %4 = and i64 %1, 4294967295
-// %5 = or i64 %4, %3
-// %dst1 = insertvalue { i64, i8* } %dst, i64 %5, 0
-// store { i64, i8* } %dst1, { i64, i8* }* %dst_lowered_ptr
-// %dst_unlowered_ptr =
-// bitcast { i64, i8* }* %dst_lowered_ptr to %"struct.impala_udf::StringVal"*
-// call void @HllUpdate(%"class.impala_udf::FunctionContext"* %agg_fn_ctx,
-// %"struct.impala_udf::DoubleVal"* %src_unlowered_ptr,
-// %"struct.impala_udf::StringVal"* %dst_unlowered_ptr)
-// %anyval_result = load { i64, i8* }, { i64, i8* }* %dst_lowered_ptr
-// %6 = extractvalue { i64, i8* } %anyval_result, 0
-// %7 = ashr i64 %6, 32
-// %8 = trunc i64 %7 to i32
-// %9 = insertvalue %"struct.impala::StringValue" zeroinitializer, i32 %8, 1
-// %10 = extractvalue { i64, i8* } %anyval_result, 1
-// %11 = insertvalue %"struct.impala::StringValue" %9, i8* %10, 0
-// store %"struct.impala::StringValue" %11, %"struct.impala::StringValue"* %dst_slot_ptr
+// %dst_lowered_ptr = alloca { i64, i64 }
+// %input_lowered_ptr = alloca { i64, i64 }
+// %expr_ctx_ptr = getelementptr %"class.impala::ExprContext"*,
+// %"class.impala::ExprContext"** %agg_expr_ctxs, i32 0
+// %expr_ctx = load %"class.impala::ExprContext"*,
+// %"class.impala::ExprContext"** %expr_ctx_ptr
+// %input0 = call { i64, i64 } @GetSlotRef(%"class.impala::ExprContext"* %expr_ctx,
+// %"class.impala::TupleRow"* %row)
+// %dst_slot_ptr = getelementptr inbounds { i8, [7 x i8],
+// %"class.impala::TimestampValue" }, { i8, [7 x i8],
+// %"class.impala::TimestampValue" }* %agg_tuple, i32 0, i32 2
+// %dst_val = load %"class.impala::TimestampValue",
+// %"class.impala::TimestampValue"* %dst_slot_ptr
+// %0 = bitcast { i8, [7 x i8], %"class.impala::TimestampValue" }* %agg_tuple to i8*
+// %null_byte_ptr = getelementptr i8, i8* %0, i32 0
+// %null_byte = load i8, i8* %null_byte_ptr
+// %null_mask = and i8 %null_byte, 1
+// %is_null = icmp ne i8 %null_mask, 0
+// %is_null_ext = zext i1 %is_null to i64
+// %1 = or i64 0, %is_null_ext
+// %dst = insertvalue { i64, i64 } zeroinitializer, i64 %1, 0
+// %time_of_day = extractvalue %"class.impala::TimestampValue" %dst_val, 0, 0, 0, 0
+// %dst1 = insertvalue { i64, i64 } %dst, i64 %time_of_day, 1
+// %date = extractvalue %"class.impala::TimestampValue" %dst_val, 1, 0, 0
+// %2 = extractvalue { i64, i64 } %dst1, 0
+// %3 = zext i32 %date to i64
+// %4 = shl i64 %3, 32
+// %5 = and i64 %2, 4294967295
+// %6 = or i64 %5, %4
+// %dst2 = insertvalue { i64, i64 } %dst1, i64 %6, 0
+// store { i64, i64 } %input0, { i64, i64 }* %input_lowered_ptr
+// %input_unlowered_ptr = bitcast { i64, i64 }* %input_lowered_ptr
+// to %"struct.impala_udf::TimestampVal"*
+// store { i64, i64 } %dst2, { i64, i64 }* %dst_lowered_ptr
+// %dst_unlowered_ptr = bitcast { i64, i64 }* %dst_lowered_ptr
+// to %"struct.impala_udf::TimestampVal"*
+// call void
+// @_ZN6impala18AggregateFunctions3MinIN10impala_udf12TimestampValEEEvPNS2_15FunctionContextERKT_PS6_.2(
+// %"class.impala_udf::FunctionContext"* %agg_fn_ctx,
+// %"struct.impala_udf::TimestampVal"* %input_unlowered_ptr,
+// %"struct.impala_udf::TimestampVal"* %dst_unlowered_ptr)
+// %anyval_result = load { i64, i64 }, { i64, i64 }* %dst_lowered_ptr
+// %7 = extractvalue { i64, i64 } %anyval_result, 1
+// %8 = insertvalue %"class.impala::TimestampValue" zeroinitializer, i64 %7, 0, 0, 0, 0
+// %9 = extractvalue { i64, i64 } %anyval_result, 0
+// %10 = ashr i64 %9, 32
+// %11 = trunc i64 %10 to i32
+// %12 = insertvalue %"class.impala::TimestampValue" %8, i32 %11, 1, 0, 0
+// %13 = extractvalue { i64, i64 } %anyval_result, 0
+// %result_is_null = trunc i64 %13 to i1
+// %14 = bitcast { i8, [7 x i8], %"class.impala::TimestampValue" }* %agg_tuple to i8*
+// %null_byte_ptr3 = getelementptr i8, i8* %14, i32 0
+// %null_byte4 = load i8, i8* %null_byte_ptr3
+// %null_bit_cleared = and i8 %null_byte4, -2
+// %15 = sext i1 %result_is_null to i8
+// %null_bit = and i8 %15, 1
+// %null_bit_set = or i8 %null_bit_cleared, %null_bit
+// store i8 %null_bit_set, i8* %null_byte_ptr3
+// store %"class.impala::TimestampValue" %12,
+// %"class.impala::TimestampValue"* %dst_slot_ptr
// br label %ret
//
-// ret: ; preds = %src_not_null, %entry
+// ret: ; preds = %entry
// ret void
// }
+//
Status PartitionedAggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
AggFnEvaluator* evaluator, SlotDescriptor* slot_desc, Function** fn) {
- // TODO: Fix this DCHECK and Init() once CodegenUpdateSlot() can handle AggFnEvaluator
- // with multiple input expressions (e.g. group_concat).
- DCHECK_EQ(evaluator->input_expr_ctxs().size(), 1);
- ExprContext* agg_expr_ctx = evaluator->input_expr_ctxs()[0];
- Expr* agg_expr = agg_expr_ctx->root();
-
- // TODO: implement timestamp
- if (agg_expr->type().type == TYPE_TIMESTAMP &&
- evaluator->agg_op() != AggFnEvaluator::AVG) {
- return Status("PartitionedAggregationNode::CodegenUpdateSlot(): timestamp input type "
- "NYI");
- }
-
- Function* agg_expr_fn;
- RETURN_IF_ERROR(agg_expr->GetCodegendComputeFn(codegen, &agg_expr_fn));
-
PointerType* fn_ctx_type =
codegen->GetPtrType(FunctionContextImpl::LLVM_FUNCTIONCONTEXT_NAME);
- PointerType* expr_ctx_type = codegen->GetPtrType(ExprContext::LLVM_CLASS_NAME);
+ PointerType* expr_ctxs_type =
+ codegen->GetPtrPtrType(codegen->GetType(ExprContext::LLVM_CLASS_NAME));
StructType* tuple_struct = intermediate_tuple_desc_->GetLlvmStruct(codegen);
if (tuple_struct == NULL) {
return Status("PartitionedAggregationNode::CodegenUpdateSlot(): failed to generate "
- "intermediate tuple desc");
+ "intermediate tuple desc");
}
PointerType* tuple_ptr_type = codegen->GetPtrType(tuple_struct);
PointerType* tuple_row_ptr_type = codegen->GetPtrType(TupleRow::LLVM_CLASS_NAME);
@@ -1562,129 +1571,129 @@ Status PartitionedAggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
// Create UpdateSlot prototype
LlvmCodeGen::FnPrototype prototype(codegen, "UpdateSlot", codegen->void_type());
prototype.AddArgument(LlvmCodeGen::NamedVariable("agg_fn_ctx", fn_ctx_type));
- prototype.AddArgument(LlvmCodeGen::NamedVariable("agg_expr_ctx", expr_ctx_type));
+ prototype.AddArgument(LlvmCodeGen::NamedVariable("agg_expr_ctxs", expr_ctxs_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("agg_tuple", tuple_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[4];
*fn = prototype.GeneratePrototype(&builder, &args[0]);
Value* agg_fn_ctx_arg = args[0];
- Value* agg_expr_ctx_arg = args[1];
+ Value* agg_expr_ctxs_arg = args[1];
Value* agg_tuple_arg = args[2];
Value* row_arg = args[3];
- BasicBlock* src_not_null_block =
- BasicBlock::Create(codegen->context(), "src_not_null", *fn);
- BasicBlock* ret_block = BasicBlock::Create(codegen->context(), "ret", *fn);
-
- // Call expr function to get src slot value
- Value* agg_expr_fn_args[] = { agg_expr_ctx_arg, row_arg };
- CodegenAnyVal src = CodegenAnyVal::CreateCallWrapped(
- codegen, &builder, agg_expr->type(), agg_expr_fn, agg_expr_fn_args, "src");
+ DCHECK_GE(evaluator->input_expr_ctxs().size(), 1);
+ vector<CodegenAnyVal> input_vals;
+ for (int i = 0; i < evaluator->input_expr_ctxs().size(); ++i) {
+ ExprContext* agg_expr_ctx = evaluator->input_expr_ctxs()[i];
+ Expr* agg_expr = agg_expr_ctx->root();
+ Function* agg_expr_fn;
+ RETURN_IF_ERROR(agg_expr->GetCodegendComputeFn(codegen, &agg_expr_fn));
+ DCHECK(agg_expr_fn != NULL);
+
+ // Call expr function with the matching expr context to get src slot value.
+ Value* expr_ctx_ptr = builder.CreateInBoundsGEP(
+ agg_expr_ctxs_arg, codegen->GetIntConstant(TYPE_INT, i), "expr_ctx_ptr");
+ Value* expr_ctx = builder.CreateLoad(expr_ctx_ptr, "expr_ctx");
+ string input_name = Substitute("input$0", i);
+ input_vals.push_back(
+ CodegenAnyVal::CreateCallWrapped(codegen, &builder, agg_expr->type(), agg_expr_fn,
+ ArrayRef<Value*>({expr_ctx, row_arg}), input_name.c_str()));
+ }
+
+ AggFnEvaluator::AggregationOp agg_op = evaluator->agg_op();
+ const ColumnType& dst_type = evaluator->intermediate_type();
+ bool dst_is_int_or_float_or_bool = dst_type.IsIntegerType()
+ || dst_type.IsFloatingPointType() || dst_type.IsBooleanType();
+ bool dst_is_numeric_or_bool = dst_is_int_or_float_or_bool || dst_type.IsDecimalType();
- Value* src_is_null = src.GetIsNull();
- builder.CreateCondBr(src_is_null, ret_block, src_not_null_block);
+ BasicBlock* ret_block = BasicBlock::Create(codegen->context(), "ret", *fn);
- // Src slot is not null, update dst_slot
- builder.SetInsertPoint(src_not_null_block);
- Value* dst_ptr = builder.CreateStructGEP(NULL, agg_tuple_arg, slot_desc->llvm_field_idx(),
- "dst_slot_ptr");
+ // Emit the code to compute 'result' and set the NULL indicator if needed. First check
+ // for special cases where we can emit a very simple instruction sequence, then fall
+ // back to the general-purpose approach of calling the cross-compiled builtin UDA.
+ CodegenAnyVal& src = input_vals[0];
+ // 'dst_slot_ptr' points to the slot in the aggregate tuple to update.
+ Value* dst_slot_ptr = builder.CreateStructGEP(
+ NULL, agg_tuple_arg, slot_desc->llvm_field_idx(), "dst_slot_ptr");
Value* result = NULL;
-
- if (slot_desc->is_nullable()) {
- // Dst is NULL, just update dst slot to src slot and clear null bit
- Function* clear_null_fn = slot_desc->GetUpdateNullFn(codegen, false);
- builder.CreateCall(clear_null_fn, ArrayRef<Value*>({agg_tuple_arg}));
- }
-
- // Update the slot
- Value* dst_value = builder.CreateLoad(dst_ptr, "dst_val");
- switch (evaluator->agg_op()) {
- case AggFnEvaluator::COUNT:
- if (evaluator->is_merge()) {
- result = builder.CreateAdd(dst_value, src.GetVal(), "count_sum");
- } else {
- result = builder.CreateAdd(dst_value,
- codegen->GetIntConstant(TYPE_BIGINT, 1), "count_inc");
- }
- break;
- case AggFnEvaluator::MIN: {
- Function* min_fn = codegen->CodegenMinMax(slot_desc->type(), true);
- Value* min_args[] = { dst_value, src.GetVal() };
- result = builder.CreateCall(min_fn, min_args, "min_value");
- break;
+ Value* dst_value = builder.CreateLoad(dst_slot_ptr, "dst_val");
+ if (agg_op == AggFnEvaluator::COUNT) {
+ src.CodegenBranchIfNull(&builder, ret_block);
+ if (evaluator->is_merge()) {
+ result = builder.CreateAdd(dst_value, src.GetVal(), "count_sum");
+ } else {
+ result = builder.CreateAdd(
+ dst_value, codegen->GetIntConstant(TYPE_BIGINT, 1), "count_inc");
}
- case AggFnEvaluator::MAX: {
- Function* max_fn = codegen->CodegenMinMax(slot_desc->type(), false);
- Value* max_args[] = { dst_value, src.GetVal() };
- result = builder.CreateCall(max_fn, max_args, "max_value");
- break;
+ DCHECK(!slot_desc->is_nullable());
+ } else if ((agg_op == AggFnEvaluator::MIN || agg_op == AggFnEvaluator::MAX)
+ && dst_is_numeric_or_bool) {
+ bool is_min = agg_op == AggFnEvaluator::MIN;
+ src.CodegenBranchIfNull(&builder, ret_block);
+ Function* min_max_fn = codegen->CodegenMinMax(slot_desc->type(), is_min);
+ Value* min_max_args[] = {dst_value, src.GetVal()};
+ result =
+ builder.CreateCall(min_max_fn, min_max_args, is_min ? "min_value" : "max_value");
+ // Dst may have been NULL, make sure to unset the NULL bit.
+ DCHECK(slot_desc->is_nullable());
+ slot_desc->CodegenSetNullIndicator(
+ codegen, &builder, agg_tuple_arg, codegen->false_value());
+ } else if (agg_op == AggFnEvaluator::SUM && dst_is_int_or_float_or_bool) {
+ src.CodegenBranchIfNull(&builder, ret_block);
+ if (dst_type.IsFloatingPointType()) {
+ result = builder.CreateFAdd(dst_value, src.GetVal());
+ } else {
+ result = builder.CreateAdd(dst_value, src.GetVal());
}
- case AggFnEvaluator::SUM:
- if (slot_desc->type().type != TYPE_DECIMAL) {
- if (slot_desc->type().type == TYPE_FLOAT ||
- slot_desc->type().type == TYPE_DOUBLE) {
- result = builder.CreateFAdd(dst_value, src.GetVal());
- } else {
- result = builder.CreateAdd(dst_value, src.GetVal());
- }
- break;
+ // Dst may have been NULL, make sure to unset the NULL bit.
+ DCHECK(slot_desc->is_nullable());
+ slot_desc->CodegenSetNullIndicator(
+ codegen, &builder, agg_tuple_arg, codegen->false_value());
+ } else {
+ // The remaining cases are implemented using the UDA interface.
+ // Create intermediate argument 'dst' from 'dst_value'
+ CodegenAnyVal dst = CodegenAnyVal::GetNonNullVal(codegen, &builder, dst_type, "dst");
+
+ // For a subset of builtins we generate a different code sequence that exploits two
+ // properties of the builtins. First, NULL input values can be skipped. Second, the
+ // value of the slot was initialized in the right way in InitAggSlots() (e.g. 0 for
+ // SUM) that we get the right result if UpdateSlot() pretends that the NULL bit of
+ // 'dst' is unset. Empirically this optimisation makes TPC-H Q1 5-10% faster.
+ bool special_null_handling = !evaluator->intermediate_type().IsStringType()
+ && !evaluator->intermediate_type().IsTimestampType()
+ && (agg_op == AggFnEvaluator::MIN || agg_op == AggFnEvaluator::MAX
+ || agg_op == AggFnEvaluator::SUM || agg_op == AggFnEvaluator::AVG
+ || agg_op == AggFnEvaluator::NDV);
+ if (slot_desc->is_nullable()) {
+ if (special_null_handling) {
+ src.CodegenBranchIfNull(&builder, ret_block);
+ slot_desc->CodegenSetNullIndicator(
+ codegen, &builder, agg_tuple_arg, codegen->false_value());
+ } else {
+ dst.SetIsNull(slot_desc->CodegenIsNull(codegen, &builder, agg_tuple_arg));
}
- DCHECK_EQ(slot_desc->type().type, TYPE_DECIMAL);
- // Fall through to xcompiled case
- case AggFnEvaluator::AVG:
- case AggFnEvaluator::NDV: {
- // Get xcompiled update/merge function from IR module
- const string& symbol = evaluator->is_merge() ?
- evaluator->merge_symbol() : evaluator->update_symbol();
- const ColumnType& dst_type = evaluator->intermediate_type();
- Function* ir_fn = codegen->GetFunction(symbol);
- DCHECK(ir_fn != NULL);
-
- // Clone and replace constants.
- ir_fn = codegen->CloneFunction(ir_fn);
- vector<FunctionContext::TypeDesc> arg_types;
- arg_types.push_back(AnyValUtil::ColumnTypeToTypeDesc(agg_expr->type()));
- Expr::InlineConstants(AnyValUtil::ColumnTypeToTypeDesc(dst_type), arg_types,
- codegen, ir_fn);
-
- // Create pointer to src to pass to ir_fn. We must use the unlowered type.
- Value* src_lowered_ptr = codegen->CreateEntryBlockAlloca(
- *fn, LlvmCodeGen::NamedVariable("src_lowered_ptr", src.value()->getType()));
- builder.CreateStore(src.value(), src_lowered_ptr);
- Type* unlowered_ptr_type =
- CodegenAnyVal::GetUnloweredPtrType(codegen, agg_expr->type());
- Value* src_unlowered_ptr =
- builder.CreateBitCast(src_lowered_ptr, unlowered_ptr_type, "src_unlowered_ptr");
-
- // Create intermediate argument 'dst' from 'dst_value'
- CodegenAnyVal dst = CodegenAnyVal::GetNonNullVal(
- codegen, &builder, dst_type, "dst");
- dst.SetFromRawValue(dst_value);
- // Create pointer to dst to pass to ir_fn. We must use the unlowered type.
- Value* dst_lowered_ptr = codegen->CreateEntryBlockAlloca(
- *fn, LlvmCodeGen::NamedVariable("dst_lowered_ptr", dst.value()->getType()));
- builder.CreateStore(dst.value(), dst_lowered_ptr);
- unlowered_ptr_type = CodegenAnyVal::GetUnloweredPtrType(codegen, dst_type);
- Value* dst_unlowered_ptr =
- builder.CreateBitCast(dst_lowered_ptr, unlowered_ptr_type, "dst_unlowered_ptr");
-
- // Call 'ir_fn'
- builder.CreateCall(ir_fn,
- ArrayRef<Value*>({agg_fn_ctx_arg, src_unlowered_ptr, dst_unlowered_ptr}));
-
- // Convert StringVal intermediate 'dst_arg' back to StringValue
- Value* anyval_result = builder.CreateLoad(dst_lowered_ptr, "anyval_result");
- result = CodegenAnyVal(codegen, &builder, dst_type, anyval_result).ToNativeValue();
- break;
}
- default:
- DCHECK(false) << "bad aggregate operator: " << evaluator->agg_op();
+ dst.SetFromRawValue(dst_value);
+
+ // Call the UDA to update/merge 'src' into 'dst', with the result stored in
+ // 'updated_dst_val'.
+ CodegenAnyVal updated_dst_val;
+ RETURN_IF_ERROR(CodegenCallUda(
+ codegen, &builder, evaluator, agg_fn_ctx_arg, input_vals, dst, &updated_dst_val));
+ result = updated_dst_val.ToNativeValue();
+
+ if (slot_desc->is_nullable() && !special_null_handling) {
+ // Set NULL bit in the slot based on the return value.
+ Value* result_is_null = updated_dst_val.GetIsNull("result_is_null");
+ slot_desc->CodegenSetNullIndicator(
+ codegen, &builder, agg_tuple_arg, result_is_null);
+ }
}
// TODO: Store to register in the loop and store once to memory at the end of the loop.
- builder.CreateStore(result, dst_ptr);
+ builder.CreateStore(result, dst_slot_ptr);
builder.CreateBr(ret_block);
builder.SetInsertPoint(ret_block);
@@ -1698,6 +1707,58 @@ Status PartitionedAggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
return Status::OK();
}
+Status PartitionedAggregationNode::CodegenCallUda(LlvmCodeGen* codegen,
+ LlvmBuilder* builder, AggFnEvaluator* evaluator, Value* agg_fn_ctx,
+ const vector<CodegenAnyVal>& input_vals, const CodegenAnyVal& dst,
+ CodegenAnyVal* updated_dst_val) {
+ DCHECK_EQ(evaluator->input_expr_ctxs().size(), input_vals.size());
+ const string& symbol =
+ evaluator->is_merge() ? evaluator->merge_symbol() : evaluator->update_symbol();
+ const ColumnType& dst_type = evaluator->intermediate_type();
+
+ // TODO: to support actual UDAs, not just builtin functions using the UDA interface,
+ // we need to load the function at this point.
+ Function* uda_fn = codegen->GetFunction(symbol, true);
+ DCHECK(uda_fn != NULL);
+
+ vector<FunctionContext::TypeDesc> arg_types;
+ for (int i = 0; i < evaluator->input_expr_ctxs().size(); ++i) {
+ arg_types.push_back(AnyValUtil::ColumnTypeToTypeDesc(
+ evaluator->input_expr_ctxs()[i]->root()->type()));
+ }
+ Expr::InlineConstants(
+ AnyValUtil::ColumnTypeToTypeDesc(dst_type), arg_types, codegen, uda_fn);
+
+ // Set up arguments for call to UDA, which are the FunctionContext*, followed by
+ // pointers to all input values, followed by a pointer to the destination value.
+ vector<Value*> uda_fn_args;
+ uda_fn_args.push_back(agg_fn_ctx);
+
+ // Create pointers to input args to pass to uda_fn. We must use the unlowered type,
+ // e.g. IntVal, because the UDA interface expects the values to be passed as const
+ // references to the classes.
+ for (int i = 0; i < evaluator->input_expr_ctxs().size(); ++i) {
+ uda_fn_args.push_back(input_vals[i].GetUnloweredPtr("input_unlowered_ptr"));
+ }
+
+ // Create pointer to dst to pass to uda_fn. We must use the unlowered type for the
+ // same reason as above.
+ Value* dst_lowered_ptr = dst.GetLoweredPtr("dst_lowered_ptr");
+ Type* dst_unlowered_ptr_type = CodegenAnyVal::GetUnloweredPtrType(codegen, dst_type);
+ Value* dst_unlowered_ptr = builder->CreateBitCast(
+ dst_lowered_ptr, dst_unlowered_ptr_type, "dst_unlowered_ptr");
+ uda_fn_args.push_back(dst_unlowered_ptr);
+
+ // Call 'uda_fn'
+ builder->CreateCall(uda_fn, uda_fn_args);
+
+ // Convert intermediate 'dst_arg' back to the native type.
+ Value* anyval_result = builder->CreateLoad(dst_lowered_ptr, "anyval_result");
+
+ *updated_dst_val = CodegenAnyVal(codegen, builder, dst_type, anyval_result);
+ return Status::OK();
+}
+
// IR codegen for the UpdateTuple loop. This loop is query specific and based on the
// aggregate functions. The function signature must match the non- codegen'd UpdateTuple
// exactly.
@@ -1706,78 +1767,60 @@ Status PartitionedAggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
//
// ; Function Attrs: alwaysinline
// define void @UpdateTuple(%"class.impala::PartitionedAggregationNode"* %this_ptr,
-// %"class.impala_udf::FunctionContext"** %agg_fn_ctxs,
-// %"class.impala::Tuple"* %tuple,
-// %"class.impala::TupleRow"* %row,
-// i1 %is_merge) #34 {
+// %"class.impala_udf::FunctionContext"** %agg_fn_ctxs, %"class.impala::Tuple"*
+// %tuple,
+// %"class.impala::TupleRow"* %row, i1 %is_merge) #34 {
// entry:
// %tuple1 =
-// bitcast %"class.impala::Tuple"* %tuple to { i8, [7 x i8], i64, i64, double }*
+// bitcast %"class.impala::Tuple"* %tuple to { i8, [7 x i8], i64, i64, double }*
// %src_slot = getelementptr inbounds { i8, [7 x i8], i64, i64, double },
-// { i8, [7 x i8], i64, i64, double }* %tuple1, i32 0, i32 2
+// { i8, [7 x i8], i64, i64, double }* %tuple1, i32 0, i32 2
// %count_star_val = load i64, i64* %src_slot
// %count_star_inc = add i64 %count_star_val, 1
// store i64 %count_star_inc, i64* %src_slot
// %0 = getelementptr %"class.impala_udf::FunctionContext"*,
-// %"class.impala_udf::FunctionContext"** %agg_fn_ctxs, i32 1
+// %"class.impala_udf::FunctionContext"** %agg_fn_ctxs, i32 1
// %agg_fn_ctx = load %"class.impala_udf::FunctionContext"*,
-// %"class.impala_udf::FunctionContext"** %0
-// %1 = call %"class.impala::ExprContext"*
-// @_ZNK6impala26PartitionedAggregationNode17GetAggExprContextEi(
-// %"class.impala::PartitionedAggregationNode"* %this_ptr, i32 1)
+// %"class.impala_udf::FunctionContext"** %0
+// %1 = call %"class.impala::ExprContext"**
+// @_ZNK6impala26PartitionedAggregationNode18GetAggExprContextsEi(
+// %"class.impala::PartitionedAggregationNode"* %this_ptr, i32 1)
// call void @UpdateSlot(%"class.impala_udf::FunctionContext"* %agg_fn_ctx,
-// %"class.impala::ExprContext"* %1,
-// { i8, [7 x i8], i64, i64, double }* %tuple1,
-// %"class.impala::TupleRow"* %row)
+// %"class.impala::ExprContext"** %1, { i8, [7 x i8], i64, i64, double }* %tuple1,
+// %"class.impala::TupleRow"* %row)
// %2 = getelementptr %"class.impala_udf::FunctionContext"*,
-// %"class.impala_udf::FunctionContext"** %agg_fn_ctxs, i32 2
+// %"class.impala_udf::FunctionContext"** %agg_fn_ctxs, i32 2
// %agg_fn_ctx2 = load %"class.impala_udf::FunctionContext"*,
-// %"class.impala_udf::FunctionContext"** %2
-// %3 = call %"class.impala::ExprContext"*
-// @_ZNK6impala26PartitionedAggregationNode17GetAggExprContextEi(
-// %"class.impala::PartitionedAggregationNode"* %this_ptr, i32 2)
-// call void @UpdateSlot.3(%"class.impala_udf::FunctionContext"* %agg_fn_ctx2,
-// %"class.impala::ExprContext"* %3,
-// { i8, [7 x i8], i64, i64, double }* %tuple1,
-// %"class.impala::TupleRow"* %row)
+// %"class.impala_udf::FunctionContext"** %2
+// %3 = call %"class.impala::ExprContext"**
+// @_ZNK6impala26PartitionedAggregationNode18GetAggExprContextsEi(
+// %"class.impala::PartitionedAggregationNode"* %this_ptr, i32 2)
+// call void @UpdateSlot.4(%"class.impala_udf::FunctionContext"* %agg_fn_ctx2,
+// %"class.impala::ExprContext"** %3, { i8, [7 x i8], i64, i64, double }* %tuple1,
+// %"class.impala::TupleRow"* %row)
// ret void
// }
-Status PartitionedAggregationNode::CodegenUpdateTuple(LlvmCodeGen* codegen,
- Function** fn) {
+Status PartitionedAggregationNode::CodegenUpdateTuple(
+ LlvmCodeGen* codegen, Function** fn) {
SCOPED_TIMER(codegen->codegen_timer());
- int j = grouping_expr_ctxs_.size();
- for (int i = 0; i < aggregate_evaluators_.size(); ++i, ++j) {
- SlotDescriptor* slot_desc = intermediate_tuple_desc_->slots()[j];
- AggFnEvaluator* evaluator = aggregate_evaluators_[i];
-
- // Don't codegen things that aren't builtins (for now)
- if (!evaluator->is_builtin()) {
- return Status("PartitionedAggregationNode::CodegenUpdateTuple(): UDA codegen NYI");
- }
-
- bool supported = true;
- AggFnEvaluator::AggregationOp op = evaluator->agg_op();
- PrimitiveType type = slot_desc->type().type;
- // Char and timestamp intermediates aren't supported
- if (type == TYPE_TIMESTAMP || type == TYPE_CHAR) supported = false;
- // Only AVG and NDV support string intermediates
- if ((type == TYPE_STRING || type == TYPE_VARCHAR) &&
- !(op == AggFnEvaluator::AVG || op == AggFnEvaluator::NDV)) {
- supported = false;
- }
- if (!supported) {
- stringstream ss;
- ss << "Could not codegen PartitionedAggregationNode::UpdateTuple because "
- << "intermediate type " << slot_desc->type() << " is not yet supported for "
- << "aggregate function \"" << evaluator->fn_name() << "()\"";
- return Status(ss.str());
+ for (const SlotDescriptor* slot_desc : intermediate_tuple_desc_->slots()) {
+ if (slot_desc->type().type == TYPE_CHAR) {
+ return Status("PartitionedAggregationNode::CodegenUpdateTuple(): cannot codegen"
+ "CHAR in aggregations");
}
}
if (intermediate_tuple_desc_->GetLlvmStruct(codegen) == NULL) {
return Status("PartitionedAggregationNode::CodegenUpdateTuple(): failed to generate "
- "intermediate tuple desc");
+ "intermediate tuple desc");
+ }
+
+ for (AggFnEvaluator* evaluator : aggregate_evaluators_) {
+ // Don't codegen things that aren't builtins (for now)
+ if (!evaluator->is_builtin()) {
+ return Status("PartitionedAggregationNode::CodegenUpdateTuple(): UDA codegen NYI");
+ }
}
// Get the types to match the UpdateTuple signature
@@ -1800,7 +1843,7 @@ Status PartitionedAggregationNode::CodegenUpdateTuple(LlvmCodeGen* codegen,
prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("is_merge", codegen->boolean_type()));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[5];
*fn = prototype.GeneratePrototype(&builder, &args[0]);
Value* this_arg = args[0];
@@ -1812,13 +1855,13 @@ Status PartitionedAggregationNode::CodegenUpdateTuple(LlvmCodeGen* codegen,
// TODO: get rid of this by using right type in function signature
tuple_arg = builder.CreateBitCast(tuple_arg, tuple_ptr, "tuple");
- Function* get_expr_ctx_fn =
- codegen->GetFunction(IRFunction::PART_AGG_NODE_GET_EXPR_CTX, false);
- DCHECK(get_expr_ctx_fn != NULL);
+ Function* get_expr_ctxs_fn =
+ codegen->GetFunction(IRFunction::PART_AGG_NODE_GET_EXPR_CTXS, false);
+ DCHECK(get_expr_ctxs_fn != NULL);
// Loop over each expr and generate the IR for that slot. If the expr is not
// count(*), generate a helper IR function to update the slot and call that.
- j = grouping_expr_ctxs_.size();
+ int j = grouping_expr_ctxs_.size();
for (int i = 0; i < aggregate_evaluators_.size(); ++i, ++j) {
SlotDescriptor* slot_desc = intermediate_tuple_desc_->slots()[j];
AggFnEvaluator* evaluator = aggregate_evaluators_[i];
@@ -1838,9 +1881,9 @@ Status PartitionedAggregationNode::CodegenUpdateTuple(LlvmCodeGen* codegen,
Value* agg_fn_ctx = builder.CreateLoad(agg_fn_ctx_ptr, "agg_fn_ctx");
// Call GetExprCtx() to get the expression context.
DCHECK(agg_expr_ctxs_[i] != NULL);
- Value* get_expr_ctx_args[] = { this_arg, codegen->GetIntConstant(TYPE_INT, i) };
- Value* agg_expr_ctx = builder.CreateCall(get_expr_ctx_fn, get_expr_ctx_args);
- Value* update_slot_args[] = { agg_fn_ctx, agg_expr_ctx, tuple_arg, row_arg };
+ Value* get_expr_ctxs_args[] = {this_arg, codegen->GetIntConstant(TYPE_INT, i)};
+ Value* agg_expr_ctxs = builder.CreateCall(get_expr_ctxs_fn, get_expr_ctxs_args);
+ Value* update_slot_args[] = {agg_fn_ctx, agg_expr_ctxs, tuple_arg, row_arg};
builder.CreateCall(update_slot_fn, update_slot_args);
}
}
@@ -1849,8 +1892,8 @@ Status PartitionedAggregationNode::CodegenUpdateTuple(LlvmCodeGen* codegen,
// CodegenProcessBatch() does the final optimizations.
*fn = codegen->FinalizeFunction(*fn);
if (*fn == NULL) {
- return Status("PartitionedAggregationNode::CodegeUpdateTuple(): codegen'd "
- "UpdateTuple() function failed verification, see log");
+ return Status("PartitionedAggregationNode::CodegenUpdateTuple(): codegen'd "
+ "UpdateTuple() function failed verification, see log");
}
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/partitioned-aggregation-node.h
----------------------------------------------------------------------
diff --git a/be/src/exec/partitioned-aggregation-node.h b/be/src/exec/partitioned-aggregation-node.h
index 9e14e66..4287cea 100644
--- a/be/src/exec/partitioned-aggregation-node.h
+++ b/be/src/exec/partitioned-aggregation-node.h
@@ -30,13 +30,17 @@
#include "runtime/string-value.h"
namespace llvm {
- class Function;
+class BasicBlock;
+class Function;
+class Value;
}
namespace impala {
class AggFnEvaluator;
+class CodegenAnyVal;
class LlvmCodeGen;
+class LlvmBuilder;
class RowBatch;
class RuntimeState;
struct StringValue;
@@ -203,7 +207,7 @@ class PartitionedAggregationNode : public ExecNode {
/// version of UpdateTuple() to avoid loading aggregate_evaluators_[i] at runtime.
/// An entry is NULL if the aggregate evaluator is not codegen'ed or there is no Expr
/// in the aggregate evaluator (e.g. count(*)).
- std::vector<ExprContext*> agg_expr_ctxs_;
+ std::vector<ExprContext* const*> agg_expr_ctxs_;
/// FunctionContext for each aggregate function and backing MemPool. String data
/// returned by the aggregate functions is allocated via these contexts.
@@ -520,12 +524,13 @@ class PartitionedAggregationNode : public ExecNode {
/// This function processes each individual row in ProcessBatch(). Must be inlined into
/// ProcessBatch for codegen to substitute function calls with codegen'd versions.
/// May spill partitions if not enough memory is available.
- template<bool AGGREGATED_ROWS>
+ template <bool AGGREGATED_ROWS>
Status IR_ALWAYS_INLINE ProcessRow(TupleRow* row, HashTableCtx* ht_ctx);
- /// Accessor for the expression context of an AggFnEvaluator. Used only in codegen'ed
+ /// Accessor for the expression contexts of an AggFnEvaluator. Returns an array of
+ /// pointers the the AggFnEvaluator's expression contexts. Used only in codegen'ed
/// version of UpdateTuple().
- ExprContext* IR_ALWAYS_INLINE GetAggExprContext(int i) const;
+ ExprContext* const* IR_ALWAYS_INLINE GetAggExprContexts(int i) const;
/// Create a new intermediate tuple in partition, initialized with row. ht_ctx is
/// the context for the partition's hash table and hash is the precomputed hash of
@@ -640,6 +645,16 @@ class PartitionedAggregationNode : public ExecNode {
Status CodegenUpdateSlot(LlvmCodeGen* codegen, AggFnEvaluator* evaluator,
SlotDescriptor* slot_desc, llvm::Function** fn);
+ /// Codegen a call to a function implementing the UDA interface with input values
+ /// from 'input_vals'. 'dst_val' should contain the previous value of the aggregate
+ /// function, and 'updated_dst_val' is set to the new value after the Update or Merge
+ /// operation is applied. The instruction sequence for the UDA call is inserted at
+ /// the insert position of 'builder'.
+ Status CodegenCallUda(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ AggFnEvaluator* evaluator, llvm::Value* agg_fn_ctx,
+ const std::vector<CodegenAnyVal>& input_vals, const CodegenAnyVal& dst_val,
+ CodegenAnyVal* updated_dst_val);
+
/// Codegen UpdateTuple(). Returns non-OK status if codegen is unsuccessful.
Status CodegenUpdateTuple(LlvmCodeGen* codegen, llvm::Function** fn);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/partitioned-hash-join-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/partitioned-hash-join-node.cc b/be/src/exec/partitioned-hash-join-node.cc
index 116ac02..c3211af 100644
--- a/be/src/exec/partitioned-hash-join-node.cc
+++ b/be/src/exec/partitioned-hash-join-node.cc
@@ -1144,7 +1144,7 @@ Status PartitionedHashJoinNode::CodegenCreateOutputRow(LlvmCodeGen* codegen,
prototype.AddArgument(LlvmCodeGen::NamedVariable("build_arg", tuple_row_ptr_type));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[4];
*fn = prototype.GeneratePrototype(&builder, args);
Value* out_row_arg = builder.CreateBitCast(args[1], tuple_row_working_type, "out");
@@ -1156,8 +1156,9 @@ Status PartitionedHashJoinNode::CodegenCreateOutputRow(LlvmCodeGen* codegen,
// Copy probe row
codegen->CodegenMemcpy(&builder, out_row_arg, probe_row_arg, probe_tuple_row_size_);
- Value* build_row_idx[] = { codegen->GetIntConstant(TYPE_INT, num_probe_tuples) };
- Value* build_row_dst = builder.CreateGEP(out_row_arg, build_row_idx, "build_dst_ptr");
+ Value* build_row_idx[] = {codegen->GetIntConstant(TYPE_INT, num_probe_tuples)};
+ Value* build_row_dst =
+ builder.CreateInBoundsGEP(out_row_arg, build_row_idx, "build_dst_ptr");
// Copy build row.
BasicBlock* build_not_null_block = BasicBlock::Create(context, "build_not_null", *fn);
@@ -1176,9 +1177,8 @@ Status PartitionedHashJoinNode::CodegenCreateOutputRow(LlvmCodeGen* codegen,
// to work.
builder.SetInsertPoint(build_null_block);
for (int i = 0; i < num_build_tuples; ++i) {
- Value* array_idx[] =
- { codegen->GetIntConstant(TYPE_INT, i + num_probe_tuples) };
- Value* dst = builder.CreateGEP(out_row_arg, array_idx, "dst_tuple_ptr");
+ Value* array_idx[] = {codegen->GetIntConstant(TYPE_INT, i + num_probe_tuples)};
+ Value* dst = builder.CreateInBoundsGEP(out_row_arg, array_idx, "dst_tuple_ptr");
builder.CreateStore(codegen->null_ptr_value(), dst);
}
builder.CreateRetVoid();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exec/text-converter.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/text-converter.cc b/be/src/exec/text-converter.cc
index d38d662..cb5d70c 100644
--- a/be/src/exec/text-converter.cc
+++ b/be/src/exec/text-converter.cc
@@ -128,19 +128,13 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
if (tuple_type == NULL) return NULL;
PointerType* tuple_ptr_type = tuple_type->getPointerTo();
- Function* set_null_fn = slot_desc->GetUpdateNullFn(codegen, true);
- if (set_null_fn == NULL) {
- LOG(ERROR) << "Could not codegen WriteSlot because slot update codegen failed.";
- return NULL;
- }
-
LlvmCodeGen::FnPrototype prototype(
codegen, "WriteSlot", codegen->GetType(TYPE_BOOLEAN));
prototype.AddArgument(LlvmCodeGen::NamedVariable("tuple_arg", tuple_ptr_type));
prototype.AddArgument(LlvmCodeGen::NamedVariable("data", codegen->ptr_type()));
prototype.AddArgument(LlvmCodeGen::NamedVariable("len", codegen->GetType(TYPE_INT)));
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
+ LlvmBuilder builder(codegen->context());
Value* args[3];
Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
@@ -267,13 +261,13 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
// Parse failed, set slot to null and return false
builder.SetInsertPoint(parse_failed_block);
- builder.CreateCall(set_null_fn, ArrayRef<Value*>({args[0]}));
+ slot_desc->CodegenSetNullIndicator(codegen, &builder, args[0], codegen->true_value());
builder.CreateRet(codegen->false_value());
}
// Case where data is \N or len == 0 and it is not a string col
builder.SetInsertPoint(set_null_block);
- builder.CreateCall(set_null_fn, ArrayRef<Value*>({args[0]}));
+ slot_desc->CodegenSetNullIndicator(codegen, &builder, args[0], codegen->true_value());
builder.CreateRet(codegen->true_value());
return codegen->FinalizeFunction(fn);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/aggregate-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/aggregate-functions-ir.cc b/be/src/exprs/aggregate-functions-ir.cc
index c07d1db..7ed6386 100644
--- a/be/src/exprs/aggregate-functions-ir.cc
+++ b/be/src/exprs/aggregate-functions-ir.cc
@@ -164,12 +164,10 @@ StringVal ToStringVal(FunctionContext* context, T val) {
stringstream ss;
ss << val;
const string &str = ss.str();
- return StringVal::CopyFrom(context, reinterpret_cast<const uint8_t*>(str.c_str()), str.size());
+ return StringVal::CopyFrom(
+ context, reinterpret_cast<const uint8_t*>(str.c_str()), str.size());
}
-// Delimiter to use if the separator is NULL.
-static const StringVal DEFAULT_STRING_CONCAT_DELIM((uint8_t*)", ", 2);
-
constexpr int AggregateFunctions::HLL_PRECISION;
constexpr int AggregateFunctions::HLL_LEN;
@@ -627,15 +625,21 @@ void AggregateFunctions::Max(FunctionContext*,
// StringConcatUpdate().
typedef int StringConcatHeader;
-void AggregateFunctions::StringConcatUpdate(FunctionContext* ctx,
- const StringVal& src, StringVal* result) {
- StringConcatUpdate(ctx, src, DEFAULT_STRING_CONCAT_DELIM, result);
+// Delimiter to use if the separator is not provided.
+static inline StringVal ALWAYS_INLINE DefaultStringConcatDelim() {
+ return StringVal(reinterpret_cast<uint8_t*>(const_cast<char*>(", ")), 2);
+}
+
+void AggregateFunctions::StringConcatUpdate(
+ FunctionContext* ctx, const StringVal& src, StringVal* result) {
+ StringConcatUpdate(ctx, src, DefaultStringConcatDelim(), result);
}
-void AggregateFunctions::StringConcatUpdate(FunctionContext* ctx,
- const StringVal& src, const StringVal& separator, StringVal* result) {
+void AggregateFunctions::StringConcatUpdate(FunctionContext* ctx, const StringVal& src,
+ const StringVal& separator, StringVal* result) {
if (src.is_null) return;
- const StringVal* sep = separator.is_null ? &DEFAULT_STRING_CONCAT_DELIM : &separator;
+ const StringVal default_delim = DefaultStringConcatDelim();
+ const StringVal* sep = separator.is_null ? &default_delim : &separator;
if (result->is_null) {
// Header of the intermediate state holds the length of the first separator.
const int header_len = sizeof(StringConcatHeader);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/case-expr.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/case-expr.cc b/be/src/exprs/case-expr.cc
index f139d64..3c0db79 100644
--- a/be/src/exprs/case-expr.cc
+++ b/be/src/exprs/case-expr.cc
@@ -196,7 +196,7 @@ Status CaseExpr::GetCodegendComputeFn(LlvmCodeGen* codegen, Function** fn) {
}
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[2];
Function* function = CreateIrFunctionPrototype(codegen, "CaseExpr", &args);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/compound-predicates.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/compound-predicates.cc b/be/src/exprs/compound-predicates.cc
index 531a70c..d367e5a 100644
--- a/be/src/exprs/compound-predicates.cc
+++ b/be/src/exprs/compound-predicates.cc
@@ -131,7 +131,7 @@ Status CompoundPredicate::CodegenComputeFn(
RETURN_IF_ERROR(children()[1]->GetCodegendComputeFn(codegen, &rhs_function));
LLVMContext& context = codegen->context();
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[2];
Function* function = CreateIrFunctionPrototype(codegen, "CompoundPredicate", &args);
@@ -140,8 +140,7 @@ Status CompoundPredicate::CodegenComputeFn(
// Control blocks for aggregating results
BasicBlock* lhs_null_block = BasicBlock::Create(context, "lhs_null", function);
- BasicBlock* lhs_not_null_block =
- BasicBlock::Create(context, "lhs_not_null", function);
+ BasicBlock* lhs_not_null_block = BasicBlock::Create(context, "lhs_not_null", function);
BasicBlock* lhs_null_rhs_not_null_block =
BasicBlock::Create(context, "lhs_null_rhs_not_null", function);
BasicBlock* lhs_not_null_rhs_null_block =
@@ -232,7 +231,7 @@ Status CompoundPredicate::CodegenComputeFn(
CodegenAnyVal ret(codegen, &builder, TYPE_BOOLEAN, NULL, "ret");
ret.SetIsNull(is_null_phi);
ret.SetVal(val_phi);
- builder.CreateRet(ret.value());
+ builder.CreateRet(ret.GetLoweredValue());
*fn = codegen->FinalizeFunction(function);
DCHECK(*fn != NULL);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/expr-codegen-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-codegen-test.cc b/be/src/exprs/expr-codegen-test.cc
index 800a440..6ccbbf4 100644
--- a/be/src/exprs/expr-codegen-test.cc
+++ b/be/src/exprs/expr-codegen-test.cc
@@ -252,7 +252,7 @@ TEST_F(ExprCodegenTest, TestInlineConstants) {
test_udf_file << getenv("IMPALA_HOME") << "/be/build/latest/exprs/expr-codegen-test.ll";
scoped_ptr<LlvmCodeGen> codegen;
ASSERT_OK(LlvmCodeGen::CreateFromFile(&pool, test_udf_file.str(), "test", &codegen));
- Function* fn = codegen->GetFunction(TEST_GET_CONSTANT_SYMBOL);
+ Function* fn = codegen->GetFunction(TEST_GET_CONSTANT_SYMBOL, false);
ASSERT_TRUE(fn != NULL);
// Function verification should fail because we haven't inlined GetConstant() calls
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/expr.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr.cc b/be/src/exprs/expr.cc
index 0141bae..045c030 100644
--- a/be/src/exprs/expr.cc
+++ b/be/src/exprs/expr.cc
@@ -684,10 +684,10 @@ Status Expr::GetCodegendComputeFnWrapper(LlvmCodeGen* codegen, Function** fn) {
ir_compute_fn_ = CreateIrFunctionPrototype(codegen, "CodegenComputeFnWrapper", &args);
BasicBlock* entry_block =
BasicBlock::Create(codegen->context(), "entry", ir_compute_fn_);
- LlvmCodeGen::LlvmBuilder builder(entry_block);
+ LlvmBuilder builder(entry_block);
Value* this_ptr =
codegen->CastPtrToLlvmPtr(codegen->GetPtrType(Expr::LLVM_CLASS_NAME), this);
- Value* compute_fn_args[] = { this_ptr, args[0], args[1] };
+ Value* compute_fn_args[] = {this_ptr, args[0], args[1]};
Value* ret = CodegenAnyVal::CreateCall(
codegen, &builder, static_getval_fn, compute_fn_args, "ret");
builder.CreateRet(ret);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/literal.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/literal.cc b/be/src/exprs/literal.cc
index 03bdd50..5b075f6 100644
--- a/be/src/exprs/literal.cc
+++ b/be/src/exprs/literal.cc
@@ -385,7 +385,7 @@ Status Literal::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
Value* args[2];
*fn = CreateIrFunctionPrototype(codegen, "Literal", &args);
BasicBlock* entry_block = BasicBlock::Create(codegen->context(), "entry", *fn);
- LlvmCodeGen::LlvmBuilder builder(entry_block);
+ LlvmBuilder builder(entry_block);
CodegenAnyVal v = CodegenAnyVal::GetNonNullVal(codegen, &builder, type_);
switch (type_.type) {
@@ -439,7 +439,7 @@ Status Literal::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
return Status(ss.str());
}
- builder.CreateRet(v.value());
+ builder.CreateRet(v.GetLoweredValue());
*fn = codegen->FinalizeFunction(*fn);
ir_compute_fn_ = *fn;
return Status::OK();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/null-literal.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/null-literal.cc b/be/src/exprs/null-literal.cc
index a8f4241..6985c5b 100644
--- a/be/src/exprs/null-literal.cc
+++ b/be/src/exprs/null-literal.cc
@@ -101,7 +101,7 @@ Status NullLiteral::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function**
Value* args[2];
*fn = CreateIrFunctionPrototype(codegen, "NullLiteral", &args);
BasicBlock* entry_block = BasicBlock::Create(codegen->context(), "entry", *fn);
- LlvmCodeGen::LlvmBuilder builder(entry_block);
+ LlvmBuilder builder(entry_block);
Value* v = CodegenAnyVal::GetNullVal(codegen, type());
builder.CreateRet(v);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/scalar-fn-call.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/scalar-fn-call.cc b/be/src/exprs/scalar-fn-call.cc
index 3a7ffb1..5457ced 100644
--- a/be/src/exprs/scalar-fn-call.cc
+++ b/be/src/exprs/scalar-fn-call.cc
@@ -319,7 +319,7 @@ Status ScalarFnCall::GetCodegendComputeFn(LlvmCodeGen* codegen, Function** fn) {
Value* expr_ctx = args[0];
Value* row = args[1];
BasicBlock* block = BasicBlock::Create(codegen->context(), "entry", *fn);
- LlvmCodeGen::LlvmBuilder builder(block);
+ LlvmBuilder builder(block);
// Populate UDF arguments
vector<Value*> udf_args;
@@ -482,7 +482,7 @@ Status ScalarFnCall::GetUdf(LlvmCodeGen* codegen, Function** udf) {
} else if (fn_.binary_type == TFunctionBinaryType::BUILTIN) {
// In this path, we're running a builtin with the UDF interface. The IR is
// in the llvm module.
- *udf = codegen->GetFunction(fn_.scalar_fn.symbol);
+ *udf = codegen->GetFunction(fn_.scalar_fn.symbol, false);
if (*udf == NULL) {
// Builtins symbols should exist unless there is a version mismatch.
stringstream ss;
@@ -502,11 +502,11 @@ Status ScalarFnCall::GetUdf(LlvmCodeGen* codegen, Function** udf) {
} else {
// We're running an IR UDF.
DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::IR);
- *udf = codegen->GetFunction(fn_.scalar_fn.symbol);
+ *udf = codegen->GetFunction(fn_.scalar_fn.symbol, false);
if (*udf == NULL) {
stringstream ss;
- ss << "Unable to locate function " << fn_.scalar_fn.symbol
- << " from LLVM module " << fn_.hdfs_location;
+ ss << "Unable to locate function " << fn_.scalar_fn.symbol << " from LLVM module "
+ << fn_.hdfs_location;
return Status(ss.str());
}
*udf = codegen->FinalizeFunction(*udf);
@@ -527,7 +527,7 @@ Status ScalarFnCall::GetFunction(RuntimeState* state, const string& symbol, void
DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::IR);
LlvmCodeGen* codegen = state->codegen();
DCHECK(codegen != NULL);
- Function* ir_fn = codegen->GetFunction(symbol);
+ Function* ir_fn = codegen->GetFunction(symbol, false);
if (ir_fn == NULL) {
stringstream ss;
ss << "Unable to locate function " << symbol << " from LLVM module "
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/exprs/slot-ref.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/slot-ref.cc b/be/src/exprs/slot-ref.cc
index eeba42f..3a70b6c 100644
--- a/be/src/exprs/slot-ref.cc
+++ b/be/src/exprs/slot-ref.cc
@@ -134,11 +134,11 @@ string SlotRef::DebugString() const {
// br label %check_slot_null
//
// check_slot_null: ; preds = %entry
-// %null_ptr = getelementptr i8* %tuple_ptr, i32 0
+// %null_byte_ptr = getelementptr i8* %tuple_ptr, i32 0
// %null_byte = load i8* %null_ptr
// %null_byte_set = and i8 %null_byte, 2
-// %slot_is_null = icmp ne i8 %null_byte_set, 0
-// br i1 %slot_is_null, label %ret, label %get_slot
+// %is_null = icmp ne i8 %null_byte_set, 0
+// br i1 %is_null, label %ret, label %get_slot
//
// get_slot: ; preds = %check_slot_null
// %slot_addr = getelementptr i8* %tuple_ptr, i32 8
@@ -189,28 +189,24 @@ Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
Value* row_ptr = args[1];
Value* tuple_offset = ConstantInt::get(codegen->int_type(), tuple_idx_);
- Value* null_byte_offset =
- ConstantInt::get(codegen->int_type(), null_indicator_offset_.byte_offset);
Value* slot_offset = ConstantInt::get(codegen->int_type(), slot_offset_);
- Value* null_mask =
- ConstantInt::get(codegen->tinyint_type(), null_indicator_offset_.bit_mask);
Value* zero = ConstantInt::get(codegen->GetType(TYPE_TINYINT), 0);
Value* one = ConstantInt::get(codegen->GetType(TYPE_TINYINT), 1);
BasicBlock* entry_block = BasicBlock::Create(context, "entry", *fn);
+ bool slot_is_nullable = null_indicator_offset_.bit_mask != 0;
BasicBlock* check_slot_null_indicator_block = NULL;
- if (null_indicator_offset_.bit_mask != 0) {
- check_slot_null_indicator_block =
- BasicBlock::Create(context, "check_slot_null", *fn);
+ if (slot_is_nullable) {
+ check_slot_null_indicator_block = BasicBlock::Create(context, "check_slot_null", *fn);
}
BasicBlock* get_slot_block = BasicBlock::Create(context, "get_slot", *fn);
BasicBlock* ret_block = BasicBlock::Create(context, "ret", *fn);
- LlvmCodeGen::LlvmBuilder builder(entry_block);
+ LlvmBuilder builder(entry_block);
// Get the tuple offset addr from the row
Value* cast_row_ptr = builder.CreateBitCast(
row_ptr, PointerType::get(codegen->ptr_type(), 0), "cast_row_ptr");
- Value* tuple_addr = builder.CreateGEP(cast_row_ptr, tuple_offset, "tuple_addr");
+ Value* tuple_addr = builder.CreateInBoundsGEP(cast_row_ptr, tuple_offset, "tuple_addr");
// Load the tuple*
Value* tuple_ptr = builder.CreateLoad(tuple_addr, "tuple_ptr");
@@ -218,32 +214,30 @@ Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
if (tuple_is_nullable_) {
Value* tuple_is_null = builder.CreateIsNull(tuple_ptr, "tuple_is_null");
// Check slot is null only if the null indicator bit is set
- if (null_indicator_offset_.bit_mask == 0) {
- builder.CreateCondBr(tuple_is_null, ret_block, get_slot_block);
- } else {
+ if (slot_is_nullable) {
builder.CreateCondBr(tuple_is_null, ret_block, check_slot_null_indicator_block);
+ } else {
+ builder.CreateCondBr(tuple_is_null, ret_block, get_slot_block);
}
} else {
- if (null_indicator_offset_.bit_mask == 0) {
- builder.CreateBr(get_slot_block);
- } else {
+ if (slot_is_nullable) {
builder.CreateBr(check_slot_null_indicator_block);
+ } else {
+ builder.CreateBr(get_slot_block);
}
}
// Branch for tuple* != NULL. Need to check if null-indicator is set
- if (check_slot_null_indicator_block != NULL) {
+ if (slot_is_nullable) {
builder.SetInsertPoint(check_slot_null_indicator_block);
- Value* null_addr = builder.CreateGEP(tuple_ptr, null_byte_offset, "null_ptr");
- Value* null_val = builder.CreateLoad(null_addr, "null_byte");
- Value* slot_null_mask = builder.CreateAnd(null_val, null_mask, "null_byte_set");
- Value* is_slot_null = builder.CreateICmpNE(slot_null_mask, zero, "slot_is_null");
+ Value* is_slot_null = SlotDescriptor::CodegenIsNull(
+ codegen, &builder, null_indicator_offset_, tuple_ptr);
builder.CreateCondBr(is_slot_null, ret_block, get_slot_block);
}
// Branch for slot != NULL
builder.SetInsertPoint(get_slot_block);
- Value* slot_ptr = builder.CreateGEP(tuple_ptr, slot_offset, "slot_addr");
+ Value* slot_ptr = builder.CreateInBoundsGEP(tuple_ptr, slot_offset, "slot_addr");
Value* val_ptr = builder.CreateBitCast(slot_ptr, codegen->GetPtrType(type_), "val_ptr");
// Depending on the type, load the values we need
Value* val = NULL;
@@ -313,7 +307,7 @@ Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
result.SetIsNull(is_null_phi);
result.SetPtr(ptr_phi);
result.SetLen(len_phi);
- builder.CreateRet(result.value());
+ builder.CreateRet(result.GetLoweredValue());
} else if (type() == TYPE_TIMESTAMP) {
DCHECK(time_of_day != NULL);
DCHECK(date != NULL);
@@ -343,7 +337,7 @@ Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
result.SetIsNull(is_null_phi);
result.SetTimeOfDay(time_of_day_phi);
result.SetDate(date_phi);
- builder.CreateRet(result.value());
+ builder.CreateRet(result.GetLoweredValue());
} else {
DCHECK(val != NULL);
PHINode* val_phi = builder.CreatePHI(val->getType(), 2, "val_phi");
@@ -360,7 +354,7 @@ Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn)
CodegenAnyVal::GetNonNullVal(codegen, &builder, type(), "result");
result.SetIsNull(is_null_phi);
result.SetVal(val_phi);
- builder.CreateRet(result.value());
+ builder.CreateRet(result.GetLoweredValue());
}
*fn = codegen->FinalizeFunction(*fn);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/runtime/descriptors.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/descriptors.cc b/be/src/runtime/descriptors.cc
index 41485f9..f8ec79c 100644
--- a/be/src/runtime/descriptors.cc
+++ b/be/src/runtime/descriptors.cc
@@ -77,9 +77,8 @@ ostream& operator<<(ostream& os, const NullIndicatorOffset& null_indicator) {
return os;
}
-SlotDescriptor::SlotDescriptor(
- const TSlotDescriptor& tdesc, const TupleDescriptor* parent,
- const TupleDescriptor* collection_item_descriptor)
+SlotDescriptor::SlotDescriptor(const TSlotDescriptor& tdesc,
+ const TupleDescriptor* parent, const TupleDescriptor* collection_item_descriptor)
: id_(tdesc.id),
type_(ColumnType::FromThrift(tdesc.slotType)),
parent_(parent),
@@ -89,9 +88,7 @@ SlotDescriptor::SlotDescriptor(
null_indicator_offset_(tdesc.nullIndicatorByte, tdesc.nullIndicatorBit),
slot_idx_(tdesc.slotIdx),
slot_size_(type_.GetSlotSize()),
- llvm_field_idx_(-1),
- set_not_null_fn_(NULL),
- set_null_fn_(NULL) {
+ llvm_field_idx_(-1) {
DCHECK_NE(type_.type, TYPE_STRUCT);
DCHECK(parent_ != NULL) << tdesc.parent;
if (type_.IsCollectionType()) {
@@ -569,60 +566,79 @@ void DescriptorTbl::GetTupleDescs(vector<TupleDescriptor*>* descs) const {
}
}
-// Generate function to set a slot to be null or not-null. The resulting IR
-// for SetNotNull looks like:
-// (in this case the tuple contains only a nullable double)
-// define void @SetNotNull({ i8, double }* %tuple) {
-// entry:
-// %null_byte_ptr = getelementptr inbounds { i8, double }* %tuple, i32 0, i32 0
-// %null_byte = load i8* %null_byte_ptr
-// %0 = and i8 %null_byte, -2
-// store i8 %0, i8* %null_byte_ptr
-// ret void
-// }
-Function* SlotDescriptor::GetUpdateNullFn(LlvmCodeGen* codegen, bool set_null) const {
- if (set_null && set_null_fn_ != NULL) return set_null_fn_;
- if (!set_null && set_not_null_fn_ != NULL) return set_not_null_fn_;
-
- StructType* tuple_type = parent()->GetLlvmStruct(codegen);
- PointerType* tuple_ptr_type = tuple_type->getPointerTo();
- LlvmCodeGen::FnPrototype prototype(codegen, (set_null) ? "SetNull" :"SetNotNull",
- codegen->void_type());
- prototype.AddArgument(LlvmCodeGen::NamedVariable("tuple", tuple_ptr_type));
-
- LlvmCodeGen::LlvmBuilder builder(codegen->context());
- Value* tuple_arg;
- Function* fn = prototype.GeneratePrototype(&builder, &tuple_arg);
-
- Value* tuple_int8_ptr =
- builder.CreateBitCast(tuple_arg, codegen->ptr_type(), "tuple_int8_ptr");
- Value* null_byte_offset =
- ConstantInt::get(codegen->int_type(), null_indicator_offset_.byte_offset);
- Value* null_byte_ptr =
- builder.CreateInBoundsGEP(tuple_int8_ptr, null_byte_offset, "null_byte_ptr");
- Value* null_byte = builder.CreateLoad(null_byte_ptr, "null_byte");
-
+Value* SlotDescriptor::CodegenIsNull(
+ LlvmCodeGen* codegen, LlvmBuilder* builder, Value* tuple) const {
+ return CodegenIsNull(codegen, builder, null_indicator_offset_, tuple);
+}
+
+// Example IR for getting the first null bit:
+// %0 = bitcast { i8, [7 x i8], %"class.impala::TimestampValue" }* %agg_tuple to i8*
+// %null_byte_ptr = getelementptr i8, i8* %0, i32 0
+// %null_byte = load i8, i8* %null_byte_ptr
+// %null_mask = and i8 %null_byte, 1
+// %is_null = icmp ne i8 %null_mask, 0
+Value* SlotDescriptor::CodegenIsNull(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const NullIndicatorOffset& null_indicator_offset, Value* tuple) {
+ Value* null_byte =
+ CodegenGetNullByte(codegen, builder, null_indicator_offset, tuple, NULL);
+ Constant* mask =
+ ConstantInt::get(codegen->tinyint_type(), null_indicator_offset.bit_mask);
+ Value* null_mask = builder->CreateAnd(null_byte, mask, "null_mask");
+ Constant* zero = ConstantInt::get(codegen->tinyint_type(), 0);
+ return builder->CreateICmpNE(null_mask, zero, "is_null");
+}
+
+// Example IR for setting the first null bit to a non-constant 'is_null' value:
+// %14 = bitcast { i8, [7 x i8], %"class.impala::TimestampValue" }* %agg_tuple to i8*
+// %null_byte_ptr3 = getelementptr i8, i8* %14, i32 0
+// %null_byte4 = load i8, i8* %null_byte_ptr3
+// %null_bit_cleared = and i8 %null_byte4, -2
+// %15 = sext i1 %result_is_null to i8
+// %null_bit = and i8 %15, 1
+// %null_bit_set = or i8 %null_bit_cleared, %null_bit
+// store i8 %null_bit_set, i8* %null_byte_ptr3
+void SlotDescriptor::CodegenSetNullIndicator(
+ LlvmCodeGen* codegen, LlvmBuilder* builder, Value* tuple, Value* is_null) const {
+ DCHECK_EQ(is_null->getType(), codegen->boolean_type());
+ Value* null_byte_ptr;
+ Value* null_byte =
+ CodegenGetNullByte(codegen, builder, null_indicator_offset_, tuple, &null_byte_ptr);
+ Constant* mask =
+ ConstantInt::get(codegen->tinyint_type(), null_indicator_offset_.bit_mask);
+ Constant* not_mask =
+ ConstantInt::get(codegen->tinyint_type(), ~null_indicator_offset_.bit_mask);
+
+ ConstantInt* constant_is_null = dyn_cast<ConstantInt>(is_null);
Value* result = NULL;
- if (set_null) {
- Value* null_set = codegen->GetIntConstant(
- TYPE_TINYINT, null_indicator_offset_.bit_mask);
- result = builder.CreateOr(null_byte, null_set);
- } else {
- Value* null_clear_val =
- codegen->GetIntConstant(TYPE_TINYINT, ~null_indicator_offset_.bit_mask);
- result = builder.CreateAnd(null_byte, null_clear_val);
- }
-
- builder.CreateStore(result, null_byte_ptr);
- builder.CreateRetVoid();
-
- fn = codegen->FinalizeFunction(fn);
- if (set_null) {
- set_null_fn_ = fn;
+ if (constant_is_null != NULL) {
+ if (constant_is_null->isOne()) {
+ result = builder->CreateOr(null_byte, mask, "null_bit_set");
+ } else {
+ DCHECK(constant_is_null->isZero());
+ result = builder->CreateAnd(null_byte, not_mask, "null_bit_cleared");
+ }
} else {
- set_not_null_fn_ = fn;
- }
- return fn;
+ // Avoid branching by computing the new byte as:
+ // (null_byte & ~mask) | (-null & mask);
+ Value* byte_with_cleared_bit =
+ builder->CreateAnd(null_byte, not_mask, "null_bit_cleared");
+ Value* sign_extended_null = builder->CreateSExt(is_null, codegen->tinyint_type());
+ Value* bit_only = builder->CreateAnd(sign_extended_null, mask, "null_bit");
+ result = builder->CreateOr(byte_with_cleared_bit, bit_only, "null_bit_set");
+ }
+
+ builder->CreateStore(result, null_byte_ptr);
+}
+
+Value* SlotDescriptor::CodegenGetNullByte(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const NullIndicatorOffset& null_indicator_offset, Value* tuple,
+ Value** null_byte_ptr) {
+ Constant* byte_offset =
+ ConstantInt::get(codegen->int_type(), null_indicator_offset.byte_offset);
+ Value* tuple_bytes = builder->CreateBitCast(tuple, codegen->ptr_type());
+ Value* byte_ptr = builder->CreateInBoundsGEP(tuple_bytes, byte_offset, "null_byte_ptr");
+ if (null_byte_ptr != NULL) *null_byte_ptr = byte_ptr;
+ return builder->CreateLoad(byte_ptr, "null_byte");
}
StructType* TupleDescriptor::GetLlvmStruct(LlvmCodeGen* codegen) const {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/runtime/descriptors.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/descriptors.h b/be/src/runtime/descriptors.h
index 8122653..4a50800 100644
--- a/be/src/runtime/descriptors.h
+++ b/be/src/runtime/descriptors.h
@@ -35,19 +35,21 @@ namespace llvm {
class Function;
class PointerType;
class StructType;
+ class Value;
};
namespace impala {
+class Expr;
+class ExprContext;
+class LlvmBuilder;
class LlvmCodeGen;
class ObjectPool;
+class RuntimeState;
class TDescriptorTable;
class TSlotDescriptor;
class TTable;
class TTupleDescriptor;
-class Expr;
-class ExprContext;
-class RuntimeState;
/// A path into a table schema (e.g. a vector of ColumnTypes) pointing to a particular
/// column/field. The i-th element of the path is the ordinal position of the column/field
@@ -133,9 +135,22 @@ class SlotDescriptor {
std::string DebugString() const;
- /// Codegen for: void SetNull(Tuple* tuple) / void SetNotNull(Tuple* tuple)
- /// The codegen'd IR function is cached.
- llvm::Function* GetUpdateNullFn(LlvmCodeGen*, bool set_null) const;
+ /// Generate LLVM code at the insert position of 'builder' that returns a boolean value
+ /// represented as a LLVM i1 indicating whether this slot is null in 'tuple'.
+ llvm::Value* CodegenIsNull(
+ LlvmCodeGen* codegen, LlvmBuilder* builder, llvm::Value* tuple) const;
+
+ /// Generate LLVM code at the insert position of 'builder' that returns a boolean value
+ /// represented as a LLVM i1 with value of the NULL bit at 'null_indicator_offset' in
+ /// 'tuple'.
+ static llvm::Value* CodegenIsNull(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const NullIndicatorOffset& null_indicator_offset, llvm::Value* tuple);
+
+ /// Generate LLVM code at the insert position of 'builder' to set this slot's
+ /// NULL bit in the given 'tuple' to the value 'is_null'. 'tuple' is a pointer
+ /// to the tuple, and 'is_null' is an boolean value represented as a LLVM i1.
+ void CodegenSetNullIndicator(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ llvm::Value* tuple, llvm::Value* is_null) const;
private:
friend class DescriptorTbl;
@@ -163,13 +178,16 @@ class SlotDescriptor {
/// any padding bytes.
int llvm_field_idx_;
- /// Cached codegen'd functions
- mutable llvm::Function* set_not_null_fn_;
- mutable llvm::Function* set_null_fn_;
-
/// collection_item_descriptor should be non-NULL iff this is a collection slot
SlotDescriptor(const TSlotDescriptor& tdesc, const TupleDescriptor* parent,
- const TupleDescriptor* collection_item_descriptor);
+ const TupleDescriptor* collection_item_descriptor);
+
+ /// Generate LLVM code at the insert position of 'builder' to get the i8 value of the
+ /// the byte containing 'null_indicator_offset' in 'tuple'. If 'null_byte_ptr' is
+ /// non-NULL, sets that to a pointer to the null byte.
+ static llvm::Value* CodegenGetNullByte(LlvmCodeGen* codegen, LlvmBuilder* builder,
+ const NullIndicatorOffset& null_indicator_offset, llvm::Value* tuple,
+ llvm::Value** null_byte_ptr);
};
class ColumnDescriptor {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/runtime/tuple.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tuple.cc b/be/src/runtime/tuple.cc
index 7fa8b2b..dc83922 100644
--- a/be/src/runtime/tuple.cc
+++ b/be/src/runtime/tuple.cc
@@ -349,7 +349,7 @@ Status Tuple::CodegenMaterializeExprs(LlvmCodeGen* codegen, bool collect_string_
prototype.AddArgument("total_string_lengths", int_ptr_type);
prototype.AddArgument("num_non_null_string_values", int_ptr_type);
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[8];
*fn = prototype.GeneratePrototype(&builder, args);
Value* opaque_tuple_arg = args[0];
@@ -363,6 +363,9 @@ Status Tuple::CodegenMaterializeExprs(LlvmCodeGen* codegen, bool collect_string_
// Cast the opaque Tuple* argument to the generated struct type
Type* tuple_struct_type = desc.GetLlvmStruct(codegen);
+ if (tuple_struct_type == NULL) {
+ return Status("CodegenMaterializeExprs(): failed to generate tuple desc");
+ }
PointerType* tuple_type = codegen->GetPtrType(tuple_struct_type);
Value* tuple = builder.CreateBitCast(opaque_tuple_arg, tuple_type, "tuple");
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/runtime/types.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/types.h b/be/src/runtime/types.h
index 9558792..b0835bf 100644
--- a/be/src/runtime/types.h
+++ b/be/src/runtime/types.h
@@ -163,13 +163,28 @@ struct ColumnType {
return thrift_type;
}
+ inline bool IsBooleanType() const { return type == TYPE_BOOLEAN; }
+
+ inline bool IsIntegerType() const {
+ return type == TYPE_TINYINT || type == TYPE_SMALLINT || type == TYPE_INT
+ || type == TYPE_BIGINT;
+ }
+
+ inline bool IsFloatingPointType() const {
+ return type == TYPE_FLOAT || type == TYPE_DOUBLE;
+ }
+
+ inline bool IsDecimalType() const { return type == TYPE_DECIMAL; }
+
inline bool IsStringType() const {
return type == TYPE_STRING || type == TYPE_VARCHAR || type == TYPE_CHAR;
}
+ inline bool IsTimestampType() const { return type == TYPE_TIMESTAMP; }
+
inline bool IsVarLenStringType() const {
- return type == TYPE_STRING || type == TYPE_VARCHAR ||
- (type == TYPE_CHAR && len > MAX_CHAR_INLINE_LENGTH);
+ return type == TYPE_STRING || type == TYPE_VARCHAR
+ || (type == TYPE_CHAR && len > MAX_CHAR_INLINE_LENGTH);
}
inline bool IsComplexType() const {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/testutil/test-udas.cc
----------------------------------------------------------------------
diff --git a/be/src/testutil/test-udas.cc b/be/src/testutil/test-udas.cc
index 569853e..0097500 100644
--- a/be/src/testutil/test-udas.cc
+++ b/be/src/testutil/test-udas.cc
@@ -130,3 +130,39 @@ void ArgIsConstUpdate(FunctionContext* context, const IntVal& val,
void ArgIsConstMerge(FunctionContext* context, const BooleanVal& src, BooleanVal* dst) {
dst->val |= src.val;
}
+
+// Defines aggregate function for testing NULL handling. Returns NULL if an even number
+// of non-NULL inputs are consumed or 1 if an odd number of non-NULL inputs are consumed.
+void ToggleNullInit(FunctionContext* context, IntVal* total) {
+ *total = IntVal::null();
+}
+
+void ToggleNullUpdate(FunctionContext* context, const IntVal& val, IntVal* total) {
+ if (total->is_null) {
+ *total = IntVal(1);
+ } else {
+ *total = IntVal::null();
+ }
+}
+
+void ToggleNullMerge(FunctionContext* context, const IntVal& src, IntVal* dst) {
+ if (src.is_null != dst->is_null) {
+ *dst = IntVal(1);
+ } else {
+ *dst = IntVal::null();
+ }
+}
+
+// Defines aggregate function for testing input NULL handling. Returns the number of NULL
+// input values.
+void CountNullsInit(FunctionContext* context, BigIntVal* total) {
+ *total = BigIntVal(0);
+}
+
+void CountNullsUpdate(FunctionContext* context, const BigIntVal& val, BigIntVal* total) {
+ if (val.is_null) ++total->val;
+}
+
+void CountNullsMerge(FunctionContext* context, const BigIntVal& src, BigIntVal* dst) {
+ dst->val += src.val;
+}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/testutil/test-udfs.cc
----------------------------------------------------------------------
diff --git a/be/src/testutil/test-udfs.cc b/be/src/testutil/test-udfs.cc
index efdfa0a..56d4524 100644
--- a/be/src/testutil/test-udfs.cc
+++ b/be/src/testutil/test-udfs.cc
@@ -56,11 +56,7 @@ IntVal AllTypes(
StringVal NoArgs(FunctionContext* context) {
const char* result = "string";
StringVal ret(context, strlen(result));
- // TODO: llvm 3.3 seems to have a bug if we use memcpy here making
- // the IR udf crash. This is fixed in 3.3.1. Fix this when we upgrade.
- //memcpy(ret.ptr, result, strlen(result));
- // IMPALA-775
- for (int i = 0; i < strlen(result); ++i) ret.ptr[i] = result[i];
+ memcpy(ret.ptr, result, strlen(result));
return ret;
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d7246d64/be/src/util/tuple-row-compare.cc
----------------------------------------------------------------------
diff --git a/be/src/util/tuple-row-compare.cc b/be/src/util/tuple-row-compare.cc
index 69892ae..c159853 100644
--- a/be/src/util/tuple-row-compare.cc
+++ b/be/src/util/tuple-row-compare.cc
@@ -205,7 +205,7 @@ Status TupleRowComparator::CodegenCompare(LlvmCodeGen* codegen, Function** fn) {
prototype.AddArgument("lhs", tuple_row_type);
prototype.AddArgument("rhs", tuple_row_type);
- LlvmCodeGen::LlvmBuilder builder(context);
+ LlvmBuilder builder(context);
Value* args[4];
*fn = prototype.GeneratePrototype(&builder, args);
Value* lhs_ctxs_arg = args[0];
[5/5] incubator-impala git commit: IMPALA-1286: Extract common
conjuncts from disjunctions.
Posted by ab...@apache.org.
IMPALA-1286: Extract common conjuncts from disjunctions.
Adds a new ExprRewriteRule to extract common conjuncts from
disjunctions.
Examples:
(a AND b AND c) OR (b AND d) ==> b AND ((a AND c) OR (d))
(a AND b) OR (a AND b) ==> a AND b
(a AND b AND c) OR (c) ==> c
Adds a new query option ENABLE_EXPR_REWRITES to enable/disable
non-essential expr rewrites in the FE. Note that some rewrites
are required, e.g., BetweenToCompoundRule. Disabling the rewrites
is useful for testing, in particular, to make sure that the exprs
specified in expr-test.cc are executed as written.
Testing: Added a new unit test in ExprRewriteRulesTest.
Change-Id: I3cf9b950afaa3fd753d1b09ba5e540b5258940ad
Reviewed-on: http://gerrit.cloudera.org:8080/4877
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/0aeb6805
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/0aeb6805
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/0aeb6805
Branch: refs/heads/master
Commit: 0aeb68050b3bc16b8a10a5a3ae65428e7edd9df5
Parents: cfac09d
Author: Alex Behm <al...@cloudera.com>
Authored: Mon Oct 17 23:01:57 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Wed Nov 9 09:44:59 2016 +0000
----------------------------------------------------------------------
be/src/exprs/expr-test.cc | 3 +
be/src/service/query-options.cc | 5 +
be/src/service/query-options.h | 5 +-
common/thrift/ImpalaInternalService.thrift | 4 +
common/thrift/ImpalaService.thrift | 4 +
.../apache/impala/analysis/AnalysisContext.java | 7 ++
.../impala/analysis/BetweenPredicate.java | 6 ++
.../impala/analysis/CompoundPredicate.java | 26 +++--
.../apache/impala/analysis/ExistsPredicate.java | 10 +-
.../apache/impala/rewrite/ExprRewriteRule.java | 3 +
.../rewrite/ExtractCommonConjunctRule.java | 102 +++++++++++++++++++
.../impala/analysis/ExprRewriteRulesTest.java | 92 +++++++++++++++++
.../org/apache/impala/analysis/ExprTest.java | 3 +-
.../queries/PlannerTest/subquery-rewrite.test | 2 +-
.../queries/PlannerTest/tpch-all.test | 12 ++-
.../queries/PlannerTest/tpch-kudu.test | 5 +-
.../queries/PlannerTest/tpch-nested.test | 12 ++-
.../queries/PlannerTest/tpch-views.test | 4 +-
18 files changed, 282 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 5b930e2..5a999b6 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -6124,6 +6124,9 @@ int main(int argc, char **argv) {
ABORT_IF_ERROR(executor_->Setup());
vector<string> options;
+ // Disable FE Expr rewrites to make sure the Exprs get executed exactly as specified
+ // in the tests here.
+ options.push_back("ENABLE_EXPR_REWRITES=0");
options.push_back("DISABLE_CODEGEN=1");
disable_codegen_ = true;
executor_->setExecOptions(options);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/be/src/service/query-options.cc
----------------------------------------------------------------------
diff --git a/be/src/service/query-options.cc b/be/src/service/query-options.cc
index eb1d2f9..d6d2cb2 100644
--- a/be/src/service/query-options.cc
+++ b/be/src/service/query-options.cc
@@ -441,6 +441,11 @@ Status impala::SetQueryOption(const string& key, const string& value,
}
break;
}
+ case TImpalaQueryOptions::ENABLE_EXPR_REWRITES: {
+ query_options->__set_enable_expr_rewrites(
+ iequals(value, "true") || iequals(value, "1"));
+ break;
+ }
default:
// We hit this DCHECK(false) if we forgot to add the corresponding entry here
// when we add a new query option.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/be/src/service/query-options.h
----------------------------------------------------------------------
diff --git a/be/src/service/query-options.h b/be/src/service/query-options.h
index b1194d3..6531dcb 100644
--- a/be/src/service/query-options.h
+++ b/be/src/service/query-options.h
@@ -35,7 +35,7 @@ class TQueryOptions;
// the DCHECK.
#define QUERY_OPTS_TABLE\
DCHECK_EQ(_TImpalaQueryOptions_VALUES_TO_NAMES.size(),\
- TImpalaQueryOptions::SCRATCH_LIMIT + 1);\
+ TImpalaQueryOptions::ENABLE_EXPR_REWRITES + 1);\
QUERY_OPT_FN(abort_on_default_limit_exceeded, ABORT_ON_DEFAULT_LIMIT_EXCEEDED)\
QUERY_OPT_FN(abort_on_error, ABORT_ON_ERROR)\
QUERY_OPT_FN(allow_unsupported_formats, ALLOW_UNSUPPORTED_FORMATS)\
@@ -85,7 +85,8 @@ class TQueryOptions;
QUERY_OPT_FN(runtime_filter_max_size, RUNTIME_FILTER_MAX_SIZE)\
QUERY_OPT_FN(prefetch_mode, PREFETCH_MODE)\
QUERY_OPT_FN(strict_mode, STRICT_MODE)\
- QUERY_OPT_FN(scratch_limit, SCRATCH_LIMIT);
+ QUERY_OPT_FN(scratch_limit, SCRATCH_LIMIT)\
+ QUERY_OPT_FN(enable_expr_rewrites, ENABLE_EXPR_REWRITES);
/// Converts a TQueryOptions struct into a map of key, value pairs.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/common/thrift/ImpalaInternalService.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/ImpalaInternalService.thrift b/common/thrift/ImpalaInternalService.thrift
index d5a92ca..e510331 100644
--- a/common/thrift/ImpalaInternalService.thrift
+++ b/common/thrift/ImpalaInternalService.thrift
@@ -213,6 +213,10 @@ struct TQueryOptions {
// A limit on the amount of scratch directory space that can be used;
50: optional i64 scratch_limit = -1
+
+ // Indicates whether the FE should rewrite Exprs for optimization purposes.
+ // It's sometimes useful to disable rewrites for testing, e.g., expr-test.cc.
+ 51: optional bool enable_expr_rewrites = true
}
// Impala currently has two types of sessions: Beeswax and HiveServer2
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/common/thrift/ImpalaService.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/ImpalaService.thrift b/common/thrift/ImpalaService.thrift
index 573709b..68a0588 100644
--- a/common/thrift/ImpalaService.thrift
+++ b/common/thrift/ImpalaService.thrift
@@ -243,6 +243,10 @@ enum TImpalaQueryOptions {
// Unspecified or a limit of -1 means no limit;
// Otherwise specified in the same way as MEM_LIMIT.
SCRATCH_LIMIT
+
+ // Indicates whether the FE should rewrite Exprs for optimization purposes.
+ // It's sometimes useful to disable rewrites for testing, e.g., expr-test.cc.
+ ENABLE_EXPR_REWRITES
}
// The summary of an insert.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index b22bccb..3323ed4 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -37,6 +37,7 @@ import org.apache.impala.common.Pair;
import org.apache.impala.rewrite.BetweenToCompoundRule;
import org.apache.impala.rewrite.ExprRewriteRule;
import org.apache.impala.rewrite.ExprRewriter;
+import org.apache.impala.rewrite.ExtractCommonConjunctRule;
import org.apache.impala.thrift.TAccessEvent;
import org.apache.impala.thrift.TLineageGraph;
import org.apache.impala.thrift.TQueryCtx;
@@ -65,7 +66,13 @@ public class AnalysisContext {
catalog_ = catalog;
queryCtx_ = queryCtx;
authzConfig_ = authzConfig;
+ // BetweenPredicates must be rewritten to be executable. Other non-essential
+ // expr rewrites can be disabled via a query option. When rewrites are enabled
+ // BetweenPredicates should be rewritten first to help trigger other rules.
List<ExprRewriteRule> rules = Lists.newArrayList(BetweenToCompoundRule.INSTANCE);
+ if (queryCtx.getRequest().getQuery_options().enable_expr_rewrites) {
+ rules.add(ExtractCommonConjunctRule.INSTANCE);
+ }
rewriter_ = new ExprRewriter(rules);
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java b/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
index 2459715..ef19a52 100644
--- a/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
@@ -75,5 +75,11 @@ public class BetweenPredicate extends Predicate {
}
@Override
+ public boolean equals(Object obj) {
+ if (!super.equals(obj)) return false;
+ return isNotBetween_ == ((BetweenPredicate)obj).isNotBetween_;
+ }
+
+ @Override
public Expr clone() { return new BetweenPredicate(this); }
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java b/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
index d549053..6757d26 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
@@ -27,6 +27,7 @@ import org.apache.impala.catalog.Type;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.thrift.TExprNode;
import org.apache.impala.thrift.TExprNodeType;
+
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
@@ -192,16 +193,27 @@ public class CompoundPredicate extends Predicate {
* Creates a conjunctive predicate from a list of exprs.
*/
public static Expr createConjunctivePredicate(List<Expr> conjuncts) {
- Expr conjunctivePred = null;
- for (Expr expr: conjuncts) {
- if (conjunctivePred == null) {
- conjunctivePred = expr;
+ return createCompoundTree(conjuncts, Operator.AND);
+ }
+
+ /**
+ * Creates a disjunctive predicate from a list of exprs.
+ */
+ public static Expr createDisjunctivePredicate(List<Expr> disjuncts) {
+ return createCompoundTree(disjuncts, Operator.OR);
+ }
+
+ private static Expr createCompoundTree(List<Expr> exprs, Operator op) {
+ Preconditions.checkState(op == Operator.AND || op == Operator.OR);
+ Expr result = null;
+ for (Expr expr: exprs) {
+ if (result == null) {
+ result = expr;
continue;
}
- conjunctivePred = new CompoundPredicate(CompoundPredicate.Operator.AND,
- expr, conjunctivePred);
+ result = new CompoundPredicate(op, result, expr);
}
- return conjunctivePred;
+ return result;
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java b/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
index 3131acf..578c786 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
@@ -17,11 +17,11 @@
package org.apache.impala.analysis;
+import org.apache.impala.common.AnalysisException;
+import org.apache.impala.thrift.TExprNode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.impala.common.AnalysisException;
-import org.apache.impala.thrift.TExprNode;
import com.google.common.base.Preconditions;
/**
@@ -69,6 +69,12 @@ public class ExistsPredicate extends Predicate {
}
@Override
+ public boolean equals(Object o) {
+ if (!super.equals(o)) return false;
+ return notExists_ == ((ExistsPredicate)o).notExists_;
+ }
+
+ @Override
public Expr clone() { return new ExistsPredicate(this); }
@Override
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java b/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java
index 48054a8..bbd7c38 100644
--- a/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java
+++ b/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java
@@ -24,6 +24,9 @@ import org.apache.impala.common.AnalysisException;
/**
* Base class for all Expr rewrite rules. A rule is free to modify Exprs in place,
* but must return a different Expr object if any modifications were made.
+ * An ExprRewriteRule is intended to apply its transformation on a single Expr and not
+ * recursively on all its children. The recursive and repeated application of
+ * ExprRewriteRules is driven by an ExprRewriter.
*/
public interface ExprRewriteRule {
/**
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.java b/fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.java
new file mode 100644
index 0000000..e1515d3
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.java
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.impala.rewrite;
+
+import java.util.List;
+
+import org.apache.impala.analysis.Analyzer;
+import org.apache.impala.analysis.CompoundPredicate;
+import org.apache.impala.analysis.Expr;
+import org.apache.impala.common.AnalysisException;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+/**
+ * This rule extracts common conjuncts from multiple disjunctions when it is applied
+ * recursively bottom-up to a tree of CompoundPredicates.
+ *
+ * Examples:
+ * (a AND b AND c) OR (b AND d) ==> b AND ((a AND c) OR (d))
+ * (a AND b) OR (a AND b) ==> a AND b
+ * (a AND b AND c) OR (c) ==> c
+ */
+public class ExtractCommonConjunctRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new ExtractCommonConjunctRule();
+
+ // Arbitrary limit the number of Expr.equals() comparisons in the O(N^2) loop below.
+ // Used to avoid pathologically expensive invocations of this rule.
+ // TODO: Implement Expr.hashCode() and move to a hash-based solution for the core
+ // Expr.equals() comparison loop below.
+ private static final int MAX_EQUALS_COMPARISONS = 30 * 30;
+
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+ if (!Expr.IS_OR_PREDICATE.apply(expr)) return expr;
+
+ // Get childrens' conjuncts and check
+ List<Expr> child0Conjuncts = expr.getChild(0).getConjuncts();
+ List<Expr> child1Conjuncts = expr.getChild(1).getConjuncts();
+ Preconditions.checkState(!child0Conjuncts.isEmpty() && !child1Conjuncts.isEmpty());
+ // Impose cost bound.
+ if (child0Conjuncts.size() * child1Conjuncts.size() > MAX_EQUALS_COMPARISONS) {
+ return expr;
+ }
+
+ // Find common conjuncts.
+ List<Expr> commonConjuncts = Lists.newArrayList();
+ for (Expr conjunct: child0Conjuncts) {
+ if (child1Conjuncts.contains(conjunct)) {
+ // The conjunct may have parenthesis but there's no need to preserve them.
+ // Removing them makes the toSql() easier to read.
+ conjunct.setPrintSqlInParens(false);
+ commonConjuncts.add(conjunct);
+ }
+ }
+ if (commonConjuncts.isEmpty()) return expr;
+
+ // Remove common conjuncts.
+ child0Conjuncts.removeAll(commonConjuncts);
+ child1Conjuncts.removeAll(commonConjuncts);
+
+ // Check special case where one child contains all conjuncts of the other.
+ // (a AND b) OR (a AND b) ==> a AND b
+ // (a AND b AND c) OR (c) ==> c
+ if (child0Conjuncts.isEmpty() || child1Conjuncts.isEmpty()) {
+ Preconditions.checkState(!commonConjuncts.isEmpty());
+ Expr result = CompoundPredicate.createConjunctivePredicate(commonConjuncts);
+ result.analyze(analyzer);
+ return result;
+ }
+
+ // Re-assemble disjunctive predicate.
+ Expr child0Disjunct = CompoundPredicate.createConjunctivePredicate(child0Conjuncts);
+ child0Disjunct.setPrintSqlInParens(expr.getChild(0).getPrintSqlInParens());
+ Expr child1Disjunct = CompoundPredicate.createConjunctivePredicate(child1Conjuncts);
+ child1Disjunct.setPrintSqlInParens(expr.getChild(1).getPrintSqlInParens());
+ List<Expr> newDisjuncts = Lists.newArrayList(child0Disjunct, child1Disjunct);
+ Expr newDisjunction = CompoundPredicate.createDisjunctivePredicate(newDisjuncts);
+ newDisjunction.setPrintSqlInParens(true);
+ Expr result = CompoundPredicate.createConjunction(newDisjunction,
+ CompoundPredicate.createConjunctivePredicate(commonConjuncts));
+ result.analyze(analyzer);
+ return result;
+ }
+
+ private ExtractCommonConjunctRule() {}
+}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
index d3d52b4..16ca7e2 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
@@ -25,6 +25,7 @@ import org.apache.impala.common.FrontendTestBase;
import org.apache.impala.rewrite.BetweenToCompoundRule;
import org.apache.impala.rewrite.ExprRewriteRule;
import org.apache.impala.rewrite.ExprRewriter;
+import org.apache.impala.rewrite.ExtractCommonConjunctRule;
import org.junit.Assert;
import org.junit.Test;
@@ -106,4 +107,95 @@ public class ExprRewriteRulesTest extends FrontendTestBase {
"int_col >= if(tinyint_col >= 1 AND tinyint_col <= 2, 10, 20) " +
"AND int_col <= CAST(smallint_col < 1 OR smallint_col > 2 AS INT)");
}
+
+ @Test
+ public void TestExtractCommonConjunctsRule() throws AnalysisException {
+ ExprRewriteRule rule = ExtractCommonConjunctRule.INSTANCE;
+
+ // One common conjunct: int_col < 10
+ RewritesOk(
+ "(int_col < 10 and bigint_col < 10) or " +
+ "(string_col = '10' and int_col < 10)", rule,
+ "int_col < 10 AND ((bigint_col < 10) OR (string_col = '10'))");
+ // One common conjunct in multiple disjuncts: int_col < 10
+ RewritesOk(
+ "(int_col < 10 and bigint_col < 10) or " +
+ "(string_col = '10' and int_col < 10) or " +
+ "(id < 20 and int_col < 10) or " +
+ "(int_col < 10 and float_col > 3.14)", rule,
+ "int_col < 10 AND " +
+ "((bigint_col < 10) OR (string_col = '10') OR " +
+ "(id < 20) OR (float_col > 3.14))");
+ // Same as above but with a bushy OR tree.
+ RewritesOk(
+ "((int_col < 10 and bigint_col < 10) or " +
+ " (string_col = '10' and int_col < 10)) or " +
+ "((id < 20 and int_col < 10) or " +
+ " (int_col < 10 and float_col > 3.14))", rule,
+ "int_col < 10 AND " +
+ "((bigint_col < 10) OR (string_col = '10') OR " +
+ "(id < 20) OR (float_col > 3.14))");
+ // Multiple common conjuncts: int_col < 10, bool_col is null
+ RewritesOk(
+ "(int_col < 10 and bigint_col < 10 and bool_col is null) or " +
+ "(bool_col is null and string_col = '10' and int_col < 10)", rule,
+ "int_col < 10 AND bool_col IS NULL AND " +
+ "((bigint_col < 10) OR (string_col = '10'))");
+ // Negated common conjunct: !(int_col=5 or tinyint_col > 9)
+ RewritesOk(
+ "(!(int_col=5 or tinyint_col > 9) and double_col = 7) or " +
+ "(!(int_col=5 or tinyint_col > 9) and double_col = 8)", rule,
+ "NOT (int_col = 5 OR tinyint_col > 9) AND " +
+ "((double_col = 7) OR (double_col = 8))");
+
+ // Test common BetweenPredicate: int_col between 10 and 30
+ RewritesOk(
+ "(int_col between 10 and 30 and bigint_col < 10) or " +
+ "(string_col = '10' and int_col between 10 and 30) or " +
+ "(id < 20 and int_col between 10 and 30) or " +
+ "(int_col between 10 and 30 and float_col > 3.14)", rule,
+ "int_col BETWEEN 10 AND 30 AND " +
+ "((bigint_col < 10) OR (string_col = '10') OR " +
+ "(id < 20) OR (float_col > 3.14))");
+ // Test common NOT BetweenPredicate: int_col not between 10 and 30
+ RewritesOk(
+ "(int_col not between 10 and 30 and bigint_col < 10) or " +
+ "(string_col = '10' and int_col not between 10 and 30) or " +
+ "(id < 20 and int_col not between 10 and 30) or " +
+ "(int_col not between 10 and 30 and float_col > 3.14)", rule,
+ "int_col NOT BETWEEN 10 AND 30 AND " +
+ "((bigint_col < 10) OR (string_col = '10') OR " +
+ "(id < 20) OR (float_col > 3.14))");
+ // Test mixed BetweenPredicates are not common.
+ RewritesOk(
+ "(int_col not between 10 and 30 and bigint_col < 10) or " +
+ "(string_col = '10' and int_col between 10 and 30) or " +
+ "(id < 20 and int_col not between 10 and 30) or " +
+ "(int_col between 10 and 30 and float_col > 3.14)", rule,
+ null);
+
+ // All conjuncts are common.
+ RewritesOk(
+ "(int_col < 10 and id between 5 and 6) or " +
+ "(id between 5 and 6 and int_col < 10) or " +
+ "(int_col < 10 and id between 5 and 6)", rule,
+ "int_col < 10 AND id BETWEEN 5 AND 6");
+ // Complex disjuncts are redundant.
+ RewritesOk(
+ "(int_col < 10) or " +
+ "(int_col < 10 and bigint_col < 10 and bool_col is null) or " +
+ "(int_col < 10) or " +
+ "(bool_col is null and int_col < 10)", rule,
+ "int_col < 10");
+
+ // Due to the shape of the original OR tree we are left with redundant
+ // disjuncts after the extraction.
+ RewritesOk(
+ "(int_col < 10 and bigint_col < 10) or " +
+ "(string_col = '10' and int_col < 10) or " +
+ "(id < 20 and int_col < 10) or " +
+ "(int_col < 10 and id < 20)", rule,
+ "int_col < 10 AND " +
+ "((bigint_col < 10) OR (string_col = '10') OR (id < 20) OR (id < 20))");
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/fe/src/test/java/org/apache/impala/analysis/ExprTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprTest.java
index 7ae0f89..e0eca0a 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ExprTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ExprTest.java
@@ -19,10 +19,9 @@ package org.apache.impala.analysis;
import static org.junit.Assert.fail;
-import org.junit.Test;
-
import org.apache.impala.catalog.ScalarType;
import org.apache.impala.catalog.Type;
+import org.junit.Test;
public class ExprTest {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
index 0880d9f..dfc9072 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
@@ -1638,7 +1638,7 @@ where
PLAN-ROOT SINK
|
05:HASH JOIN [RIGHT SEMI JOIN]
-| hash predicates: sum(t1.id) = t.int_col
+| hash predicates: t1.id + t2.id = t.int_col
|
|--00:SCAN HDFS [functional.alltypestiny t]
| partitions=4/4 files=4 size=460B
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
index 9b17ff1..219e5a9 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
@@ -3415,14 +3415,16 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN]
| hash predicates: l_partkey = p_partkey
-| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size <= 5) OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size <= 10) OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size <= 15))
| runtime filters: RF000 <- p_partkey
|
|--01:SCAN HDFS [tpch.part]
| partitions=1/1 files=1 size=22.83MB
+| predicates: p_size >= 1
|
00:SCAN HDFS [tpch.lineitem]
partitions=1/1 files=1 size=718.94MB
+ predicates: l_shipmode IN ('AIR', 'AIR REG'), l_shipinstruct = 'DELIVER IN PERSON'
runtime filters: RF000 -> l_partkey
---- DISTRIBUTEDPLAN
PLAN-ROOT SINK
@@ -3437,16 +3439,18 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN, BROADCAST]
| hash predicates: l_partkey = p_partkey
-| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size <= 5) OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size <= 10) OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size <= 15))
| runtime filters: RF000 <- p_partkey
|
|--04:EXCHANGE [BROADCAST]
| |
| 01:SCAN HDFS [tpch.part]
| partitions=1/1 files=1 size=22.83MB
+| predicates: p_size >= 1
|
00:SCAN HDFS [tpch.lineitem]
partitions=1/1 files=1 size=718.94MB
+ predicates: l_shipmode IN ('AIR', 'AIR REG'), l_shipinstruct = 'DELIVER IN PERSON'
runtime filters: RF000 -> l_partkey
---- PARALLELPLANS
PLAN-ROOT SINK
@@ -3461,7 +3465,7 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN, BROADCAST]
| hash predicates: l_partkey = p_partkey
-| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size <= 5) OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size <= 10) OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size <= 15))
| runtime filters: RF000 <- p_partkey
|
|--JOIN BUILD
@@ -3472,9 +3476,11 @@ PLAN-ROOT SINK
| |
| 01:SCAN HDFS [tpch.part]
| partitions=1/1 files=1 size=22.83MB
+| predicates: p_size >= 1
|
00:SCAN HDFS [tpch.lineitem]
partitions=1/1 files=1 size=718.94MB
+ predicates: l_shipmode IN ('AIR', 'AIR REG'), l_shipinstruct = 'DELIVER IN PERSON'
runtime filters: RF000 -> l_partkey
====
# TPCH-Q20
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
index a5e6e07..a5f2511 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
@@ -1074,11 +1074,14 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN]
| hash predicates: l_partkey = p_partkey
-| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size <= 5) OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size <= 10) OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size <= 15))
|
|--01:SCAN KUDU [tpch_kudu.part]
+| kudu predicates: p_size >= 1
|
00:SCAN KUDU [tpch_kudu.lineitem]
+ predicates: l_shipmode IN ('AIR', 'AIR REG')
+ kudu predicates: l_shipinstruct = 'DELIVER IN PERSON'
====
# Q20 - Potential Part Promotion Query
select
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
index 0c3dd9b..31e5d40 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
@@ -632,7 +632,7 @@ PLAN-ROOT SINK
| output: sum(l_extendedprice * l_discount)
|
00:SCAN HDFS [tpch_nested_parquet.customer.c_orders.o_lineitems]
- partitions=1/1 files=4 size=292.35MB
+ partitions=1/1 files=4 size=577.87MB
predicates: l_discount <= 0.07, l_discount >= 0.05, l_quantity < 24, l_shipdate < '1995-01-01', l_shipdate >= '1994-01-01'
---- DISTRIBUTEDPLAN
PLAN-ROOT SINK
@@ -646,7 +646,7 @@ PLAN-ROOT SINK
| output: sum(l_extendedprice * l_discount)
|
00:SCAN HDFS [tpch_nested_parquet.customer.c_orders.o_lineitems]
- partitions=1/1 files=4 size=292.35MB
+ partitions=1/1 files=4 size=577.87MB
predicates: l_discount <= 0.07, l_discount >= 0.05, l_quantity < 24, l_shipdate < '1995-01-01', l_shipdate >= '1994-01-01'
====
# TPCH-Q7
@@ -2117,14 +2117,16 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN]
| hash predicates: l_partkey = p_partkey
-| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size <= 5) OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size <= 10) OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size <= 15))
| runtime filters: RF000 <- p_partkey
|
|--01:SCAN HDFS [tpch_nested_parquet.part p]
| partitions=1/1 files=1 size=6.20MB
+| predicates: p_size >= 1
|
00:SCAN HDFS [tpch_nested_parquet.customer.c_orders.o_lineitems l]
partitions=1/1 files=4 size=577.87MB
+ predicates: l_shipmode IN ('AIR', 'AIR REG'), l_shipinstruct = 'DELIVER IN PERSON'
runtime filters: RF000 -> l_partkey
---- DISTRIBUTEDPLAN
PLAN-ROOT SINK
@@ -2139,16 +2141,18 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN, BROADCAST]
| hash predicates: l_partkey = p_partkey
-| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size <= 5) OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size <= 10) OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size <= 15))
| runtime filters: RF000 <- p_partkey
|
|--04:EXCHANGE [BROADCAST]
| |
| 01:SCAN HDFS [tpch_nested_parquet.part p]
| partitions=1/1 files=1 size=6.20MB
+| predicates: p_size >= 1
|
00:SCAN HDFS [tpch_nested_parquet.customer.c_orders.o_lineitems l]
partitions=1/1 files=4 size=577.87MB
+ predicates: l_shipmode IN ('AIR', 'AIR REG'), l_shipinstruct = 'DELIVER IN PERSON'
runtime filters: RF000 -> l_partkey
====
# TPCH-Q20
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0aeb6805/testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
index de442b2..c927b8b 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
@@ -1253,14 +1253,16 @@ PLAN-ROOT SINK
|
02:HASH JOIN [INNER JOIN]
| hash predicates: tpch.lineitem.l_partkey = tpch.part.p_partkey
-| other predicates: ((tpch.part.p_brand = 'Brand#12' AND tpch.part.p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND tpch.lineitem.l_quantity >= 1 AND tpch.lineitem.l_quantity <= 11 AND tpch.part.p_size >= 1 AND tpch.part.p_size <= 5 AND tpch.lineitem.l_shipmode IN ('AIR', 'AIR REG') AND tpch.lineitem.l_shipinstruct = 'DELIVER IN PERSON') OR (tpch.part.p_brand = 'Brand#23' AND tpch.part.p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND tpch.lineitem.l_quantity >= 10 AND tpch.lineitem.l_quantity <= 20 AND tpch.part.p_size >= 1 AND tpch.part.p_size <= 10 AND tpch.lineitem.l_shipmode IN ('AIR', 'AIR REG') AND tpch.lineitem.l_shipinstruct = 'DELIVER IN PERSON') OR (tpch.part.p_brand = 'Brand#34' AND tpch.part.p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND tpch.lineitem.l_quantity >= 20 AND tpch.lineitem.l_quantity <= 30 AND tpch.part.p_size >= 1 AND tpch.part.p_size <= 15 AND tpch.lineitem.l_shipmode IN ('AIR', 'AIR REG') AND tpch.lineitem.l_sh
ipinstruct = 'DELIVER IN PERSON'))
+| other predicates: ((tpch.part.p_brand = 'Brand#12' AND tpch.part.p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND tpch.lineitem.l_quantity >= 1 AND tpch.lineitem.l_quantity <= 11 AND tpch.part.p_size <= 5) OR (tpch.part.p_brand = 'Brand#23' AND tpch.part.p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND tpch.lineitem.l_quantity >= 10 AND tpch.lineitem.l_quantity <= 20 AND tpch.part.p_size <= 10) OR (tpch.part.p_brand = 'Brand#34' AND tpch.part.p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND tpch.lineitem.l_quantity >= 20 AND tpch.lineitem.l_quantity <= 30 AND tpch.part.p_size <= 15))
| runtime filters: RF000 <- tpch.part.p_partkey
|
|--01:SCAN HDFS [tpch.part]
| partitions=1/1 files=1 size=22.83MB
+| predicates: tpch.part.p_size >= 1
|
00:SCAN HDFS [tpch.lineitem]
partitions=1/1 files=1 size=718.94MB
+ predicates: tpch.lineitem.l_shipmode IN ('AIR', 'AIR REG'), tpch.lineitem.l_shipinstruct = 'DELIVER IN PERSON'
runtime filters: RF000 -> tpch.lineitem.l_partkey
====
# TPCH-Q20
[4/5] incubator-impala git commit: IMPALA-3710: Kudu DML should
ignore conflicts, pt2
Posted by ab...@apache.org.
IMPALA-3710: Kudu DML should ignore conflicts, pt2
Second part of IMPALA-3710, which removed the IGNORE DML
option and changed the following errors on Kudu DML
operations to be ignored:
1) INSERT where the PK already exists
2) UPDATE/DELETE where the PK doesn't exist
This changes other data-related errors to be ignored as
well:
3) NULLs in non-nullable columns, i.e. null constraint
violoations.
4) Rows with PKs that are in an 'uncovered range'.
It became clear that we can't differentiate between (3) and
(4) because both return a Kudu 'NotFound' error code. The
Impala error codes have been simplified as well: we just
report a generic KUDU_NOT_FOUND error in these cases.
This also adds some metadata to the thrift report sent to
the coordinator from sinks so the total number of rows with
errors can be added to the profile. Note that this does not
include a breakdown of error counts by type/code because we
cannot differentiate between all of these cases yet.
An upcoming change will add this new info to the beeswax
interface and show it in the shell output (IMPALA-3713).
Testing: Updated kudu_crud tests to check the number of rows
with errors.
Change-Id: I4eb1ad91dc355ea51de261c3a14df0f9d28c879c
Reviewed-on: http://gerrit.cloudera.org:8080/4985
Reviewed-by: Alex Behm <al...@cloudera.com>
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/cfac09de
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/cfac09de
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/cfac09de
Branch: refs/heads/master
Commit: cfac09de10c996a48852b9a9d50c70cf24cf5f5f
Parents: d7246d6
Author: Matthew Jacobs <mj...@cloudera.com>
Authored: Mon Nov 7 15:55:42 2016 -0800
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Wed Nov 9 06:43:41 2016 +0000
----------------------------------------------------------------------
be/src/exec/data-sink.cc | 17 ++-
be/src/exec/data-sink.h | 9 +-
be/src/exec/hdfs-table-sink.cc | 2 +-
be/src/exec/kudu-table-sink.cc | 89 +++++++++-------
be/src/exec/kudu-table-sink.h | 31 +++---
be/src/runtime/coordinator.cc | 4 +-
common/thrift/ImpalaInternalService.thrift | 21 +++-
common/thrift/generate_error_codes.py | 5 +-
.../queries/QueryTest/kudu_crud.test | 103 ++++++++++++-------
9 files changed, 180 insertions(+), 101 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/be/src/exec/data-sink.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/data-sink.cc b/be/src/exec/data-sink.cc
index 6a34543..17f1f8c 100644
--- a/be/src/exec/data-sink.cc
+++ b/be/src/exec/data-sink.cc
@@ -112,9 +112,18 @@ Status DataSink::CreateDataSink(ObjectPool* pool,
return Status::OK();
}
-void DataSink::MergeInsertStats(const TInsertStats& src_stats,
+void DataSink::MergeDmlStats(const TInsertStats& src_stats,
TInsertStats* dst_stats) {
dst_stats->bytes_written += src_stats.bytes_written;
+ if (src_stats.__isset.kudu_stats) {
+ if (!dst_stats->__isset.kudu_stats) dst_stats->__set_kudu_stats(TKuduDmlStats());
+ if (!dst_stats->kudu_stats.__isset.num_row_errors) {
+ dst_stats->kudu_stats.__set_num_row_errors(0);
+ }
+
+ dst_stats->kudu_stats.__set_num_row_errors(
+ dst_stats->kudu_stats.num_row_errors + src_stats.kudu_stats.num_row_errors);
+ }
if (src_stats.__isset.parquet_stats) {
if (dst_stats->__isset.parquet_stats) {
MergeMapValues<string, int64_t>(src_stats.parquet_stats.per_column_size,
@@ -125,7 +134,7 @@ void DataSink::MergeInsertStats(const TInsertStats& src_stats,
}
}
-string DataSink::OutputInsertStats(const PartitionStatusMap& stats,
+string DataSink::OutputDmlStats(const PartitionStatusMap& stats,
const string& prefix) {
const char* indent = " ";
stringstream ss;
@@ -148,6 +157,10 @@ string DataSink::OutputInsertStats(const PartitionStatusMap& stats,
if (!val.second.__isset.stats) continue;
const TInsertStats& stats = val.second.stats;
+ if (stats.__isset.kudu_stats) {
+ ss << "NumRowErrors: " << stats.kudu_stats.num_row_errors << endl;
+ }
+
ss << indent << "BytesWritten: "
<< PrettyPrinter::Print(stats.bytes_written, TUnit::BYTES);
if (stats.__isset.parquet_stats) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/be/src/exec/data-sink.h
----------------------------------------------------------------------
diff --git a/be/src/exec/data-sink.h b/be/src/exec/data-sink.h
index 95acf16..1970cf2 100644
--- a/be/src/exec/data-sink.h
+++ b/be/src/exec/data-sink.h
@@ -85,14 +85,13 @@ class DataSink {
const TPlanFragmentInstanceCtx& fragment_instance_ctx,
const RowDescriptor& row_desc, boost::scoped_ptr<DataSink>* sink);
- /// Merges one update to the insert stats for a partition. dst_stats will have the
+ /// Merges one update to the DML stats for a partition. dst_stats will have the
/// combined stats of src_stats and dst_stats after this method returns.
- static void MergeInsertStats(const TInsertStats& src_stats,
+ static void MergeDmlStats(const TInsertStats& src_stats,
TInsertStats* dst_stats);
- /// Outputs the insert stats contained in the map of insert partition updates to a
- /// string
- static std::string OutputInsertStats(const PartitionStatusMap& stats,
+ /// Outputs the DML stats contained in the map of partition updates to a string
+ static std::string OutputDmlStats(const PartitionStatusMap& stats,
const std::string& prefix = "");
MemTracker* mem_tracker() const { return mem_tracker_; }
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/be/src/exec/hdfs-table-sink.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-table-sink.cc b/be/src/exec/hdfs-table-sink.cc
index 63bd648..07c1167 100644
--- a/be/src/exec/hdfs-table-sink.cc
+++ b/be/src/exec/hdfs-table-sink.cc
@@ -595,7 +595,7 @@ Status HdfsTableSink::FinalizePartitionFile(RuntimeState* state,
// initialised.
DCHECK(it != state->per_partition_status()->end());
it->second.num_modified_rows += partition->num_rows;
- DataSink::MergeInsertStats(partition->writer->stats(), &it->second.stats);
+ DataSink::MergeDmlStats(partition->writer->stats(), &it->second.stats);
}
RETURN_IF_ERROR(ClosePartitionFile(state, partition));
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/be/src/exec/kudu-table-sink.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-table-sink.cc b/be/src/exec/kudu-table-sink.cc
index a58baab..11613aa 100644
--- a/be/src/exec/kudu-table-sink.cc
+++ b/be/src/exec/kudu-table-sink.cc
@@ -61,9 +61,9 @@ KuduTableSink::KuduTableSink(const RowDescriptor& row_desc,
select_list_texprs_(select_list_texprs),
sink_action_(tsink.table_sink.action),
kudu_table_sink_(tsink.table_sink.kudu_table_sink),
- kudu_error_counter_(NULL),
- rows_written_(NULL),
- rows_written_rate_(NULL) {
+ total_rows_(NULL),
+ num_row_errors_(NULL),
+ rows_processed_rate_(NULL) {
DCHECK(KuduIsAvailable());
}
@@ -96,15 +96,19 @@ Status KuduTableSink::Prepare(RuntimeState* state, MemTracker* mem_tracker) {
TInsertPartitionStatus root_status;
root_status.__set_num_modified_rows(0L);
root_status.__set_id(-1L);
+ TKuduDmlStats kudu_dml_stats;
+ kudu_dml_stats.__set_num_row_errors(0L);
+ root_status.__set_stats(TInsertStats());
+ root_status.stats.__set_kudu_stats(kudu_dml_stats);
state->per_partition_status()->insert(make_pair(ROOT_PARTITION_KEY, root_status));
// Add counters
- kudu_error_counter_ = ADD_COUNTER(profile(), "TotalKuduFlushErrors", TUnit::UNIT);
- rows_written_ = ADD_COUNTER(profile(), "RowsWritten", TUnit::UNIT);
+ total_rows_ = ADD_COUNTER(profile(), "TotalNumRows", TUnit::UNIT);
+ num_row_errors_ = ADD_COUNTER(profile(), "NumRowErrors", TUnit::UNIT);
kudu_apply_timer_ = ADD_TIMER(profile(), "KuduApplyTimer");
- rows_written_rate_ = profile()->AddDerivedCounter(
- "RowsWrittenRate", TUnit::UNIT_PER_SECOND,
- bind<int64_t>(&RuntimeProfile::UnitsPerSecond, rows_written_,
+ rows_processed_rate_ = profile()->AddDerivedCounter(
+ "RowsProcessedRate", TUnit::UNIT_PER_SECOND,
+ bind<int64_t>(&RuntimeProfile::UnitsPerSecond, total_rows_,
profile()->total_time_counter()));
return Status::OK();
@@ -174,16 +178,21 @@ Status KuduTableSink::Send(RuntimeState* state, RowBatch* batch) {
SCOPED_TIMER(profile()->total_time_counter());
ExprContext::FreeLocalAllocations(output_expr_ctxs_);
RETURN_IF_ERROR(state->CheckQueryState());
+ const KuduSchema& table_schema = table_->schema();
// Collect all write operations and apply them together so the time in Apply() can be
// easily timed.
vector<unique_ptr<kudu::client::KuduWriteOperation>> write_ops;
- int rows_added = 0;
+ // Count the number of rows with nulls in non-nullable columns, i.e. null constraint
+ // violations.
+ int num_null_violations = 0;
+
// Since everything is set up just forward everything to the writer.
for (int i = 0; i < batch->num_rows(); ++i) {
TupleRow* current_row = batch->GetRow(i);
unique_ptr<kudu::client::KuduWriteOperation> write(NewWriteOp());
+ bool add_row = true;
for (int j = 0; j < output_expr_ctxs_.size(); ++j) {
// For INSERT, output_expr_ctxs_ will contain all columns of the table in order.
@@ -198,14 +207,20 @@ Status KuduTableSink::Send(RuntimeState* state, RowBatch* batch) {
// If the value is NULL, we only need to explicitly set it for UPDATE and UPSERT.
// For INSERT, it can be ignored as unspecified cols will be implicitly set to NULL.
if (value == NULL) {
- if (sink_action_ == TSinkAction::UPDATE || sink_action_ == TSinkAction::UPSERT) {
- DCHECK(!kudu_table_sink_.referenced_columns.empty());
+ if (table_schema.Column(j).is_nullable()) {
KUDU_RETURN_IF_ERROR(write->mutable_row()->SetNull(col),
"Could not add Kudu WriteOp.");
+ continue;
} else {
- DCHECK(kudu_table_sink_.referenced_columns.empty());
+ // This row violates the nullability constraints of the column, do not attempt
+ // to write this row because it is already known to be an error and the Kudu
+ // error will be difficult to interpret later (error code isn't specific).
+ ++num_null_violations;
+ state->LogError(ErrorMsg::Init(TErrorCode::KUDU_NULL_CONSTRAINT_VIOLATION,
+ table_desc_->table_name()));
+ add_row = false;
+ break; // skip remaining columns for this row
}
- continue;
}
PrimitiveType type = output_expr_ctxs_[j]->root()->type().type;
@@ -257,18 +272,22 @@ Status KuduTableSink::Send(RuntimeState* state, RowBatch* batch) {
return Status(TErrorCode::IMPALA_KUDU_TYPE_MISSING, TypeToString(type));
}
}
- write_ops.push_back(move(write));
+ if (add_row) write_ops.push_back(move(write));
}
{
SCOPED_TIMER(kudu_apply_timer_);
for (auto&& write: write_ops) {
KUDU_RETURN_IF_ERROR(session_->Apply(write.release()), "Error applying Kudu Op.");
- ++rows_added;
}
}
- COUNTER_ADD(rows_written_, rows_added);
+ // Increment for all rows received by the sink, including errors.
+ COUNTER_ADD(total_rows_, batch->num_rows());
+ // Add the number of null constraint violations to the number of row errors, which
+ // isn't reported by Kudu in CheckForErrors() because those rows were never
+ // successfully added to the KuduSession.
+ COUNTER_ADD(num_row_errors_, num_null_violations);
RETURN_IF_ERROR(CheckForErrors(state));
return Status::OK();
}
@@ -281,7 +300,6 @@ Status KuduTableSink::CheckForErrors(RuntimeState* state) {
// Get the pending errors from the Kudu session. If errors overflowed the error buffer
// we can't be sure all errors can be ignored, so an error status will be reported.
- // TODO: Make sure Kudu handles conflict errors properly if IGNORE is set (KUDU-1563).
bool error_overflow = false;
session_->GetPendingErrors(&errors, &error_overflow);
if (UNLIKELY(error_overflow)) {
@@ -292,28 +310,27 @@ Status KuduTableSink::CheckForErrors(RuntimeState* state) {
// them accordingly.
for (int i = 0; i < errors.size(); ++i) {
kudu::Status e = errors[i]->status();
- // 'Duplicate key' or 'key already present' errors from Kudu do not fail the query.
- if ((sink_action_ == TSinkAction::DELETE && !e.IsNotFound()) ||
- (sink_action_ == TSinkAction::UPDATE && !e.IsNotFound()) ||
- (sink_action_ == TSinkAction::INSERT && !e.IsAlreadyPresent())) {
- if (status.ok()) {
- status = Status(strings::Substitute(
- "Kudu error(s) reported, first error: $0", e.ToString()));
- }
- }
if (e.IsNotFound()) {
- state->LogError(ErrorMsg::Init(TErrorCode::KUDU_KEY_NOT_FOUND,
- table_desc_->table_name()));
+ // Kudu does not yet have a way to programmatically differentiate between 'row not
+ // found' and 'tablet not found' (e.g. PK in a non-covered range) and both have the
+ // IsNotFound error code.
+ state->LogError(ErrorMsg::Init(TErrorCode::KUDU_NOT_FOUND,
+ table_desc_->table_name(), e.ToString()), 2);
} else if (e.IsAlreadyPresent()) {
state->LogError(ErrorMsg::Init(TErrorCode::KUDU_KEY_ALREADY_PRESENT,
- table_desc_->table_name()));
+ table_desc_->table_name()), 2);
} else {
+ if (status.ok()) {
+ status = Status(strings::Substitute(
+ "Kudu error(s) reported, first error: $0", e.ToString()));
+ }
state->LogError(ErrorMsg::Init(TErrorCode::KUDU_SESSION_ERROR,
- table_desc_->table_name(), e.ToString()));
+ table_desc_->table_name(), e.ToString()), 2);
}
delete errors[i];
}
- COUNTER_ADD(kudu_error_counter_, errors.size());
+
+ COUNTER_ADD(num_row_errors_, errors.size());
return status;
}
@@ -326,10 +343,12 @@ Status KuduTableSink::FlushFinal(RuntimeState* state) {
VLOG_RPC << "Ignoring Flush() error status: " << flush_status.ToString();
}
Status status = CheckForErrors(state);
- (*state->per_partition_status())[ROOT_PARTITION_KEY].__set_num_modified_rows(
- rows_written_->value() - kudu_error_counter_->value());
- (*state->per_partition_status())[ROOT_PARTITION_KEY].__set_kudu_latest_observed_ts(
- client_->GetLatestObservedTimestamp());
+ TInsertPartitionStatus& insert_status =
+ (*state->per_partition_status())[ROOT_PARTITION_KEY];
+ insert_status.__set_num_modified_rows(
+ total_rows_->value() - num_row_errors_->value());
+ insert_status.stats.kudu_stats.__set_num_row_errors(num_row_errors_->value());
+ insert_status.__set_kudu_latest_observed_ts(client_->GetLatestObservedTimestamp());
return status;
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/be/src/exec/kudu-table-sink.h
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-table-sink.h b/be/src/exec/kudu-table-sink.h
index adf264e..3ff667c 100644
--- a/be/src/exec/kudu-table-sink.h
+++ b/be/src/exec/kudu-table-sink.h
@@ -44,14 +44,13 @@ namespace impala {
/// because Kudu currently has some 8MB buffer limits.
///
/// Kudu doesn't have transactions yet, so some rows may fail to write while others are
-/// successful. The Kudu client reports errors, some of which may be considered to be
-/// expected: rows that fail to be written/updated/deleted due to a key conflict will not
-/// result in the sink returning an error. Any other kind of error reported by Kudu
-/// results in the sink returning an error status. The first non-ignored error is
-/// returned in the sink's Status. All reported errors (ignored or not) will be logged
-/// via the RuntimeState.
-/// TODO: Handle other data/constraint-violation errors as ignored, e.g. null in
-/// non-nullable col
+/// successful. The Kudu client reports errors, some of which are treated as warnings and
+/// will not fail the query: PK already exists on INSERT, key not found on UPDATE/DELETE,
+/// NULL in a non-nullable column, and PK specifying rows in an uncovered range.
+/// The number of rows that cannot be modified due to these errors is reported in the
+/// TInsertPartitionStatus report sent by the DataSink to the coordinator.
+/// Any other kind of error reported by Kudu results in the sink returning an error
+/// status. All reported errors (ignored or not) will be logged via the RuntimeState.
class KuduTableSink : public DataSink {
public:
KuduTableSink(const RowDescriptor& row_desc,
@@ -101,7 +100,6 @@ class KuduTableSink : public DataSink {
std::vector<ExprContext*> output_expr_ctxs_;
/// The Kudu client, table and session.
- /// This uses 'kudu::client::sp::shared_ptr' as that is the type expected by Kudu.
kudu::client::sp::shared_ptr<kudu::client::KuduClient> client_;
kudu::client::sp::shared_ptr<kudu::client::KuduTable> table_;
kudu::client::sp::shared_ptr<kudu::client::KuduSession> session_;
@@ -112,19 +110,22 @@ class KuduTableSink : public DataSink {
/// Captures parameters passed down from the frontend
TKuduTableSink kudu_table_sink_;
- /// Total number of errors returned from Kudu.
- RuntimeProfile::Counter* kudu_error_counter_;
-
/// Time spent applying Kudu operations. In normal circumstances, Apply() should be
/// negligible because it is asynchronous with AUTO_FLUSH_BACKGROUND enabled.
/// Significant time spent in Apply() may indicate that Kudu cannot buffer and send
/// rows as fast as the sink can write them.
RuntimeProfile::Counter* kudu_apply_timer_;
- /// Total number of rows written including errors.
- RuntimeProfile::Counter* rows_written_;
- RuntimeProfile::Counter* rows_written_rate_;
+ /// Total number of rows processed, i.e. rows written to Kudu and also rows with
+ /// errors.
+ RuntimeProfile::Counter* total_rows_;
+
+ /// The number of rows with errors.
+ RuntimeProfile::Counter* num_row_errors_;
+ /// Rate at which the sink consumes and processes rows, i.e. writing rows to Kudu or
+ /// skipping rows that are known to violate nullability constraints.
+ RuntimeProfile::Counter* rows_processed_rate_;
};
} // namespace impala
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/be/src/runtime/coordinator.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/coordinator.cc b/be/src/runtime/coordinator.cc
index c9a166d..0a278ae 100644
--- a/be/src/runtime/coordinator.cc
+++ b/be/src/runtime/coordinator.cc
@@ -1090,7 +1090,7 @@ Status Coordinator::Wait() {
if (needs_finalization_) RETURN_IF_ERROR(FinalizeQuery());
query_profile_->AddInfoString(
- "Insert Stats", DataSink::OutputInsertStats(per_partition_status_, "\n"));
+ "DML Stats", DataSink::OutputDmlStats(per_partition_status_, "\n"));
// For DML queries, when Wait is done, the query is complete. Report aggregate
// query profiles at this point.
// TODO: make sure ReportQuerySummary gets called on error
@@ -1489,7 +1489,7 @@ Status Coordinator::UpdateFragmentExecStatus(const TReportExecStatusParams& para
if (partition.second.__isset.stats) {
if (!status->__isset.stats) status->__set_stats(TInsertStats());
- DataSink::MergeInsertStats(partition.second.stats, &status->stats);
+ DataSink::MergeDmlStats(partition.second.stats, &status->stats);
}
}
files_to_move_.insert(
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/common/thrift/ImpalaInternalService.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/ImpalaInternalService.thrift b/common/thrift/ImpalaInternalService.thrift
index 15c0020..d5a92ca 100644
--- a/common/thrift/ImpalaInternalService.thrift
+++ b/common/thrift/ImpalaInternalService.thrift
@@ -410,16 +410,28 @@ struct TParquetInsertStats {
1: required map<string, i64> per_column_size
}
-// Per partition insert stats
+struct TKuduDmlStats {
+ // The number of reported per-row errors, i.e. this many rows were not modified.
+ // Note that this aggregate is less useful than a breakdown of the number of errors by
+ // error type, e.g. number of rows with duplicate key conflicts, number of rows
+ // violating nullability constraints, etc., but it isn't possible yet to differentiate
+ // all error types in the KuduTableSink yet.
+ 1: optional i64 num_row_errors
+}
+
+// Per partition DML stats
// TODO: this should include the table stats that we update the metastore with.
+// TODO: Refactor to reflect usage by other DML statements.
struct TInsertStats {
1: required i64 bytes_written
2: optional TParquetInsertStats parquet_stats
+ 3: optional TKuduDmlStats kudu_stats
}
const string ROOT_PARTITION_KEY = ''
-// Per-partition statistics and metadata resulting from INSERT queries.
+// Per-partition statistics and metadata resulting from DML statements.
+// TODO: Refactor to reflect usage by other DML statements.
struct TInsertPartitionStatus {
// The id of the partition written to (may be -1 if the partition is created by this
// query). See THdfsTable.partitions.
@@ -434,13 +446,14 @@ struct TInsertPartitionStatus {
// Fully qualified URI to the base directory for this partition.
4: required string partition_base_dir
- // The latest observed Kudu timestamp reported by the KuduSession at this partition.
+ // The latest observed Kudu timestamp reported by the local KuduSession.
// This value is an unsigned int64.
5: optional i64 kudu_latest_observed_ts
}
-// The results of an INSERT query, sent to the coordinator as part of
+// The results of a DML statement, sent to the coordinator as part of
// TReportExecStatusParams
+// TODO: Refactor to reflect usage by other DML statements.
struct TInsertExecStatus {
// A map from temporary absolute file path to final absolute destination. The
// coordinator performs these updates after the query completes.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/common/thrift/generate_error_codes.py
----------------------------------------------------------------------
diff --git a/common/thrift/generate_error_codes.py b/common/thrift/generate_error_codes.py
index e9df146..c96cfc8 100755
--- a/common/thrift/generate_error_codes.py
+++ b/common/thrift/generate_error_codes.py
@@ -295,7 +295,7 @@ error_codes = (
("KUDU_KEY_ALREADY_PRESENT", 95, "Key already present in Kudu table '$0'."),
- ("KUDU_KEY_NOT_FOUND", 96, "Key not found in Kudu table '$0'."),
+ ("KUDU_NOT_FOUND", 96, "Not found in Kudu table '$0': $1"),
("KUDU_SESSION_ERROR", 97, "Error in Kudu table '$0': $1"),
@@ -303,6 +303,9 @@ error_codes = (
("AVRO_INVALID_DECIMAL", 99,
"Column '$0': invalid Avro decimal type with precision = '$1' scale = '$2'"),
+
+ ("KUDU_NULL_CONSTRAINT_VIOLATION", 100,
+ "Row with null value violates nullability constraint on table '$0'."),
)
import sys
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cfac09de/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
index 761db13..1ecfdf5 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
@@ -31,14 +31,16 @@ insert into tdata values
---- RESULTS
: 3
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 3.*
+NumModifiedRows: 3
+NumRowErrors: 0
====
---- QUERY
update tdata set vali=43 where id = 1
---- RESULTS
# TODO: Verify row count after fixing IMPALA-3713 (Here and UPDATE/DELETE below)
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -56,7 +58,8 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
update tdata set valv=cast('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' as varchar(20)) where id = 1
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -71,7 +74,8 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
update tdata set valb=false where id = 1
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -86,7 +90,8 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
update tdata set vali=43 where id > 1
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 2.*
+NumModifiedRows: 2
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -101,7 +106,8 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
update tdata set name='unknown' where name = 'martin'
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -119,7 +125,8 @@ insert into tdata values
---- RESULTS
: 2
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 2.*
+NumModifiedRows: 2
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -136,7 +143,8 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
update tdata set name=null where id = 40
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -152,7 +160,8 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
---- QUERY
update tdata set name='he' where id = 40
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
---- RESULTS
====
---- QUERY
@@ -173,7 +182,8 @@ insert into tdata values (320, '', 2.0, 932, cast('' as string), false)
---- RESULTS
: 1
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select id, name, valv, valb from tdata where id = 320;
@@ -194,7 +204,8 @@ insert into ignore_column_case values (1, 'Martin', 1.0, 10);
---- RESULTS
: 1
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
select ID, nAmE, VALF, VALI from ignore_column_case where NaMe = 'Martin';
@@ -209,7 +220,8 @@ insert into tdata values
---- RESULTS
: 1
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 0
====
---- QUERY
insert into tdata values
@@ -217,28 +229,23 @@ insert into tdata values
---- RESULTS
: 0
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 0.*
+NumModifiedRows: 0
+NumRowErrors: 1
====
---- QUERY
-- Updating the same record many times: cross join produces 7 identical updates
update a set a.name='Satan' from tdata a, tdata b where a.id = 666
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 7.*
-====
----- QUERY
--- Does not exercise any error path in the sink because updating the same record multiple
--- times is valid.
-update a set a.name='Satan' from tdata a, tdata b where a.id = 666
----- RESULTS
----- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 7.*
+NumModifiedRows: 7
+NumRowErrors: 0
====
---- QUERY
delete a from tdata a, tdata b where a.id = 666
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 1.*
+NumModifiedRows: 1
+NumRowErrors: 6
====
---- QUERY
select * from tdata
@@ -256,6 +263,9 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
upsert into table tdata values (40, 'they', 1, 43, cast('e' as VARCHAR(20)), false),
(1, NULL, 1, 0, cast('a' as VARCHAR(20)), true)
---- RESULTS
+---- RUNTIME_PROFILE
+NumModifiedRows: 2
+NumRowErrors: 0
====
---- QUERY
select * from tdata
@@ -306,8 +316,9 @@ INT,STRING,FLOAT,BIGINT,STRING,BOOLEAN
====
---- QUERY
upsert into table tdata (id, name) values (null, '')
----- CATCH
-Could not add Kudu WriteOp.: Invalid argument: column not nullable: id[int32 NOT NULL]
+---- RUNTIME_PROFILE
+NumModifiedRows: 0
+NumRowErrors: 1
====
---- QUERY
# IMPALA-3454: A delete that requires a rewrite may not get the Kudu column order correct
@@ -328,7 +339,8 @@ insert into impala_3454 values
delete from impala_3454 where key_1 < (select max(key_2) from impala_3454)
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 2.*
+NumModifiedRows: 2
+NumRowErrors: 0
====
---- QUERY
select * from impala_3454
@@ -345,7 +357,8 @@ SELECT * FROM functional_kudu.alltypes WHERE id < 100;
---- RESULTS
'Inserted 100 row(s)'
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 100.*
+NumModifiedRows: 100
+NumRowErrors: 0
====
---- QUERY
INSERT INTO kudu_test_tbl
@@ -353,7 +366,8 @@ SELECT * FROM functional_kudu.alltypes WHERE id < 100;
---- RESULTS
: 0
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 0.*
+NumModifiedRows: 0
+NumRowErrors: 100
====
---- QUERY
INSERT INTO kudu_test_tbl
@@ -361,42 +375,53 @@ SELECT * FROM functional_kudu.alltypes;
---- RESULTS
: 7200
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 7200.*
+NumModifiedRows: 7200
+NumRowErrors: 100
====
---- QUERY
# Test a larger UPDATE
UPDATE kudu_test_tbl SET int_col = -1;
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 7300.*
+NumModifiedRows: 7300
+NumRowErrors: 0
====
---- QUERY
# Test a larger DELETE
DELETE FROM kudu_test_tbl WHERE id > -1;
---- RESULTS
---- RUNTIME_PROFILE
-row_regex: .*NumModifiedRows: 7300.*
+NumModifiedRows: 7300
+NumRowErrors: 0
====
---- QUERY
# Insert rows that are not covered by any of the existing range partitions
+# Only the row at 10000 is inserted.
INSERT INTO kudu_test_tbl SELECT cast(id + 10000 as int), bool_col, tinyint_col,
smallint_col, int_col, bigint_col, float_col, double_col, date_string_col, string_col,
timestamp_col, year, month
FROM functional_kudu.alltypes
----- CATCH
-Kudu error(s) reported, first error: Not found: No tablet covering the requested range partition: NonCoveredRange { lower_bound: (int32 id=10001), upper_bound: (<end>)
+---- RUNTIME_PROFILE
+NumModifiedRows: 1
+NumRowErrors: 7299
====
---- QUERY
# Try to delete a row with a primary key value that is not covered by the existing range
-# partitions
+# partitions. This doesn't actually end up selecting any rows to modify.
DELETE FROM kudu_test_tbl WHERE id = 10001
---- RESULTS
+---- RUNTIME_PROFILE
+NumModifiedRows: 0
+NumRowErrors: 0
====
---- QUERY
# Try to update a row with a primary key value that is not covered by the existing range
-# partitions
+# partitions. This doesn't actually end up selecting any rows to modify.
UPDATE kudu_test_tbl SET int_col = 10 WHERE id = 10001
---- RESULTS
+---- RUNTIME_PROFILE
+NumModifiedRows: 0
+NumRowErrors: 0
====
---- QUERY
# IMPALA-2521: clustered insert into table.
@@ -414,6 +439,9 @@ from functional_kudu.alltypessmall group by id, name
) as sub;
---- RESULTS
: 10
+---- RUNTIME_PROFILE
+NumModifiedRows: 10
+NumRowErrors: 0
====
---- QUERY
select * from impala_2521
@@ -442,7 +470,10 @@ create table allkeytypes (i1 tinyint, i2 smallint, i3 int, i4 bigint, name strin
---- QUERY
insert into allkeytypes select cast(id as tinyint), smallint_col, int_col,
cast (bigint_col/10 as bigint), string_col, float_col, double_col
- from functional.alltypes where id > 0 and id < 4
+ from functional.alltypes where id > 0 and id < 10
---- RESULTS
: 3
+---- RUNTIME_PROFILE
+NumModifiedRows: 3
+NumRowErrors: 6
====