Add Nullif case for eval_const_expressions_mutator
Hi
I notice that there are no Nullif case in eval_const_expression.
Since Nullif is similar to Opexpr and is easy to implement,
I try add this case in eval_const_expressions_mutator.
Best regards,
houzj
Attachments:
0001-patch-eval-NULLIF.patchapplication/octet-stream; name=0001-patch-eval-NULLIF.patchDownload
From 888dcb811bcdd6ed6a576ebd65f851b750da3937 Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Tue, 10 Nov 2020 14:13:22 -0500
Subject: [PATCH] OSS patch eval NULLIF
---
src/backend/optimizer/util/clauses.c | 70 ++++++++++++++++++++++++++++++++++++
src/test/regress/expected/case.out | 24 +++++++++++++
src/test/regress/sql/case.sql | 9 +++++
3 files changed, 103 insertions(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 65534ed..d83ce24 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2596,6 +2596,76 @@ eval_const_expressions_mutator(Node *node,
newexpr->location = expr->location;
return (Node *) newexpr;
}
+ case T_NullIfExpr:
+ {
+ NullIfExpr *expr = (NullIfExpr *) node;
+ List *args = expr->args;
+ ListCell *arg;
+ Expr *simple;
+ NullIfExpr *newexpr;
+
+ /* Recursively simplify the args */
+ args = (List *) expression_tree_mutator((Node *) expr->args,
+ eval_const_expressions_mutator,
+ (void *) context);
+
+ /* If either argument is NULL they can't be equal */
+ foreach(arg, args)
+ {
+ if (IsA(lfirst(arg), Const) && ((Const *) lfirst(arg))->constisnull)
+ {
+ return (Node *) linitial(args);
+ }
+ }
+
+ /*
+ * Need to get OID of underlying function. Okay to scribble
+ * on input to this extent.
+ */
+ set_opfuncid(expr);
+
+ /*
+ * Code for op/func reduction is pretty bulky, so split it out
+ * as a separate function.
+ */
+ simple = simplify_function(expr->opfuncid,
+ BOOLOID, -1,
+ expr->opcollid,
+ expr->inputcollid,
+ &args,
+ false,
+ false,
+ true,
+ context);
+
+ /* successfully simplified it */
+ if (simple && IsA(simple, Const))
+ {
+ /* if the arguments are equal return null */
+ if(DatumGetBool(((Const *) simple)->constvalue))
+ return (Node *) makeNullConst(exprType(node),
+ exprTypmod(node),
+ exprCollation(node));
+ else
+ return (Node *) linitial(args);
+ }
+
+ /*
+ * The expression cannot be simplified any further, so build
+ * and return a replacement NullIfExpr node using the
+ * possibly-simplified arguments.
+ */
+ newexpr = makeNode(NullIfExpr);
+ newexpr->opno = expr->opno;
+ newexpr->opfuncid = expr->opfuncid;
+ newexpr->opresulttype = expr->opresulttype;
+ newexpr->opretset = expr->opretset;
+ newexpr->opcollid = expr->opcollid;
+ newexpr->inputcollid = expr->inputcollid;
+ newexpr->args = args;
+ newexpr->location = expr->location;
+ return (Node *) newexpr;
+ }
case T_DistinctExpr:
{
DistinctExpr *expr = (DistinctExpr *) node;
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index c0c8acf..130b9f0 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -263,6 +263,30 @@ SELECT '' AS "Two", *
| 4 | | 2 | -4
(2 rows)
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
--
-- Examples of updates involving tables
--
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 17436c5..c328f34 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -137,6 +137,15 @@ SELECT '' AS "Two", *
FROM CASE_TBL a, CASE2_TBL b
WHERE COALESCE(f,b.i) = 2;
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+
--
-- Examples of updates involving tables
--
--
2.7.2.windows.1
"Hou, Zhijie" <houzj.fnst@cn.fujitsu.com> writes:
I notice that there are no Nullif case in eval_const_expression.
Since Nullif is similar to Opexpr and is easy to implement,
I try add this case in eval_const_expressions_mutator.
I think this patch should be about a tenth the size. Try modeling
it on the T_SubscriptingRef-etc case, ie, use ece_generic_processing
and then ece_evaluate_expr to cover the generic cases. OpExpr is
common enough to deserve specially optimized code, but NullIf isn't,
so shorter is better.
regards, tom lane
I notice that there are no Nullif case in eval_const_expression.
Since Nullif is similar to Opexpr and is easy to implement, I try add
this case in eval_const_expressions_mutator.I think this patch should be about a tenth the size. Try modeling it on
the T_SubscriptingRef-etc case, ie, use ece_generic_processing and then
ece_evaluate_expr to cover the generic cases. OpExpr is common enough to
deserve specially optimized code, but NullIf isn't, so shorter is better.
Thanks for the review.
Attaching v2 patch , which followed the suggestion
to use ece_generic_processing and ece_evaluate_expr to simplify the code.
Please have a check.
Best regards,
houzj
Attachments:
v2-0001-add-nullif-case-for-eval_const_expressions.patchapplication/octet-stream; name=v2-0001-add-nullif-case-for-eval_const_expressions.patchDownload
From f818f8b6d65b0756fc1166e4c8c8fe663df23222 Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Mon, 11 Jan 2021 22:47:12 -0500
Subject: [PATCH] add nullif case for eval_const_expressions
---
src/backend/optimizer/util/clauses.c | 38 ++++++++++++++++++++++++++++++++++++
src/test/regress/expected/case.out | 24 +++++++++++++++++++++++
src/test/regress/sql/case.sql | 9 +++++++++
3 files changed, 71 insertions(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 51d26a0..b5a2f00 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2329,6 +2329,44 @@ eval_const_expressions_mutator(Node *node,
newexpr->location = expr->location;
return (Node *) newexpr;
}
+ case T_NullIfExpr:
+ {
+ NullIfExpr *expr = (NullIfExpr *) node;
+ NullIfExpr *newexpr;
+ ListCell *arg;
+ List *args;
+ bool has_nonconst_input = false;
+
+ args = (List *) ece_generic_processing(expr->args);
+
+ /* If either argument is NULL they can't be equal */
+ foreach(arg, args)
+ {
+ if (!IsA(lfirst(arg), Const))
+ has_nonconst_input = true;
+ else if (((Const *) lfirst(arg))->constisnull)
+ return (Node *) linitial(args);
+ }
+
+ if(!has_nonconst_input)
+ return ece_evaluate_expr(expr);
+
+ /*
+ * The expression cannot be simplified any further, so build
+ * and return a replacement NullIfExpr node using the
+ * possibly-simplified arguments.
+ */
+ newexpr = makeNode(NullIfExpr);
+ newexpr->opno = expr->opno;
+ newexpr->opfuncid = expr->opfuncid;
+ newexpr->opresulttype = expr->opresulttype;
+ newexpr->opretset = expr->opretset;
+ newexpr->opcollid = expr->opcollid;
+ newexpr->inputcollid = expr->inputcollid;
+ newexpr->args = args;
+ newexpr->location = expr->location;
+ return (Node *) newexpr;
+ }
case T_DistinctExpr:
{
DistinctExpr *expr = (DistinctExpr *) node;
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index 7fcfe9a..2063c73 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -263,6 +263,30 @@ SELECT *
4 | | 2 | -4
(2 rows)
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
--
-- Examples of updates involving tables
--
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 0655d26..4742e1d 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -137,6 +137,15 @@ SELECT *
FROM CASE_TBL a, CASE2_TBL b
WHERE COALESCE(f,b.i) = 2;
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+
--
-- Examples of updates involving tables
--
--
1.8.3.1
I think this patch should be about a tenth the size. Try modeling it
on the T_SubscriptingRef-etc case, ie, use ece_generic_processing and
then ece_evaluate_expr to cover the generic cases. OpExpr is common
enough to deserve specially optimized code, but NullIf isn't, so shorteris better.
Thanks for the review.
Attaching v2 patch , which followed the suggestion to use
ece_generic_processing and ece_evaluate_expr to simplify the code.Please have a check.
Sorry, I found the code still be simplified better.
Attaching the new patch.
Best regards,
houzj
Attachments:
v2_1-0001-add-nullif-case-for-eval_const_expressions.patchapplication/octet-stream; name=v2_1-0001-add-nullif-case-for-eval_const_expressions.patchDownload
From 45a4d3aa352894386a88436048106c38335b94b2 Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Tue, 12 Jan 2021 01:39:50 -0500
Subject: [PATCH] add nullif case for eval_const_expressions
---
src/backend/optimizer/util/clauses.c | 23 +++++++++++++++++++++++
src/test/regress/expected/case.out | 24 ++++++++++++++++++++++++
src/test/regress/sql/case.sql | 9 +++++++++
3 files changed, 56 insertions(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 51d26a0..94e703e 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2329,6 +2329,29 @@ eval_const_expressions_mutator(Node *node,
newexpr->location = expr->location;
return (Node *) newexpr;
}
+ case T_NullIfExpr:
+ {
+ NullIfExpr *expr;
+ ListCell *arg;
+ bool has_nonconst_input = false;
+
+ /* Copy the node and const-simplify its arguments */
+ expr = (NullIfExpr *) ece_generic_processing(node);
+
+ /* If either argument is NULL they can't be equal */
+ foreach(arg, expr->args)
+ {
+ if (!IsA(lfirst(arg), Const))
+ has_nonconst_input = true;
+ else if (((Const *) lfirst(arg))->constisnull)
+ return (Node *) linitial(expr->args);
+ }
+
+ if(!has_nonconst_input)
+ return ece_evaluate_expr(expr);
+
+ return (Node *) expr;
+ }
case T_DistinctExpr:
{
DistinctExpr *expr = (DistinctExpr *) node;
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index 7fcfe9a..2063c73 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -263,6 +263,30 @@ SELECT *
4 | | 2 | -4
(2 rows)
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
--
-- Examples of updates involving tables
--
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 0655d26..4742e1d 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -137,6 +137,15 @@ SELECT *
FROM CASE_TBL a, CASE2_TBL b
WHERE COALESCE(f,b.i) = 2;
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+
--
-- Examples of updates involving tables
--
--
1.8.3.1
On 2021-01-12 07:43, Hou, Zhijie wrote:
I think this patch should be about a tenth the size. Try modeling it
on the T_SubscriptingRef-etc case, ie, use ece_generic_processing and
then ece_evaluate_expr to cover the generic cases. OpExpr is common
enough to deserve specially optimized code, but NullIf isn't, so shorteris better.
Thanks for the review.
Attaching v2 patch , which followed the suggestion to use
ece_generic_processing and ece_evaluate_expr to simplify the code.Please have a check.
Sorry, I found the code still be simplified better.
Attaching the new patch.
It's a bit unfortunate now that between OpExpr, DistinctExpr,
NullIfExpr, and to a lesser extent ScalarArrayOpExpr we will now have
several different implementations of nearly the same thing, without any
explanation why one approach was chosen here and another there. We
should at least document this.
Some inconsistencies I found: The code for DistinctExpr calls
expression_tree_mutator() directly, but your code for NullIfExpr calls
ece_generic_processing(), even though the explanation in the comment for
DistinctExpr would apply there as well.
Your code for NullIfExpr doesn't appear to call set_opfuncid() anywhere.
I would move your new block for NullIfExpr after the block for
DistinctExpr. That's the order in which these blocks appear elsewhere
for generic node processing.
Check your whitespace usage:
if(!has_nonconst_input)
should have a space after the "if". (It's easy to fix of course, but I
figure I'd point it out here since you have submitted several patches
with this style, so it's perhaps a habit to break.)
Perhaps add a comment to the tests like this so it's clear what they are
for:
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 4742e1d0e0..98e3fb8de5 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -137,6 +137,7 @@ CREATE TABLE CASE2_TBL (
FROM CASE_TBL a, CASE2_TBL b
WHERE COALESCE(f,b.i) = 2;+-- Tests for constant subexpression simplification
explain (costs off)
SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
Hi
Thanks for the review.
It's a bit unfortunate now that between OpExpr, DistinctExpr, NullIfExpr,
and to a lesser extent ScalarArrayOpExpr we will now have several different
implementations of nearly the same thing, without any explanation why one
approach was chosen here and another there. We should at least document
this.
I am not quiet sure where to document the difference.
Temporarily, I tried to add some comments for the Nullif to explain why this one is different.
+ /*
+ * Since NullIf is not common enough to deserve specially
+ * optimized code, use ece_generic_processing and
+ * ece_evaluate_expr to simplify the code as much as possible.
+ */Any suggestions ?
Some inconsistencies I found: The code for DistinctExpr calls
expression_tree_mutator() directly, but your code for NullIfExpr calls
ece_generic_processing(), even though the explanation in the comment for
DistinctExpr would apply there as well.Your code for NullIfExpr doesn't appear to call set_opfuncid() anywhere.
IMO, we will call set_opfuncid in function ece_evaluate_expr.
Like the following flow:
ece_evaluate_expr-->evaluate_expr--> fix_opfuncids--> fix_opfuncids_walker--> set_opfuncid
And we do not need the opfuncid till we call ece_evaluate_expr.
So, to simplify the code as much as possible, I did not call set_opfuncid in eval_const_expressions_mutator again.
I would move your new block for NullIfExpr after the block for DistinctExpr.
That's the order in which these blocks appear elsewhere for generic node
processing.
Changed.
Check your whitespace usage:
if(!has_nonconst_input)
should have a space after the "if". (It's easy to fix of course, but I
figure I'd point it out here since you have submitted several patches with
this style, so it's perhaps a habit to break.)
Changed.
Perhaps add a comment to the tests like this so it's clear what they are
for:diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql index 4742e1d0e0..98e3fb8de5 100644 --- a/src/test/regress/sql/case.sql +++ b/src/test/regress/sql/case.sql @@ -137,6 +137,7 @@ CREATE TABLE CASE2_TBL ( FROM CASE_TBL a, CASE2_TBL b WHERE COALESCE(f,b.i) = 2;+-- Tests for constant subexpression simplification
explain (costs off)
SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
Added.
Attatching v3 patch, please consider it for further review.
Best regards,
houzj
Attachments:
v3_0001-add-nullif-case-for-eval_const_expressions.patchapplication/octet-stream; name=v3_0001-add-nullif-case-for-eval_const_expressions.patchDownload
From 43005ebda2f694cd08dffd5674ffa4609ef5b8bc Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Tue, 19 Jan 2021 20:04:45 -0500
Subject: [PATCH] add nullif case for eval_const_expressions
---
src/backend/optimizer/util/clauses.c | 28 ++++++++++++++++++++++++++++
src/test/regress/expected/case.out | 25 +++++++++++++++++++++++++
src/test/regress/sql/case.sql | 11 +++++++++++
3 files changed, 64 insertions(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 51d26a0..924b294 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2430,6 +2430,34 @@ eval_const_expressions_mutator(Node *node,
newexpr->location = expr->location;
return (Node *) newexpr;
}
+ case T_NullIfExpr:
+ {
+ /*
+ * Since NullIf is not common enough to deserve specially
+ * optimized code, use ece_generic_processing and
+ * ece_evaluate_expr to simplify the code as much as possible.
+ */
+ NullIfExpr *expr;
+ ListCell *arg;
+ bool has_nonconst_input = false;
+
+ /* Copy the node and const-simplify its arguments */
+ expr = (NullIfExpr *) ece_generic_processing(node);
+
+ /* If either argument is NULL they can't be equal */
+ foreach(arg, expr->args)
+ {
+ if (!IsA(lfirst(arg), Const))
+ has_nonconst_input = true;
+ else if (((Const *) lfirst(arg))->constisnull)
+ return (Node *) linitial(expr->args);
+ }
+
+ if (!has_nonconst_input)
+ return ece_evaluate_expr(expr);
+
+ return (Node *) expr;
+ }
case T_ScalarArrayOpExpr:
{
ScalarArrayOpExpr *saop;
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index 7fcfe9a..f5136c1 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -263,6 +263,31 @@ SELECT *
4 | | 2 | -4
(2 rows)
+-- Tests for constant subexpression simplification
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
--
-- Examples of updates involving tables
--
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 0655d26..83fe43b 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -137,6 +137,17 @@ SELECT *
FROM CASE_TBL a, CASE2_TBL b
WHERE COALESCE(f,b.i) = 2;
+-- Tests for constant subexpression simplification
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+
--
-- Examples of updates involving tables
--
--
1.8.3.1
On 1/19/21 8:16 PM, Hou, Zhijie wrote:
Attatching v3 patch, please consider it for further review.
Peter, thoughts on the new patch in [1]/messages/by-id/ab53b3dbdbd6436f970f33b51ccd7dd3@G08CNEXMBPEKD05.g08.fujitsu.local?
--
-David
david@pgmasters.net
[1]: /messages/by-id/ab53b3dbdbd6436f970f33b51ccd7dd3@G08CNEXMBPEKD05.g08.fujitsu.local
/messages/by-id/ab53b3dbdbd6436f970f33b51ccd7dd3@G08CNEXMBPEKD05.g08.fujitsu.local
David Steele <david@pgmasters.net> writes:
Peter, thoughts on the new patch in [1]?
I'm not Peter, but I have a complaint about this bit:
+ if (!has_nonconst_input)
+ return ece_evaluate_expr(expr);
That's not okay without a further check to see if the comparison function
used by the node is immutable. Compare ScalarArrayOpExpr, for instance.
regards, tom lane
+ if (!has_nonconst_input)
+ return ece_evaluate_expr(expr);That's not okay without a further check to see if the comparison function used
by the node is immutable. Compare ScalarArrayOpExpr, for instance.
Thanks for pointing it out.
Attaching new patch with this change.
Best regards,
houzj
Attachments:
v4-0001-add-nullif-case-for-eval_const_expressions.patchapplication/octet-stream; name=v4-0001-add-nullif-case-for-eval_const_expressions.patchDownload
From 8b6e978f2f8654cd1f1fbd74caa87a672404269f Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.fnst@cn.fujitsu.com>
Date: Wed, 24 Mar 2021 14:42:10 +0800
Subject: [PATCH] add nullif case for eval_const_expressions
---
src/backend/optimizer/util/clauses.c | 35 +++++++++++++++++++++++++++++++++++
src/test/regress/expected/case.out | 25 +++++++++++++++++++++++++
src/test/regress/sql/case.sql | 11 +++++++++++
3 files changed, 71 insertions(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index c6be4f8..bef53dc 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2987,6 +2987,41 @@ eval_const_expressions_mutator(Node *node,
newexpr->location = expr->location;
return (Node *) newexpr;
}
+ case T_NullIfExpr:
+ {
+ /*
+ * Since NullIf is not common enough to deserve specially
+ * optimized code, use ece_generic_processing and
+ * ece_evaluate_expr to simplify the code as much as possible.
+ */
+ NullIfExpr *expr;
+ ListCell *arg;
+ bool has_nonconst_input = false;
+
+ /* Copy the node and const-simplify its arguments */
+ expr = (NullIfExpr *) ece_generic_processing(node);
+
+ /* If either argument is NULL they can't be equal */
+ foreach(arg, expr->args)
+ {
+ if (!IsA(lfirst(arg), Const))
+ has_nonconst_input = true;
+ else if (((Const *) lfirst(arg))->constisnull)
+ return (Node *) linitial(expr->args);
+ }
+
+ /*
+ * Need to get OID of underlying function before checking if
+ * the function is OK to evaluate.
+ */
+ set_opfuncid((OpExpr *) expr);
+
+ if (!has_nonconst_input &&
+ ece_function_is_safe(expr->opfuncid, context))
+ return ece_evaluate_expr(expr);
+
+ return (Node *) expr;
+ }
case T_ScalarArrayOpExpr:
{
ScalarArrayOpExpr *saop;
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index 7fcfe9a..f5136c1 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -263,6 +263,31 @@ SELECT *
4 | | 2 | -4
(2 rows)
+-- Tests for constant subexpression simplification
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
--
-- Examples of updates involving tables
--
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 0655d26..83fe43b 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -137,6 +137,17 @@ SELECT *
FROM CASE_TBL a, CASE2_TBL b
WHERE COALESCE(f,b.i) = 2;
+-- Tests for constant subexpression simplification
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 2) = 2;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, 1) IS NOT NULL;
+
+explain (costs off)
+SELECT * FROM CASE_TBL WHERE NULLIF(1, null) = 2;
+
--
-- Examples of updates involving tables
--
--
2.7.2.windows.1
On 24.03.21 11:52, houzj.fnst@fujitsu.com wrote:
+ if (!has_nonconst_input)
+ return ece_evaluate_expr(expr);That's not okay without a further check to see if the comparison function used
by the node is immutable. Compare ScalarArrayOpExpr, for instance.Thanks for pointing it out.
Attaching new patch with this change.
This patch looks okay to me and addresses all the feedback that was
given. If there are no more comments, I'll commit it in a few days.
On 30.03.21 11:20, Peter Eisentraut wrote:
On 24.03.21 11:52, houzj.fnst@fujitsu.com wrote:
+ if (!has_nonconst_input) + return ece_evaluate_expr(expr);That's not okay without a further check to see if the comparison
function used
by the node is immutable. Compare ScalarArrayOpExpr, for instance.Thanks for pointing it out.
Attaching new patch with this change.This patch looks okay to me and addresses all the feedback that was
given. If there are no more comments, I'll commit it in a few days.
done