Skip to content

Commit a1cc1c9

Browse files
jrgemignanidehowef
andauthored
Refactor the IN operator to use '= ANY()' syntax (#1236) (#1488)
This change reduces use of internal function calls from the IN implementation to optimize performance. This also changes IN behavior to correctly return NULL upon NULL elements included in the list that IN checks against. Added and corrected regression tests. Corrected for lack of A_Const field isnull and needed #include files. Corrected for lack of function availability in PG12 Co-authored by: Josh Innis <JoshInnis@gmail.com> Co-authored-by: Dehowe Feng <8065116+dehowef@users.noreply.github.com>
1 parent 8f9b07c commit a1cc1c9

4 files changed

Lines changed: 232 additions & 19 deletions

File tree

regress/expected/expr.out

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,28 @@ $$RETURN {bool: true, int: 1} IN ['str', 1, 1.0, true, null, {bool: true, int: 1
197197
t
198198
(1 row)
199199

200+
SELECT * FROM cypher('expr',
201+
$$RETURN 1 IN [1.0, [NULL]]$$) AS r(c boolean);
202+
c
203+
---
204+
t
205+
(1 row)
206+
207+
SELECT * FROM cypher('expr',
208+
$$RETURN [NULL] IN [1.0, [NULL]]$$) AS r(c boolean);
209+
c
210+
---
211+
t
212+
(1 row)
213+
200214
-- should return SQL null, nothing
215+
SELECT * FROM cypher('expr',
216+
$$RETURN true IN NULL $$) AS r(c boolean);
217+
c
218+
---
219+
220+
(1 row)
221+
201222
SELECT * FROM cypher('expr',
202223
$$RETURN null IN ['str', 1, 1.0, true, null]$$) AS r(c boolean);
203224
c
@@ -219,49 +240,95 @@ $$RETURN 'str' IN null $$) AS r(c boolean);
219240

220241
(1 row)
221242

222-
-- should all return false
223243
SELECT * FROM cypher('expr',
224244
$$RETURN 0 IN ['str', 1, 1.0, true, null]$$) AS r(c boolean);
225245
c
226246
---
227-
f
247+
228248
(1 row)
229249

230250
SELECT * FROM cypher('expr',
231251
$$RETURN 1.1 IN ['str', 1, 1.0, true, null]$$) AS r(c boolean);
232252
c
233253
---
234-
f
254+
235255
(1 row)
236256

237257
SELECT * FROM cypher('expr',
238258
$$RETURN 'Str' IN ['str', 1, 1.0, true, null]$$) AS r(c boolean);
239259
c
240260
---
241-
f
261+
242262
(1 row)
243263

244264
SELECT * FROM cypher('expr',
245265
$$RETURN [1,3,5,[2,4,5]] IN ['str', 1, 1.0, true, null, [1,3,5,[2,4,6]]]$$) AS r(c boolean);
246266
c
247267
---
248-
f
268+
249269
(1 row)
250270

251271
SELECT * FROM cypher('expr',
252272
$$RETURN {bool: true, int: 2} IN ['str', 1, 1.0, true, null, {bool: true, int: 1}, [1,3,5,[2,4,6]]]$$) AS r(c boolean);
253273
c
254274
---
275+
276+
(1 row)
277+
278+
-- should return false
279+
SELECT * FROM cypher('expr',
280+
$$RETURN 'str' IN ['StR', 1, true]$$) AS r(c boolean);
281+
c
282+
---
283+
f
284+
(1 row)
285+
286+
SELECT * FROM cypher('expr',
287+
$$RETURN 2 IN ['StR', 1, true]$$) AS r(c boolean);
288+
c
289+
---
290+
f
291+
(1 row)
292+
293+
SELECT * FROM cypher('expr',
294+
$$RETURN false IN ['StR', 1, true]$$) AS r(c boolean);
295+
c
296+
---
297+
f
298+
(1 row)
299+
300+
SELECT * FROM cypher('expr',
301+
$$RETURN [1,2] IN ['StR', 1, 2, true]$$) AS r(c boolean);
302+
c
303+
---
304+
f
305+
(1 row)
306+
307+
SELECT * FROM cypher('expr',
308+
$$RETURN 1 in [[1]]$$) AS r(c boolean);
309+
c
310+
---
311+
f
312+
(1 row)
313+
314+
SELECT * FROM cypher('expr',
315+
$$RETURN 1 IN [[null]]$$) AS r(c boolean);
316+
c
317+
---
255318
f
256319
(1 row)
257320

258321
-- should error - ERROR: object of IN must be a list
259322
SELECT * FROM cypher('expr',
260323
$$RETURN null IN 'str' $$) AS r(c boolean);
261324
ERROR: object of IN must be a list
325+
LINE 2: $$RETURN null IN 'str' $$) AS r(c boolean);
326+
^
262327
SELECT * FROM cypher('expr',
263328
$$RETURN 'str' IN 'str' $$) AS r(c boolean);
264329
ERROR: object of IN must be a list
330+
LINE 2: $$RETURN 'str' IN 'str' $$) AS r(c boolean);
331+
^
265332
-- list access
266333
SELECT * FROM cypher('expr',
267334
$$RETURN [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10][0]$$) AS r(c agtype);

