hawq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From odiache...@apache.org
Subject incubator-hawq git commit: HAWQ-1526. Added support for functions in WHERE clause for PXF tables.
Date Fri, 22 Sep 2017 18:27:01 GMT
Repository: incubator-hawq
Updated Branches:
  refs/heads/master be4af7785 -> b282aef2e


HAWQ-1526. Added support for functions in WHERE clause for PXF tables.


Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/b282aef2
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/b282aef2
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/b282aef2

Branch: refs/heads/master
Commit: b282aef2e1a6105ef5fa1db340ad7e11aff72ba1
Parents: be4af77
Author: Oleksandr Diachenko <odiachenko@pivotal.io>
Authored: Fri Sep 22 11:26:49 2017 -0700
Committer: Oleksandr Diachenko <odiachenko@pivotal.io>
Committed: Fri Sep 22 11:26:49 2017 -0700

----------------------------------------------------------------------
 src/backend/access/external/pxffilters.c        | 110 ++++++++++++++----
 src/backend/access/external/pxfheaders.c        |   8 +-
 .../access/external/test/pxffilters_test.c      | 114 +++++++++++++++++--
 .../access/external/test/pxfheaders_test.c      |  35 ++++++
 src/include/access/pxffilters.h                 |   2 +-
 5 files changed, 237 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/b282aef2/src/backend/access/external/pxffilters.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/pxffilters.c b/src/backend/access/external/pxffilters.c
index f502e4e..7c1db56 100644
--- a/src/backend/access/external/pxffilters.c
+++ b/src/backend/access/external/pxffilters.c
@@ -44,6 +44,7 @@ static void scalar_const_to_str(Const *constval, StringInfo buf);
 static void list_const_to_str(Const *constval, StringInfo buf);
 static List* append_attr_from_var(Var* var, List* attrs);
 static void enrich_trivial_expression(List *expressionItems);
+static List* get_attrs_from_expr(Expr *expr, bool* expressionIsSupported);
 
 /*
  * All supported HAWQ operators, and their respective HDFS operator code.
@@ -851,12 +852,51 @@ append_attr_from_var(Var* var, List* attrs)
 	return attrs;
 }
 
+/*
+ * append_attr_from_func_args
+ *
+ * extracts all columns from FuncExpr into attrs
+ * assigns false to expressionIsSupported if at least one of items is not supported
+ */
+static List*
+append_attr_from_func_args(FuncExpr *expr, List* attrs, bool* expressionIsSupported) {
+	if (!expressionIsSupported) {
+		return NIL;
+	}
+	ListCell *lc = NULL;
+	foreach (lc, expr->args)
+	{
+		Node *node = (Node *) lfirst(lc);
+		if (IsA(node, FuncExpr)) {
+			attrs = append_attr_from_func_args((FuncExpr *) node, attrs, expressionIsSupported);
+		} else if (IsA(node, Var)) {
+			attrs = append_attr_from_var((Var *) node, attrs);
+		} else if (IsA(node, OpExpr)) {
+			attrs = get_attrs_from_expr((OpExpr *) node, expressionIsSupported);
+		} else {
+			*expressionIsSupported = false;
+			return NIL;
+		}
+	}
+
+	return attrs;
+
+}
+
+/*
+ * get_attrs_from_expr
+ *
+ * extracts and returns list of all columns from Expr
+ * assigns false to expressionIsSupported if at least one of items is not supported
+ */
 static List*
-get_attrs_from_expr(Expr *expr)
+get_attrs_from_expr(Expr *expr, bool* expressionIsSupported)
 {
-	Node	*leftop 	= NULL;
-	Node	*rightop	= NULL;
-	List	*attrs = NIL;
+	Node *leftop = NULL;
+	Node *rightop = NULL;
+	List *attrs = NIL;
+
+	*expressionIsSupported = true;
 
 	if ((!expr))
 		return attrs;
@@ -870,30 +910,46 @@ get_attrs_from_expr(Expr *expr)
 		ScalarArrayOpExpr *saop = (ScalarArrayOpExpr *) expr;
 		leftop = (Node *) linitial(saop->args);
 		rightop = (Node *) lsecond(saop->args);
+	} else {
+		// If expression type is not known, report that it's not supported
+		*expressionIsSupported = false;
+		return NIL;
 	}
 
