[PATCH] inet << indexability
This is second take at indexability of << operator for inet types.
Please take a look at it.
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.
I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.
-alex
Attachments:
inet-idx.difftext/plain; charset=US-ASCII; name=inet-idx.diffDownload
Index: src/backend/optimizer/path/indxpath.c
===================================================================
RCS file: /cvs/pgsql/pgsql/src/backend/optimizer/path/indxpath.c,v
retrieving revision 1.106
diff --unified -r1.106 indxpath.c
--- src/backend/optimizer/path/indxpath.c 2001/06/05 17:13:51 1.106
+++ src/backend/optimizer/path/indxpath.c 2001/06/16 02:31:38
@@ -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);
@@ -1761,6 +1762,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 */
@@ -1810,6 +1816,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;
@@ -1924,6 +1938,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;
@@ -1931,6 +1954,86 @@
}
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_scan_first( 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_scan_first, 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 <= network_scan_last( rightop )" */
+
+ opr2right = DirectFunctionCall1( network_scan_last, rightop );
+
+ 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: src/backend/port/dynloader/openbsd.h
===================================================================
RCS file: /cvs/pgsql/pgsql/src/backend/port/dynloader/openbsd.h,v
retrieving revision 1.5
diff --unified -r1.5 openbsd.h
--- src/backend/port/dynloader/openbsd.h 2001/05/15 16:55:27 1.5
+++ src/backend/port/dynloader/openbsd.h 2001/06/16 02:31:39
@@ -34,7 +34,7 @@
* begin with an underscore is fairly tricky, and some versions of
* NetBSD (like 1.0, and 1.0A pre June 1995) have no dlerror.)
*/
-#define pg_dlopen(f) BSD44_derived_dlopen((f), RTLD_LAZY | RTLD_GLOBAL)
+#define pg_dlopen(f) BSD44_derived_dlopen((f), RTLD_LAZY)
#define pg_dlsym BSD44_derived_dlsym
#define pg_dlclose BSD44_derived_dlclose
#define pg_dlerror BSD44_derived_dlerror
Index: src/backend/utils/adt/network.c
===================================================================
RCS file: /cvs/pgsql/pgsql/src/backend/utils/adt/network.c,v
retrieving revision 1.31
diff --unified -r1.31 network.c
--- src/backend/utils/adt/network.c 2001/06/13 21:08:59 1.31
+++ src/backend/utils/adt/network.c 2001/06/16 02:31:41
@@ -490,6 +490,34 @@
PG_RETURN_INT32(ip_bits(ip));
}
+
+/* These functions are used by planner when generating index paths
+ * for clause a << b
+ */
+
+/* return the minimal value for an IP on a given network */
+Datum
+network_scan_first(PG_FUNCTION_ARGS)
+{
+ Datum in = PG_GETARG_DATUM(0);
+ return DirectFunctionCall1(network_network, in);
+}
+
+/* return "last" IP on a given network. Its the broadcast address,
+ * however, masklen has to be set to 32, since
+ * 192.168.0.255/24 is considered less than 192.168.0.255/32
+ */
+Datum
+network_scan_last(PG_FUNCTION_ARGS)
+{
+ Datum in = PG_GETARG_DATUM(0);
+ return DirectFunctionCall2( inet_set_masklen,
+ DirectFunctionCall1(network_broadcast, in),
+ Int32GetDatum(32)
+ );
+
+}
+
Datum
network_broadcast(PG_FUNCTION_ARGS)
{
Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /cvs/pgsql/pgsql/src/bin/psql/tab-complete.c,v
retrieving revision 1.33
diff --unified -r1.33 tab-complete.c
--- src/bin/psql/tab-complete.c 2001/06/11 22:12:48 1.33
+++ src/bin/psql/tab-complete.c 2001/06/16 02:31:45
@@ -62,6 +62,8 @@
#ifdef HAVE_RL_FILENAME_COMPLETION_FUNCTION
#define filename_completion_function rl_filename_completion_function
+#else
+#define filename_completion_function psql_completion
#endif
#ifdef HAVE_RL_COMPLETION_MATCHES
Index: src/include/catalog/pg_operator.h
===================================================================
RCS file: /cvs/pgsql/pgsql/src/include/catalog/pg_operator.h,v
retrieving revision 1.90
diff --unified -r1.90 pg_operator.h
--- src/include/catalog/pg_operator.h 2001/06/10 22:32:35 1.90
+++ src/include/catalog/pg_operator.h 2001/06/16 02:31:46
@@ -674,9 +674,13 @@
DATA(insert OID = 1205 ( ">" PGUID 0 b t f 869 869 16 1203 1204 0 0 network_gt scalargtsel scalargtjoinsel ));
DATA(insert OID = 1206 ( ">=" PGUID 0 b t f 869 869 16 1204 1203 0 0 network_ge scalargtsel scalargtjoinsel ));
DATA(insert OID = 931 ( "<<" PGUID 0 b t f 869 869 16 933 0 0 0 network_sub - - ));
+#define OID_INET_SUB_OP 931
DATA(insert OID = 932 ( "<<=" PGUID 0 b t f 869 869 16 934 0 0 0 network_subeq - - ));
+#define OID_INET_SUBEQ_OP 932
DATA(insert OID = 933 ( ">>" PGUID 0 b t f 869 869 16 931 0 0 0 network_sup - - ));
+#define OID_INET_SUP_OP 933
DATA(insert OID = 934 ( ">>=" PGUID 0 b t f 869 869 16 932 0 0 0 network_supeq - - ));
+#define OID_INET_SUPEQ_OP 934
/* CIDR type */
DATA(insert OID = 820 ( "=" PGUID 0 b t f 650 650 16 820 821 822 822 network_eq eqsel eqjoinsel ));
@@ -686,9 +690,13 @@
DATA(insert OID = 824 ( ">" PGUID 0 b t f 650 650 16 822 823 0 0 network_gt scalargtsel scalargtjoinsel ));
DATA(insert OID = 825 ( ">=" PGUID 0 b t f 650 650 16 823 822 0 0 network_ge scalargtsel scalargtjoinsel ));
DATA(insert OID = 826 ( "<<" PGUID 0 b t f 650 650 16 828 0 0 0 network_sub - - ));
+#define OID_CIDR_SUB_OP 826
DATA(insert OID = 827 ( "<<=" PGUID 0 b t f 650 650 16 1004 0 0 0 network_subeq - - ));
+#define OID_CIDR_SUBEQ_OP 827
DATA(insert OID = 828 ( ">>" PGUID 0 b t f 650 650 16 826 0 0 0 network_sup - - ));
+#define OID_CIDR_SUP_OP 828
DATA(insert OID = 1004 ( ">>=" PGUID 0 b t f 650 650 16 827 0 0 0 network_supeq - - ));
+#define OID_CIDR_SUPEQ_OP 1004
/* case-insensitive LIKE hacks */
DATA(insert OID = 1625 ( "~~*" PGUID 0 b t f 19 25 16 0 1626 0 0 nameiclike iclikesel iclikejoinsel ));
Index: src/include/utils/builtins.h
===================================================================
RCS file: /cvs/pgsql/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.154
diff --unified -r1.154 builtins.h
--- src/include/utils/builtins.h 2001/06/14 01:09:22 1.154
+++ src/include/utils/builtins.h 2001/06/16 02:31:48
@@ -527,6 +527,8 @@
extern Datum text_cidr(PG_FUNCTION_ARGS);
extern Datum text_inet(PG_FUNCTION_ARGS);
extern Datum inet_set_masklen(PG_FUNCTION_ARGS);
+extern Datum network_scan_first(PG_FUNCTION_ARGS);
+extern Datum network_scan_last(PG_FUNCTION_ARGS);
/* mac.c */
extern Datum macaddr_in(PG_FUNCTION_ARGS);
Index: src/test/regress/expected/inet.out
===================================================================
RCS file: /cvs/pgsql/pgsql/src/test/regress/expected/inet.out,v
retrieving revision 1.12
diff --unified -r1.12 inet.out
--- src/test/regress/expected/inet.out 2001/06/13 21:08:59 1.12
+++ src/test/regress/expected/inet.out 2001/06/16 02:31:57
@@ -154,3 +154,20 @@
| 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..3.68 rows=5 width=64)
+
+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: src/test/regress/sql/inet.sql
===================================================================
RCS file: /cvs/pgsql/pgsql/src/test/regress/sql/inet.sql,v
retrieving revision 1.6
diff --unified -r1.6 inet.sql
--- src/test/regress/sql/inet.sql 2001/06/13 21:09:00 1.6
+++ src/test/regress/sql/inet.sql 2001/06/16 02:31:57
@@ -49,3 +49,11 @@
-- 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;
+
Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.
-alex
On Fri, 15 Jun 2001, Alex Pilosov wrote:
Show quoted text
This is second take at indexability of << operator for inet types.
Please take a look at it.
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.-alex
Attachments:
inet-idx.difftext/plain; charset=US-ASCII; name=inet-idx.diffDownload
Index: src/backend/optimizer/path/indxpath.c
===================================================================
RCS file: /cvs/pgsql/pgsql/src/backend/optimizer/path/indxpath.c,v
retrieving revision 1.106
diff --unified -r1.106 indxpath.c
--- src/backend/optimizer/path/indxpath.c 2001/06/05 17:13:51 1.106
+++ src/backend/optimizer/path/indxpath.c 2001/06/16 02:31:38
@@ -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);
@@ -1761,6 +1762,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 */
@@ -1810,6 +1816,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;
@@ -1924,6 +1938,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;
@@ -1931,6 +1954,86 @@
}
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_scan_first( 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_scan_first, 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 <= network_scan_last( rightop )" */
+
+ opr2right = DirectFunctionCall1( network_scan_last, rightop );
+
+ 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: src/backend/utils/adt/network.c
===================================================================
RCS file: /cvs/pgsql/pgsql/src/backend/utils/adt/network.c,v
retrieving revision 1.31
diff --unified -r1.31 network.c
--- src/backend/utils/adt/network.c 2001/06/13 21:08:59 1.31
+++ src/backend/utils/adt/network.c 2001/06/16 02:31:41
@@ -490,6 +490,34 @@
PG_RETURN_INT32(ip_bits(ip));
}
+
+/* These functions are used by planner when generating index paths
+ * for clause a << b
+ */
+
+/* return the minimal value for an IP on a given network */
+Datum
+network_scan_first(PG_FUNCTION_ARGS)
+{
+ Datum in = PG_GETARG_DATUM(0);
+ return DirectFunctionCall1(network_network, in);
+}
+
+/* return "last" IP on a given network. Its the broadcast address,
+ * however, masklen has to be set to 32, since
+ * 192.168.0.255/24 is considered less than 192.168.0.255/32
+ */
+Datum
+network_scan_last(PG_FUNCTION_ARGS)
+{
+ Datum in = PG_GETARG_DATUM(0);
+ return DirectFunctionCall2( inet_set_masklen,
+ DirectFunctionCall1(network_broadcast, in),
+ Int32GetDatum(32)
+ );
+
+}
+
Datum
network_broadcast(PG_FUNCTION_ARGS)
{
Index: src/include/catalog/pg_operator.h
===================================================================
RCS file: /cvs/pgsql/pgsql/src/include/catalog/pg_operator.h,v
retrieving revision 1.90
diff --unified -r1.90 pg_operator.h
--- src/include/catalog/pg_operator.h 2001/06/10 22:32:35 1.90
+++ src/include/catalog/pg_operator.h 2001/06/16 02:31:46
@@ -674,9 +674,13 @@
DATA(insert OID = 1205 ( ">" PGUID 0 b t f 869 869 16 1203 1204 0 0 network_gt scalargtsel scalargtjoinsel ));
DATA(insert OID = 1206 ( ">=" PGUID 0 b t f 869 869 16 1204 1203 0 0 network_ge scalargtsel scalargtjoinsel ));
DATA(insert OID = 931 ( "<<" PGUID 0 b t f 869 869 16 933 0 0 0 network_sub - - ));
+#define OID_INET_SUB_OP 931
DATA(insert OID = 932 ( "<<=" PGUID 0 b t f 869 869 16 934 0 0 0 network_subeq - - ));
+#define OID_INET_SUBEQ_OP 932
DATA(insert OID = 933 ( ">>" PGUID 0 b t f 869 869 16 931 0 0 0 network_sup - - ));
+#define OID_INET_SUP_OP 933
DATA(insert OID = 934 ( ">>=" PGUID 0 b t f 869 869 16 932 0 0 0 network_supeq - - ));
+#define OID_INET_SUPEQ_OP 934
/* CIDR type */
DATA(insert OID = 820 ( "=" PGUID 0 b t f 650 650 16 820 821 822 822 network_eq eqsel eqjoinsel ));
@@ -686,9 +690,13 @@
DATA(insert OID = 824 ( ">" PGUID 0 b t f 650 650 16 822 823 0 0 network_gt scalargtsel scalargtjoinsel ));
DATA(insert OID = 825 ( ">=" PGUID 0 b t f 650 650 16 823 822 0 0 network_ge scalargtsel scalargtjoinsel ));
DATA(insert OID = 826 ( "<<" PGUID 0 b t f 650 650 16 828 0 0 0 network_sub - - ));
+#define OID_CIDR_SUB_OP 826
DATA(insert OID = 827 ( "<<=" PGUID 0 b t f 650 650 16 1004 0 0 0 network_subeq - - ));
+#define OID_CIDR_SUBEQ_OP 827
DATA(insert OID = 828 ( ">>" PGUID 0 b t f 650 650 16 826 0 0 0 network_sup - - ));
+#define OID_CIDR_SUP_OP 828
DATA(insert OID = 1004 ( ">>=" PGUID 0 b t f 650 650 16 827 0 0 0 network_supeq - - ));
+#define OID_CIDR_SUPEQ_OP 1004
/* case-insensitive LIKE hacks */
DATA(insert OID = 1625 ( "~~*" PGUID 0 b t f 19 25 16 0 1626 0 0 nameiclike iclikesel iclikejoinsel ));
Index: src/include/utils/builtins.h
===================================================================
RCS file: /cvs/pgsql/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.154
diff --unified -r1.154 builtins.h
--- src/include/utils/builtins.h 2001/06/14 01:09:22 1.154
+++ src/include/utils/builtins.h 2001/06/16 02:31:48
@@ -527,6 +527,8 @@
extern Datum text_cidr(PG_FUNCTION_ARGS);
extern Datum text_inet(PG_FUNCTION_ARGS);
extern Datum inet_set_masklen(PG_FUNCTION_ARGS);
+extern Datum network_scan_first(PG_FUNCTION_ARGS);
+extern Datum network_scan_last(PG_FUNCTION_ARGS);
/* mac.c */
extern Datum macaddr_in(PG_FUNCTION_ARGS);
Index: src/test/regress/expected/inet.out
===================================================================
RCS file: /cvs/pgsql/pgsql/src/test/regress/expected/inet.out,v
retrieving revision 1.12
diff --unified -r1.12 inet.out
--- src/test/regress/expected/inet.out 2001/06/13 21:08:59 1.12
+++ src/test/regress/expected/inet.out 2001/06/16 02:31:57
@@ -154,3 +154,20 @@
| 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..3.68 rows=5 width=64)
+
+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: src/test/regress/sql/inet.sql
===================================================================
RCS file: /cvs/pgsql/pgsql/src/test/regress/sql/inet.sql,v
retrieving revision 1.6
diff --unified -r1.6 inet.sql
--- src/test/regress/sql/inet.sql 2001/06/13 21:09:00 1.6
+++ src/test/regress/sql/inet.sql 2001/06/16 02:31:57
@@ -49,3 +49,11 @@
-- 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:
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.
Yes, I'd noted that already in looking at your prior version. I think
it's best not to do an EXPLAIN in the regress test, because I don't want
to have to touch the tests every time the cost functions are tweaked.
However, we can certainly check to make sure that the results of an
indexscan are what we expect. Is the table set up so that this is a
useful test case? For example, it'd be nice to check boundary
conditions (eg, try both << and <<= on a case where they should give
different results).
Do you have any thought of making network_scan_first and
network_scan_last user-visible functions? (Offhand I see no use for
them to a user, but maybe you do.) If not, I'd suggest not using the
fmgr call protocol for them, but just making them pass and return inet*,
or possibly Datum. No need for the extra notational cruft of
DirectFunctionCall.
Another minor stylistic gripe is that you should use bool/true/false
where appropriate, not int/1/0. Otherwise it looks pretty good.
Oh, one more thing: those dynloader/openbsd.h and psql/tab-complete.c
changes don't belong in this patch...
regards, tom lane
Alex Pilosov <alex@pilosoft.com> writes:
I didn't want to make them user-visible, however, the alternative, IMHO,
is worse, since these functions rely on network_broadcast and
network_network to do the work, calling sequence would be:
a) indxpath casts Datum to inet, passes to network_scan*
b) network_scan will create new Datum, pass it to network_broadcast
c) network_scan will extract inet from Datum returned
d) indxpath will then cast inet back to Datum :)
Which, I think, is pretty messy :)
Sure, but you could make them look like
Datum network_scan_first(Datum networkaddress)
without incurring any of that overhead. (Anyway, Datum <-> inet* is
only a cast.)
regards, tom lane
Import Notes
Reply to msg id not found: Pine.BSO.4.10.10106161444060.17529-100000@spider.pilosoft.comReference msg id not found: Pine.BSO.4.10.10106161444060.17529-100000@spider.pilosoft.com | Resolved by subject fallback
On Sat, 16 Jun 2001, Tom Lane wrote:
Alex Pilosov <alex@pilosoft.com> writes:
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.Yes, I'd noted that already in looking at your prior version. I think
it's best not to do an EXPLAIN in the regress test, because I don't want
to have to touch the tests every time the cost functions are tweaked.
I'll remove it with resubmitted patch.
However, we can certainly check to make sure that the results of an
indexscan are what we expect. Is the table set up so that this is a
useful test case? For example, it'd be nice to check boundary
conditions (eg, try both << and <<= on a case where they should give
different results).
I'll do that too.
Do you have any thought of making network_scan_first and
network_scan_last user-visible functions? (Offhand I see no use for
them to a user, but maybe you do.) If not, I'd suggest not using the
fmgr call protocol for them, but just making them pass and return inet*,
or possibly Datum. No need for the extra notational cruft of
DirectFunctionCall.
I didn't want to make them user-visible, however, the alternative, IMHO,
is worse, since these functions rely on network_broadcast and
network_network to do the work, calling sequence would be:
a) indxpath casts Datum to inet, passes to network_scan*
b) network_scan will create new Datum, pass it to network_broadcast
c) network_scan will extract inet from Datum returned
d) indxpath will then cast inet back to Datum :)
Which, I think, is pretty messy :)
Another minor stylistic gripe is that you should use bool/true/false
where appropriate, not int/1/0. Otherwise it looks pretty good.
I'll clean it up and resubmit.
Oh, one more thing: those dynloader/openbsd.h and psql/tab-complete.c
changes don't belong in this patch...
Sorry, my fault
-alex
On Sat, 16 Jun 2001, Tom Lane wrote:
Alex Pilosov <alex@pilosoft.com> writes:
I didn't want to make them user-visible, however, the alternative, IMHO,
is worse, since these functions rely on network_broadcast and
network_network to do the work, calling sequence would be:
a) indxpath casts Datum to inet, passes to network_scan*
b) network_scan will create new Datum, pass it to network_broadcast
c) network_scan will extract inet from Datum returned
d) indxpath will then cast inet back to Datum :)
Which, I think, is pretty messy :)Sure, but you could make them look like
Datum network_scan_first(Datum networkaddress)
without incurring any of that overhead. (Anyway, Datum <-> inet* is
only a cast.)
Gotcha, I misunderstood you the first time.
Thanks
-alex
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.
Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.-alex
On Fri, 15 Jun 2001, Alex Pilosov wrote:
This is second take at indexability of << operator for inet types.
Please take a look at it.
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.-alex
Content-Description:
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
--
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 19026Tom already merged [latest version of this patch] it in, so you can delete
this one from patch list.
Oh, OK. I thought he had done that but I couldn't find the commit
message in my mailbox.
Thanks
-alexOn Mon, 18 Jun 2001, Bruce Momjian wrote:
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.
Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.-alex
On Fri, 15 Jun 2001, Alex Pilosov wrote:
This is second take at indexability of << operator for inet types.
Please take a look at it.
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.-alex
Content-Description:
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
--
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 19026Import Notes
Reply to msg id not found: Pine.BSO.4.10.10106181422120.23128-100000@spider.pilosoft.com | Resolved by subject fallback
Tom already merged [latest version of this patch] it in, so you can delete
this one from patch list.
Thanks
-alex
On Mon, 18 Jun 2001, Bruce Momjian wrote:
Show quoted text
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.
Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.-alex
On Fri, 15 Jun 2001, Alex Pilosov wrote:
This is second take at indexability of << operator for inet types.
Please take a look at it.
Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.-alex
Content-Description:
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
... best not to do an EXPLAIN in the regress test, because I don't want
to have to touch the tests every time the cost functions are tweaked...
At some point we really should have an "optimizer" regression test, so
y'all *do* have to touch some regression test when the cost functions
are tweaked. If it were isolated into a single test, with appropriate
comments to keep it easy to remember *why* a result should be a certain
way, then it should be manageable and make it easier to proactively
evaluate changes.
It likely would have have full coverage, but at least some over/under
test cases would help...
- Thomas