regress/sql/expr.sql

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,19 @@ SELECT * FROM cypher('expr',
120120
$$RETURN [1,3,5,[2,4,6]] IN ['str', 1, 1.0, true, null, [1,3,5,[2,4,6]]]$$) AS r(c boolean);
121121
SELECT * FROM cypher('expr',
122122
$$RETURN {bool: true, int: 1} IN ['str', 1, 1.0, true, null, {bool: true, int: 1}, [1,3,5,[2,4,6]]]$$) AS r(c boolean);
123+
SELECT * FROM cypher('expr',
124+
$$RETURN 1 IN [1.0, [NULL]]$$) AS r(c boolean);
125+
SELECT * FROM cypher('expr',
126+
$$RETURN [NULL] IN [1.0, [NULL]]$$) AS r(c boolean);
123127
-- should return SQL null, nothing
124128
SELECT * FROM cypher('expr',
129+
$$RETURN true IN NULL $$) AS r(c boolean);
130+
SELECT * FROM cypher('expr',
125131
$$RETURN null IN ['str', 1, 1.0, true, null]$$) AS r(c boolean);
126132
SELECT * FROM cypher('expr',
127133
$$RETURN null IN ['str', 1, 1.0, true]$$) AS r(c boolean);
128134
SELECT * FROM cypher('expr',
129135
$$RETURN 'str' IN null $$) AS r(c boolean);
130-
-- should all return false
131136
SELECT * FROM cypher('expr',
132137
$$RETURN 0 IN ['str', 1, 1.0, true, null]$$) AS r(c boolean);
133138
SELECT * FROM cypher('expr',
@@ -138,6 +143,19 @@ SELECT * FROM cypher('expr',
138143
$$RETURN [1,3,5,[2,4,5]] IN ['str', 1, 1.0, true, null, [1,3,5,[2,4,6]]]$$) AS r(c boolean);
139144
SELECT * FROM cypher('expr',
140145
$$RETURN {bool: true, int: 2} IN ['str', 1, 1.0, true, null, {bool: true, int: 1}, [1,3,5,[2,4,6]]]$$) AS r(c boolean);
146+
-- should return false
147+
SELECT * FROM cypher('expr',
148+
$$RETURN 'str' IN ['StR', 1, true]$$) AS r(c boolean);
149+
SELECT * FROM cypher('expr',
150+
$$RETURN 2 IN ['StR', 1, true]$$) AS r(c boolean);
151+
SELECT * FROM cypher('expr',
152+
$$RETURN false IN ['StR', 1, true]$$) AS r(c boolean);
153+
SELECT * FROM cypher('expr',
154+
$$RETURN [1,2] IN ['StR', 1, 2, true]$$) AS r(c boolean);
155+
SELECT * FROM cypher('expr',
156+
$$RETURN 1 in [[1]]$$) AS r(c boolean);
157+
SELECT * FROM cypher('expr',
158+
$$RETURN 1 IN [[null]]$$) AS r(c boolean);
141159
-- should error - ERROR: object of IN must be a list
142160
SELECT * FROM cypher('expr',
143161
$$RETURN null IN 'str' $$) AS r(c boolean);

src/backend/parser/cypher_expr.c

Lines changed: 140 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "miscadmin.h"
2828
#include "catalog/pg_type_d.h"
2929
#include "nodes/nodeFuncs.h"
30+
#include "optimizer/var.h"
3031
#include "parser/parse_coerce.h"
3132
#include "parser/parse_collate.h"
3233
#include "parser/parse_func.h"
@@ -483,26 +484,153 @@ static Node *transform_cypher_comparison_aexpr_OP(cypher_parsestate *cpstate,
483484
return (Node *)transform_AEXPR_OP(cpstate, n);
484485
}
485486

487+
/* copied over from PostgreSQL version 13 function of the same name */
488+
static bool verify_common_type(Oid common_type, List *exprs)
489+
{
490+
ListCell *lc;
491+
492+
foreach(lc, exprs)
493+
{
494+
Node *nexpr = (Node *) lfirst(lc);
495+
Oid ntype = exprType(nexpr);
496+
497+
if (!can_coerce_type(1, &ntype, &common_type, COERCION_IMPLICIT))
498+
{
499+
return false;
500+
}
501+
}
502+
return true;
503+
}
486504

487505
static Node *transform_AEXPR_IN(cypher_parsestate *cpstate, A_Expr *a)
488506
{
489-
Oid func_in_oid;
490-
FuncExpr *result;
491-
List *args = NIL;
507+
ParseState *pstate = (ParseState *)cpstate;
508+
cypher_list *rexpr;
509+
Node *result = NULL;
510+
Node *lexpr;
511+
List *rexprs;
512+
List *rvars;
513+
List *rnonvars;
514+
bool useOr;
515+
ListCell *l;
516+
517+
/* Check for null arguments in the list to return NULL*/
518+
if (!is_ag_node(a->rexpr, cypher_list))
519+
{
520+
if (nodeTag(a->rexpr) == T_A_Const)
521+
{
522+
A_Const *r_a_const = (A_Const*)a->rexpr;
523+
if (r_a_const->val.type == T_Null)
524+
{
525+
return (Node *)makeConst(AGTYPEOID, -1, InvalidOid, -1,
526+
(Datum)NULL, true, false);
527+
}
528+
}
492529

493-
args = lappend(args, transform_cypher_expr_recurse(cpstate, a->rexpr));
494-
args = lappend(args, transform_cypher_expr_recurse(cpstate, a->lexpr));
530+
ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
531+
errmsg("object of IN must be a list")));
532+
}
495533