-	//Process left operand
-	//For most of datatypes column is represented by Var node
-	if (IsA(leftop, Var))
+	// We support following combinations of operands:
+	// Var, Const
+	// Relabel, Const
+	// FuncExpr, Const
+	// Const, Var
+	// Const, Relabel
+	// Const, FuncExpr
+	// For most of datatypes column is represented by Var node
+	// For varchar column is represented by RelabelType node
+	if (IsA(leftop, Var) && IsA(rightop, Const))
 	{
 		attrs = append_attr_from_var((Var *) leftop, attrs);
-	}
-	//For varchar column is represented by RelabelType node
-	if (IsA(leftop, RelabelType))
+	} else if (IsA(leftop, RelabelType) && IsA(rightop, Const))
 	{
 		attrs = append_attr_from_var((Var *) ((RelabelType *) leftop)->arg, attrs);
+	} else if (IsA(leftop, FuncExpr) && IsA(rightop, Const)) {
+		FuncExpr *expr = (FuncExpr *) leftop;
+		attrs = append_attr_from_func_args(expr, attrs, expressionIsSupported);
 	}
-
-	//Process right operand
-	//For most of datatypes column is represented by Var node
-	if (IsA(rightop, Var))
+	else if (IsA(rightop, Var) && IsA(leftop, Const))
 	{
 		attrs = append_attr_from_var((Var *) rightop, attrs);
-	}
-	//For varchar column is represented by RelabelType node
-	if (IsA(rightop, RelabelType))
+	} else if (IsA(rightop, RelabelType) && IsA(leftop, Const))
 	{
 		attrs = append_attr_from_var((Var *) ((RelabelType *) rightop)->arg, attrs);
+	} else if (IsA(rightop, FuncExpr) && IsA(leftop, Const)) {
+		FuncExpr *expr = (FuncExpr *) rightop;
+		attrs = append_attr_from_func_args(expr, attrs, expressionIsSupported);
+	}
+	else {
+		// If operand type or combination is not known, report that it's not supported
+		// to avoid partially extracted attributes from expression
+		*expressionIsSupported = false;
+		return NIL;
 	}
 
 	return attrs;
