[PATCH] indexability of << operator for inet/cidr
Attached is a patch that explains to optimizer that its possible to use
index when performing a << b where a is an inet/cidr value and b is a
constant.
Indexpath generated for such an expression is this:
(a > network(b)) and (a <= set_masklen(broadcast(b, 32)))
Since this is my first time delving in the guts of postgres, someone
definitely should review it :)
I mostly based my code on prefix_quals function for string types.
Thanks
Attachments:
pg-index-inetsub.patchtext/plain; charset=US-ASCII; name=pg-index-inetsub.patchDownload
Index: backend/optimizer/path/indxpath.c
===================================================================
RCS file: /cvs/pgsql/pgsql/src/backend/optimizer/path/indxpath.c,v
retrieving revision 1.104
diff --unified -r1.104 indxpath.c
--- backend/optimizer/path/indxpath.c 2001/03/23 04:49:53 1.104
+++ backend/optimizer/path/indxpath.c 2001/06/14 20:15:58
@@ -95,6 +95,7 @@
bool indexkey_on_left);
static List *prefix_quals(Var *leftop, Oid expr_op,
char *prefix, Pattern_Prefix_Status pstatus);
+static List *network_prefix_quals(Var *leftop, Oid expr_op, Datum rightop);
static Oid find_operator(const char *opname, Oid datatype);
static Datum string_to_datum(const char *str, Oid datatype);
static Const *string_to_const(const char *str, Oid datatype);
@@ -1731,6 +1732,11 @@
pfree(patt);
}
break;
+ case OID_INET_SUB_OP:
+ case OID_INET_SUBEQ_OP:
+ case OID_CIDR_SUB_OP:
+ case OID_CIDR_SUBEQ_OP:
+ isIndexable = 1;
}
/* done if the expression doesn't look indexable */
@@ -1780,6 +1786,14 @@
!op_class(find_operator("<", NAMEOID), opclass, relam))
isIndexable = false;
break;
+ case OID_INET_SUB_OP:
+ case OID_INET_SUBEQ_OP:
+ case OID_CIDR_SUB_OP:
+ case OID_CIDR_SUBEQ_OP:
+ if (!op_class(find_operator(">=", INETOID), opclass, relam) ||
+ !op_class(find_operator("<", INETOID), opclass, relam))
+ isIndexable = false;
+ break;
}
return isIndexable;
@@ -1894,6 +1908,15 @@
pfree(patt);
break;
+ case OID_INET_SUB_OP:
+ case OID_INET_SUBEQ_OP:
+ case OID_CIDR_SUB_OP:
+ case OID_CIDR_SUBEQ_OP:
+ constvalue = ((Const *) rightop)->constvalue;
+ resultquals = nconc(resultquals,
+ network_prefix_quals(leftop, expr_op, constvalue));
+ break;
+
default:
resultquals = lappend(resultquals, clause);
break;
@@ -1901,6 +1924,89 @@
}
return resultquals;
+}
+
+/*
+ * Given a leftop and a rightop, and a inet-class sup/sub operator,
+ * generate suitable indexqual condition(s). expr_op is the original
+ * operator.
+ * translations:
+ * a << b to (a > network(b)) and (a <= set_masklen(broadcast(b, 32)))
+ * a <<= b to (a >= network(b)) and (a <= set_masklen(broadcast(b, 32)))
+ */
+static List *
+network_prefix_quals(Var *leftop, Oid expr_op, Datum rightop)
+{
+ int is_eq=0;
+ char *opr1name;
+ Datum opr1right;
+ Datum opr2right;
+ Oid opr1oid;
+ Oid opr2oid;
+
+ List *result;
+ Oid datatype;
+ Oper *op;
+ Expr *expr;
+
+ switch (expr_op)
+ {
+ case OID_INET_SUB_OP:
+ datatype = INETOID;
+ is_eq=0;
+ break;
+ case OID_INET_SUBEQ_OP:
+ datatype = INETOID;
+ is_eq=1;
+ break;
+ case OID_CIDR_SUB_OP:
+ datatype = CIDROID;
+ is_eq=0;
+ break;
+ case OID_CIDR_SUBEQ_OP:
+ datatype = CIDROID;
+ is_eq=1;
+ break;
+ default:
+ elog(ERROR, "network_prefix_quals: unexpected operator %u", expr_op);
+ return NIL;
+ }
+
+/* create clause "key >= network( rightop )" */
+
+ opr1name = is_eq?">=":">";
+ opr1oid = find_operator(opr1name, datatype);
+ if (opr1oid == InvalidOid)
+ elog(ERROR, "network_prefix_quals: no %s operator for type %u", opr1name, datatype);
+
+ opr1right = DirectFunctionCall1( network_network, rightop );
+
+ op = makeOper(opr1oid, InvalidOid, BOOLOID);
+ expr = make_opclause(op, leftop,
+ (Var *) makeConst(datatype, -1, opr1right,
+ false, false, false, false )
+ );
+ result = makeList1(expr);
+
+/* create clause "key <= set_masklen( broadcast( rightop, 32 ) )" */
+
+ opr2right = DirectFunctionCall2( inet_set_masklen,
+ DirectFunctionCall1(network_broadcast, rightop),
+ Int32GetDatum(32)
+ );
+
+ opr2oid = find_operator("<=", datatype);
+ if (opr2oid == InvalidOid)
+ elog(ERROR, "network_prefix_quals: no <= operator for type %u", datatype);
+
+ op = makeOper(opr2oid, InvalidOid, BOOLOID);
+ expr = make_opclause(op, leftop,
+ (Var *) makeConst(datatype, -1, opr2right,
+ false, false, false, false )
+ );
+ result = lappend(result, expr);
+
+ return result;
}
/*
Index: test/regress/expected/inet.out
===================================================================
RCS file: /cvs/pgsql/pgsql/src/test/regress/expected/inet.out,v
retrieving revision 1.11
diff --unified -r1.11 inet.out
--- test/regress/expected/inet.out 2000/12/22 18:00:21 1.11
+++ test/regress/expected/inet.out 2001/06/14 20:15:59
@@ -18,6 +18,9 @@
-- check that CIDR rejects invalid input:
INSERT INTO INET_TBL (c, i) VALUES ('192.168.1.2/24', '192.168.1.226');
ERROR: invalid CIDR value '192.168.1.2/24': has bits set to right of mask
+-- check that CIDR rejects invalid input when converting from text:
+INSERT INTO INET_TBL (c, i) VALUES (cidr('192.168.1.2/24'), '192.168.1.226');
+ERROR: invalid CIDR value '192.168.1.2/24': has bits set to right of mask
SELECT '' AS ten, c AS cidr, i AS inet FROM INET_TBL;
ten | cidr | inet
-----+----------------+------------------
@@ -135,3 +138,36 @@
| 9.1.2.3/8 | 10.0.0.0/8 | t | t | f | f | f | t | f | f | f | f
(10 rows)
+-- check the conversion to/from text and set_netmask
+select '' AS ten, set_masklen(inet(text(i)), 24) FROM INET_TBL;
+ ten | set_masklen
+-----+------------------
+ | 192.168.1.226/24
+ | 192.168.1.226/24
+ | 10.1.2.3/24
+ | 10.1.2.3/24
+ | 10.1.2.3/24
+ | 10.1.2.3/24
+ | 10.1.2.3/24
+ | 10.1.2.3/24
+ | 11.1.2.3/24
+ | 9.1.2.3/24
+(10 rows)
+
+-- check the indexability of <<
+create index inet_idx1 on inet_tbl(i);
+set enable_seqscan to off;
+explain select * from inet_tbl where i<<'192/8'::cidr;
+NOTICE: QUERY PLAN:
+
+Index Scan using inet_idx1 on inet_tbl (cost=0.00..2.01 rows=1 width=24)
+
+select * from inet_tbl where i<<'192/8'::cidr;
+ c | i
+----------------+------------------
+ 192.168.1.0/24 | 192.168.1.226/24
+ 192.168.1.0/24 | 192.168.1.226
+(2 rows)
+
+set enable_seqscan to on;
+drop index inet_idx1;
Index: test/regress/sql/inet.sql
===================================================================
RCS file: /cvs/pgsql/pgsql/src/test/regress/sql/inet.sql,v
retrieving revision 1.5
diff --unified -r1.5 inet.sql
--- test/regress/sql/inet.sql 2000/11/10 20:13:27 1.5
+++ test/regress/sql/inet.sql 2001/06/14 20:15:59
@@ -18,6 +18,8 @@
INSERT INTO INET_TBL (c, i) VALUES ('10', '9.1.2.3/8');
-- check that CIDR rejects invalid input:
INSERT INTO INET_TBL (c, i) VALUES ('192.168.1.2/24', '192.168.1.226');
+-- check that CIDR rejects invalid input when converting from text:
+INSERT INTO INET_TBL (c, i) VALUES (cidr('192.168.1.2/24'), '192.168.1.226');
SELECT '' AS ten, c AS cidr, i AS inet FROM INET_TBL;
@@ -44,4 +46,14 @@
i << c AS sb, i <<= c AS sbe,
i >> c AS sup, i >>= c AS spe
FROM INET_TBL;
+
+-- check the conversion to/from text and set_netmask
+select '' AS ten, set_masklen(inet(text(i)), 24) FROM INET_TBL;
+-- check the indexability of <<
+create index inet_idx1 on inet_tbl(i);
+set enable_seqscan to off;
+explain select * from inet_tbl where i<<'192/8'::cidr;
+select * from inet_tbl where i<<'192/8'::cidr;
+set enable_seqscan to on;
+drop index inet_idx1;
Alex Pilosov <alex@pilosoft.com> writes:
Indexpath generated for such an expression is this:
(a > network(b)) and (a <= set_masklen(broadcast(b, 32)))
What happens to that set_masklen thing for IPv6?
If the network.c code were exporting a function that made this value,
I'd not worry; but I don't like wiring an IPv4 assumption into code far
away in the planner. Can't we do better here? Perhaps move the
generation of the indexscan bound values into a subroutine in network.c?
regards, tom lane
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
Attached is a patch that explains to optimizer that its possible to use
index when performing a << b where a is an inet/cidr value and b is a
constant.Indexpath generated for such an expression is this:
(a > network(b)) and (a <= set_masklen(broadcast(b, 32)))Since this is my first time delving in the guts of postgres, someone
definitely should review it :)I mostly based my code on prefix_quals function for string types.
Thanks
Content-Description:
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026On Thu, 14 Jun 2001, Tom Lane wrote:
Alex Pilosov <alex@pilosoft.com> writes:
Indexpath generated for such an expression is this:
(a > network(b)) and (a <= set_masklen(broadcast(b, 32)))What happens to that set_masklen thing for IPv6?
If the network.c code were exporting a function that made this value,
I'd not worry; but I don't like wiring an IPv4 assumption into code far
away in the planner. Can't we do better here? Perhaps move the
generation of the indexscan bound values into a subroutine in network.c?
Good point. I already rewrote it, but I am going to send it in tomorrow, I
want to resync to HEAD, since some of network.c was taken in and I want to
have a clean patch for you guys :)
Thanks
-alex
Patch removed from queue, awaiting updated version.
Attached is a patch that explains to optimizer that its possible to use
index when performing a << b where a is an inet/cidr value and b is a
constant.Indexpath generated for such an expression is this:
(a > network(b)) and (a <= set_masklen(broadcast(b, 32)))Since this is my first time delving in the guts of postgres, someone
definitely should review it :)I mostly based my code on prefix_quals function for string types.
Thanks
Content-Description:
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026