496-
/* get the agtype_access_slice function */
497-
func_in_oid = get_ag_func_oid("agtype_in_operator", 2, AGTYPEOID,
498-
AGTYPEOID);
534+
Assert(is_ag_node(a->rexpr, cypher_list));
535+
536+
// If the operator is <>, combine with AND not OR.
537+
if (strcmp(strVal(linitial(a->name)), "<>") == 0)
538+
{
539+
useOr = false;
540+
}
541+
else
542+
{
543+
useOr = true;
544+
}
545+
546+
lexpr = transform_cypher_expr_recurse(cpstate, a->lexpr);
547+
548+
rexprs = rvars = rnonvars = NIL;
549+
550+
rexpr = (cypher_list *)a->rexpr;
551+
552+
foreach(l, (List *) rexpr->elems)
553+
{
554+
Node *rexpr = transform_cypher_expr_recurse(cpstate, lfirst(l));
555+
556+
rexprs = lappend(rexprs, rexpr);
557+
if (contain_vars_of_level(rexpr, 0))
558+
{
559+
rvars = lappend(rvars, rexpr);
560+
}
561+
else
562+
{
563+
rnonvars = lappend(rnonvars, rexpr);
564+
}
565+
}
499566

500-
result = makeFuncExpr(func_in_oid, AGTYPEOID, args, InvalidOid, InvalidOid,
501-
COERCE_EXPLICIT_CALL);
567+
/*
568+
* ScalarArrayOpExpr is only going to be useful if there's more than one
569+
* non-Var righthand item.
570+
*/
571+
if (list_length(rnonvars) > 1)
572+
{
573+
List *allexprs;
574+
Oid scalar_type;
575+
List *aexprs;
576+
ArrayExpr *newa;
577+
578+
allexprs = list_concat(list_make1(lexpr), rnonvars);
502579

503-
result->location = exprLocation(a->lexpr);
580+
scalar_type = AGTYPEOID;
581+
582+
Assert(verify_common_type(scalar_type, allexprs));
583+
/*
584+
* coerce all the right-hand non-Var inputs to the common type
585+
* and build an ArrayExpr for them.
586+
*/
587+
588+
aexprs = NIL;
589+
foreach(l, rnonvars)
590+
{
591+
Node *rexpr = (Node *) lfirst(l);
504592

505-
return (Node *)result;
593+
rexpr = coerce_to_common_type(pstate, rexpr, AGTYPEOID, "IN");
594+
aexprs = lappend(aexprs, rexpr);
595+
}
596+
newa = makeNode(ArrayExpr);
597+
newa->array_typeid = get_array_type(AGTYPEOID);
598+
/* array_collid will be set by parse_collate.c */
599+
newa->element_typeid = AGTYPEOID;
600+
newa->elements = aexprs;
601+
newa->multidims = false;
602+
result = (Node *) make_scalar_array_op(pstate, a->name, useOr,
603+
lexpr, (Node *) newa,
604+
a->location);
605+
606+
/* Consider only the Vars (if any) in the loop below */
607+
rexprs = rvars;
608+
}
609+
610+
// Must do it the hard way, with a boolean expression tree.
611+
foreach(l, rexprs)
612+
{
613+
Node *rexpr = (Node *) lfirst(l);
614+
Node *cmp;
615+
616+
// Ordinary scalar operator
617+
cmp = (Node *) make_op(pstate, a->name, copyObject(lexpr), rexpr,
618+
pstate->p_last_srf, a->location);
619+
620+
cmp = coerce_to_boolean(pstate, cmp, "IN");
621+
if (result == NULL)
622+
{
623+
result = cmp;
624+
}
625+
else
626+
{
627+
result = (Node *) makeBoolExpr(useOr ? OR_EXPR : AND_EXPR,
628+
list_make2(result, cmp),
629+
a->location);
630+
}
631+
}
632+
633+
return result;
506634
}
507635

508636
static Node *transform_BoolExpr(cypher_parsestate *cpstate, BoolExpr *expr)

src/backend/utils/adt/agtype_ops.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1403,7 +1403,7 @@ Datum agtype_exists_all_agtype(PG_FUNCTION_ARGS)
14031403

14041404
PG_FUNCTION_INFO_V1(agtype_contains);
14051405
/*
1406-
* <@ operator for agtype. Returns true if the right agtype path/value entries
1406+
* @> operator for agtype. Returns true if the right agtype path/value entries
14071407
* contained at the top level within the left agtype value
14081408
*/
14091409
Datum agtype_contains(PG_FUNCTION_ARGS)

0 commit comments

Comments
 (0)