From: | Cary Huang <cary(dot)huang(at)highgo(dot)ca> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Cc: | Emre Hasegeli <emre(at)hasegeli(dot)com> |
Subject: | Re: postgres_fdw: Handle boolean comparison predicates |
Date: | 2021-08-20 19:33:12 |
Message-ID: | 162948799238.22084.17682262819167731211.pgcf@coridan.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
Hello
I tried to apply the patch to master branch and got a couple of errors, so I think the patch needs a rebase.
I also agree with Ashutosh that the "IS NOT TRUE" case can be simplified to just "IS FALSE". it's simpler to understand.
based on this, I think we should restructure the switch-case statement in deparseBooleanTest because some of the cases in there evaluate to the same result but handles differently.
For example, "IS TRUE" and "IS NOT FALSE" both evaluate to true, so can be handled in the same way
something like:
switch (node->booltesttype)
{
case IS_TRUE:
case IS_NOT_FALSE:
appendStringInfoChar(buf, '(');
deparseExpr(node->arg, context);
appendStringInfoString(buf, ")");
break;
case IS_FALSE:
case IS_NOT_TRUE:
appendStringInfoChar(buf, '(');
deparseExpr(node->arg, context);
appendStringInfoString(buf, " IS FALSE)");
break;
case IS_UNKNOWN:
appendStringInfoChar(buf, '(');
deparseExpr(node->arg, context);
appendStringInfoString(buf, " IS NULL)");
break;
case IS_NOT_UNKNOWN:
appendStringInfoChar(buf, '(');
deparseExpr(node->arg, context);
appendStringInfoString(buf, " IS NOT NULL)");
break;
}
just a thought
thanks!
-------------------------------
Cary Huang
HighGo Software Canada
www.highgo.ca
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2021-08-20 19:40:08 | Re: The Free Space Map: Problems and Opportunities |
Previous Message | Tomas Vondra | 2021-08-20 19:32:05 | Re: Use extended statistics to estimate (Var op Var) clauses |