@@ -1251,11 +1307,16 @@ void enrich_trivial_expression(List *expressionItems) {
  * List might contain duplicates.
  * Caller should release memory once result is not needed.
  */
-List* extractPxfAttributes(List* quals)
+List* extractPxfAttributes(List* quals, bool* qualsAreSupported)
 {
 
+	if (!(*qualsAreSupported)) {
+		return NIL;
+	}
 	ListCell		*lc = NULL;
 	List *attributes = NIL;
+	bool expressionIsSupported;
+	*qualsAreSupported = true;
 
 	if (list_length(quals) == 0)
 		return NIL;
@@ -1271,14 +1332,23 @@ List* extractPxfAttributes(List* quals)
 			case T_ScalarArrayOpExpr:
 			{
 				Expr* expr = (Expr *) node;
-				List			*attrs = get_attrs_from_expr(expr);
+				List			*attrs = get_attrs_from_expr(expr, &expressionIsSupported);
+				if (!expressionIsSupported) {
+					*qualsAreSupported = false;
+					return NIL;
+				}
 				attributes = list_concat(attributes, attrs);
 				break;
 			}
 			case T_BoolExpr:
 			{
 				BoolExpr* expr = (BoolExpr *) node;
-				List *inner_result = extractPxfAttributes(expr->args);
+				bool innerBoolQualsAreSupported = true;
+				List *inner_result = extractPxfAttributes(expr->args, &innerBoolQualsAreSupported);
+				if (!innerBoolQualsAreSupported) {
+					*qualsAreSupported = false;
+					return NIL;
+				}
 				attributes = list_concat(attributes, inner_result);
 				break;
 			}

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/b282aef2/src/backend/access/external/pxfheaders.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/pxfheaders.c b/src/backend/access/external/pxfheaders.c
index d8904b4..395fe25 100644
--- a/src/backend/access/external/pxfheaders.c
+++ b/src/backend/access/external/pxfheaders.c
@@ -65,14 +65,18 @@ void build_http_header(PxfInputData *input)
 	
 	if (proj_info != NULL && proj_info->pi_isVarList)
 	{
-		List* qualsAttributes = extractPxfAttributes(input->quals);
+		bool qualsAreSupported = true;
+		List* qualsAttributes = extractPxfAttributes(input->quals, &qualsAreSupported);
 		/* projection information is incomplete if columns from WHERE clause wasn't extracted */
-		if (qualsAttributes !=  NIL || list_length(input->quals) == 0)
+		/* if any of expressions in WHERE clause is not supported - do not send any projection
information at all*/
+		if (qualsAreSupported && (qualsAttributes !=  NIL || list_length(input->quals)
== 0))
 		{
 			add_projection_desc_httpheader(headers, proj_info, qualsAttributes);
 		}
 		else
+		{
 			elog(DEBUG2, "Query will not be optimized to use projection information");
+		}
 	}
 
 	/* GP cluster configuration */

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/b282aef2/src/backend/access/external/test/pxffilters_test.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/test/pxffilters_test.c b/src/backend/access/external/test/pxffilters_test.c
index 5d38e04..8e28890 100644
--- a/src/backend/access/external/test/pxffilters_test.c
+++ b/src/backend/access/external/test/pxffilters_test.c
@@ -30,6 +30,10 @@ void run__scalar_const_to_str__negative(Const* input, StringInfo result,
char* v
 void run__list_const_to_str(Const* input, StringInfo result, char* expected);
 void run__list_const_to_str__negative(Const* input, StringInfo result, int len, Datum *dats);
 
+
+/* date_eq(date, date) => bool */
+#define DateEqualFuncId 1086
+
 void
 test__supported_filter_type(void **state)
 {
@@ -541,13 +545,13 @@ Var* build_var(Oid oid, int attno) {
 	return arg_var;
 }
 
-Const* build_const(Oid oid, char* value)
+Const* build_const(Oid oid, char* value, bool expectToBeRead)
 {
 	Const* arg_const = (Const*) palloc0(sizeof(Const));
 	arg_const->xpr.type = T_Const;
 	arg_const->constisnull = (value == NULL);
 	arg_const->consttype = oid;
-	if (value != NULL)
+	if (value != NULL && expectToBeRead)
 	{
 		mock__scalar_const_to_str(oid, value);
 	}
@@ -594,7 +598,7 @@ ExpressionItem* build_expression_item(int lattnum, Oid lattrtype, char*
rconstst
 {
 	ExpressionItem *expressionItem = (ExpressionItem*) palloc0(sizeof(ExpressionItem));
 	Var *leftop = build_var(lattrtype, lattnum);
-	Const *rightop = build_const(rattrtype, strdup(rconststr));
+	Const *rightop = build_const(rattrtype, strdup(rconststr), true);
 	OpExpr *operationExpression = build_op_expr(leftop, rightop, op);
 
 	expressionItem->node = operationExpression;
@@ -604,12 +608,44 @@ ExpressionItem* build_expression_item(int lattnum, Oid lattrtype, char*
rconstst
 	return expressionItem;
 }
 
+FuncExpr* build_func_expr_operand(List *args, CoercionForm funcformat) {
+	FuncExpr* operand = palloc0(sizeof(FuncExpr));
+	((Node*) operand)->type = T_FuncExpr;
+	operand->args = args;
+	operand->funcformat = funcformat;
+	return operand;
+}
+
+/**
+ * builds FuncExpr for a following expression: function(column1, column2,...,columnk)
+ * columnOids - oids of column1, column2, ... types
+ *
+ * Typical representation of a functions we support:
+ * COERCE_EXPLICIT_CAST->COERCE_EXPLICIT_CALL->COERCE_IMPLICIT_CAST->T_Var,
+ * where Var holds actual arguments - column1, column2,...,columnk
+ */
+FuncExpr* build_nested_func_expr_operand(List *columnsOids, List *attrsIndices) {
+	assert_int_equal(columnsOids->length, attrsIndices->length);
+	ListCell *columnOidLc = NULL, *attrIndexLc = NULL;
+	Var *var = NULL;
+	List* args = NIL;
+	forboth(columnOidLc, columnsOids, attrIndexLc, attrsIndices) {
+		var = build_var(lfirst(columnOidLc), lfirst(attrIndexLc));
+		args = lappend(args, var);
+	}
+	FuncExpr* operandImplicitCast = build_func_expr_operand(args, COERCE_IMPLICIT_CAST);
+	FuncExpr* operandExplicitCall = build_func_expr_operand(list_make1(operandImplicitCast),
COERCE_EXPLICIT_CALL);
+	FuncExpr* operandExplicitCast = build_func_expr_operand(list_make1(operandExplicitCall),
COERCE_EXPLICIT_CALL);
+
+	return operandExplicitCast;
+}
+
 void run__opexpr_to_pxffilter__positive(Oid dbop, PxfOperatorCode expectedPxfOp)
 {
 	PxfFilterDesc *filter = (PxfFilterDesc*) palloc0(sizeof(PxfFilterDesc));
 	Var *arg_var = build_var(INT2OID, 1);
 	char* const_value = strdup("1984"); /* will be free'd by const_to_str */
-	Const* arg_const = build_const(INT2OID, const_value);
+	Const* arg_const = build_const(INT2OID, const_value, true);
 
 	OpExpr *expr = build_op_expr(arg_var, arg_const, dbop);
 	PxfFilterDesc* expected = build_filter(
@@ -657,7 +693,8 @@ test__opexpr_to_pxffilter__attributeEqualsNull(void **state)
 {
 	PxfFilterDesc *filter = (PxfFilterDesc*) palloc0(sizeof(PxfFilterDesc));
 	Var *arg_var = build_var(INT2OID, 1);
-	Const* arg_const = build_const(INT2OID, NULL);
+	Const* arg_const = build_const(INT2OID, NULL, true
+			);
 	OpExpr *expr = build_op_expr(arg_var, arg_const, 94 /* int2eq */);
 
 	PxfFilterDesc* expected = build_filter(
@@ -698,7 +735,7 @@ test__opexpr_to_pxffilter__differentTypes(void **state)
 	PxfFilterDesc *filter = (PxfFilterDesc*) palloc0(sizeof(PxfFilterDesc));
 	Var *arg_var = build_var(INT2OID, 3);
 	char* const_value = strdup("13"); /* will be free'd by const_to_str */
-	Const *arg_const = build_const(INT8OID, const_value);
+	Const *arg_const = build_const(INT8OID, const_value, true);
 	OpExpr *expr = build_op_expr(arg_const, arg_var, 1864 /* int28lt */);
 
 
@@ -720,7 +757,7 @@ test__opexpr_to_pxffilter__unsupportedTypeCircle(void **state)
 {
 	PxfFilterDesc *filter = (PxfFilterDesc*) palloc0(sizeof(PxfFilterDesc));
 	Var *arg_var = build_var(CIRCLEOID, 8);
-	Const *arg_const = build_const(CIRCLEOID, NULL);
+	Const *arg_const = build_const(CIRCLEOID, NULL, true);
 	OpExpr *expr = build_op_expr(arg_const, arg_var, 0 /* whatever */);
 
 	/* run test */
@@ -754,7 +791,7 @@ test__opexpr_to_pxffilter__unsupportedOpNot(void **state)
 {
 	PxfFilterDesc *filter = (PxfFilterDesc*) palloc0(sizeof(PxfFilterDesc));
 	Var *arg_var = build_var(INT2OID, 3);
-	Const *arg_const = build_const(INT2OID, NULL);
+	Const *arg_const = build_const(INT2OID, NULL, true);
 	OpExpr *expr = build_op_expr(arg_const, arg_var, 1877 /* int2not */);
 
 	/* run test */
@@ -837,6 +874,63 @@ test__pxf_serialize_filter_list__manyFilters(void **state)
 	expressionItems = NIL;
 }
 
+void
+test__extractPxfAttributes_empty_quals(void **state)
+{
+	bool qualsAreSupported;
+	qualsAreSupported = true;
+	List* quals = NIL;
+	extractPxfAttributes(quals, &qualsAreSupported);
+	assert_true(qualsAreSupported);
+}
+
+/**
+ * covers queries like:
+ * SELECT a,b,c
+ * FROM tab1
+ * WHERE d = function(e) = const
+ *
+ * d is a column number 5 of int4 datatype
+ * e is a column number 7 of date datatype
+ * const is a int4 datatype
+ *
+ */
+void
+test__extractPxfAttributes_supported_function_one_arg(void **state) {
+
+	int argumentColumnIndex = 6; // index starts from 0
+	bool qualsAreSupported;
+	qualsAreSupported = true;
+	List* columnsOids = list_make1(DATEOID);
+	List* attrsIndices = list_make1(argumentColumnIndex);
+
+	// Create operands FuncExpr, Const
+	FuncExpr* leftop = build_nested_func_expr_operand(columnsOids, attrsIndices);
+
+	// extractPxfAttributes just extracts columns, doesn't read values of constants
+	Node* rightop = build_const(INT4OID, strdup("42"), false);
+	OpExpr* opExpr = build_op_expr(leftop, rightop, Int4EqualOperator);
+	List* quals = list_make1(opExpr);
+
+	// Extract columns(attributes) from WHERE clause
+	List* attrs = extractPxfAttributes(quals, &qualsAreSupported);
+
+	// Make sure we extracted one attribute, column e
+	assert_int_equal(1, attrs->length);
+
+	// Make sure it's supported
+	assert_true(qualsAreSupported);
+
+	ListCell* lc = NULL;
+	int attnum;
+	foreach(lc, attrs)
+	{
+		attnum = lfirst_int(lc);
+		// Index of attnum starts from 0
+		assert_int_equal(attnum + 1, argumentColumnIndex);
+	}
+}
+
 int 
 main(int argc, char* argv[]) 
 {
@@ -865,7 +959,9 @@ main(int argc, char* argv[])
 			unit_test(test__opexpr_to_pxffilter__attributeIsNull),
 			unit_test(test__pxf_serialize_filter_list__oneFilter),
 			unit_test(test__pxf_serialize_fillter_list__nullFilter),
-			unit_test(test__pxf_serialize_filter_list__manyFilters)
+			unit_test(test__pxf_serialize_filter_list__manyFilters),
+			unit_test(test__extractPxfAttributes_empty_quals),
+			unit_test(test__extractPxfAttributes_supported_function_one_arg)
 	};
 	return run_tests(tests);
 }

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/b282aef2/src/backend/access/external/test/pxfheaders_test.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/test/pxfheaders_test.c b/src/backend/access/external/test/pxfheaders_test.c
index 851483d..35ac7e3 100644
--- a/src/backend/access/external/test/pxfheaders_test.c
+++ b/src/backend/access/external/test/pxfheaders_test.c
@@ -125,6 +125,39 @@ test__build_http_header__remote_credentials_are_not_null(void **state)
 	build_http_header(input_data);
 }
 
+/**
+ * Query has valid projection info,
+ * but some of expression in WHERE clause is not supported
+ * Make sure we are not sending any projection information at all,
+ * to avoid incorrect results.
+ */
+void test__build_http_header__where_is_not_supported(void **state)
+{
+
+	expect_external_vars();
+
+	expect_any(extractPxfAttributes, quals);
+	expect_any(extractPxfAttributes, qualsAreSupported);
+	will_return(extractPxfAttributes, NIL);
+	will_assign_value(extractPxfAttributes, qualsAreSupported, false);
+
+	expect_churl_headers("X-GP-SEGMENT-ID", mock_extvar->GP_SEGMENT_ID);
+	expect_churl_headers("X-GP-SEGMENT-COUNT", mock_extvar->GP_SEGMENT_COUNT);
+	expect_churl_headers("X-GP-XID", mock_extvar->GP_XID);
+	expect_churl_headers_alignment();
+	expect_churl_headers("X-GP-URL-HOST", gphd_uri->host);
+	expect_churl_headers("X-GP-URL-PORT", gphd_uri->port);
+	expect_churl_headers("X-GP-DATA-DIR", gphd_uri->data);
+	expect_churl_headers("X-GP-URI", gphd_uri->uri);
+	expect_churl_headers("X-GP-HAS-FILTER", "0");
+
+	input_data->proj_info = palloc0(sizeof(ProjectionInfo));
+	input_data->proj_info->pi_isVarList = true;
+	input_data->quals = list_make1("Some not supported quals");
+
+	build_http_header(input_data);
+}
+
 void
 test__get_format_name(void **state)
 {
@@ -262,6 +295,8 @@ main(int argc, char* argv[])
 		unit_test_setup_teardown(test__build_http_header__remote_credentials_are_not_null, 
 								 common_setup, common_teardown),
 		unit_test_setup_teardown(test__get_format_name,
+								 common_setup, common_teardown),
+		unit_test_setup_teardown(test__build_http_header__where_is_not_supported,
 								 common_setup, common_teardown)
 	};
 

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/b282aef2/src/include/access/pxffilters.h
----------------------------------------------------------------------
diff --git a/src/include/access/pxffilters.h b/src/include/access/pxffilters.h
index c7e8aa8..206adce 100644
--- a/src/include/access/pxffilters.h
+++ b/src/include/access/pxffilters.h
@@ -137,6 +137,6 @@ static inline bool pxfoperand_is_list_const(PxfOperand x)
 }
 
 char *serializePxfFilterQuals(List *quals);
-List* extractPxfAttributes(List* quals);
+List* extractPxfAttributes(List* quals, bool* qualsAreSupported);
 
 #endif // _PXF_FILTERS_H_


Mime
View raw message