Обсуждение: [PATCH 0/3] Introduce spgist quadtree @<(point,circle) operator
Hi, This patch series is to add support for spgist quadtree @<(point,circle) operator. The first two patches are to refactor existing code before implemention the new feature. The third commit is the actual implementation provided with a set of simple unit tests. Matwey V. Kornilov (3): Introduce helper variable in spgquadtreeproc.c Introduce spg_quad_inner_consistent_box_helper() in spgquadtreeproc.c Add initial support for spgist quadtree @<(point,circle) operator src/backend/access/spgist/spgquadtreeproc.c | 138 +++++++++++++++++++--------- src/include/catalog/pg_amop.dat | 3 + src/test/regress/expected/create_index.out | 96 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 32 +++++++ 4 files changed, 225 insertions(+), 44 deletions(-) -- 2.13.7
Use shorter variable name instead of repeating in->scankeys[i] in
spg_quad_leaf_consistent() and spg_quad_inner_consistent().
Signed-off-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>
---
src/backend/access/spgist/spgquadtreeproc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/access/spgist/spgquadtreeproc.c b/src/backend/access/spgist/spgquadtreeproc.c
index e50108e1ca..f2e980b758 100644
--- a/src/backend/access/spgist/spgquadtreeproc.c
+++ b/src/backend/access/spgist/spgquadtreeproc.c
@@ -300,10 +300,11 @@ spg_quad_inner_consistent(PG_FUNCTION_ARGS)
for (i = 0; i < in->nkeys; i++)
{
- Point *query = DatumGetPointP(in->scankeys[i].sk_argument);
+ const ScanKey sk = in->scankeys + i;
+ Point *query = DatumGetPointP(sk->sk_argument);
BOX *boxQuery;
- switch (in->scankeys[i].sk_strategy)
+ switch (sk->sk_strategy)
{
case RTLeftStrategyNumber:
if (SPTEST(point_right, centroid, query))
@@ -331,7 +332,7 @@ spg_quad_inner_consistent(PG_FUNCTION_ARGS)
* cheat to the extent of assuming that DatumGetPointP won't
* do anything that would be bad for a pointer-to-box.
*/
- boxQuery = DatumGetBoxP(in->scankeys[i].sk_argument);
+ boxQuery = DatumGetBoxP(sk->sk_argument);
if (DatumGetBool(DirectFunctionCall2(box_contain_pt,
PointerGetDatum(boxQuery),
@@ -358,8 +359,7 @@ spg_quad_inner_consistent(PG_FUNCTION_ARGS)
}
break;
default:
- elog(ERROR, "unrecognized strategy number: %d",
- in->scankeys[i].sk_strategy);
+ elog(ERROR, "unrecognized strategy number: %d", sk->sk_strategy);
break;
}
@@ -421,9 +421,10 @@ spg_quad_leaf_consistent(PG_FUNCTION_ARGS)
res = true;
for (i = 0; i < in->nkeys; i++)
{
- Point *query = DatumGetPointP(in->scankeys[i].sk_argument);
+ const ScanKey sk = in->scankeys + i;
+ Point *query = DatumGetPointP(sk->sk_argument);
- switch (in->scankeys[i].sk_strategy)
+ switch (sk->sk_strategy)
{
case RTLeftStrategyNumber:
res = SPTEST(point_left, datum, query);
@@ -450,8 +451,7 @@ spg_quad_leaf_consistent(PG_FUNCTION_ARGS)
res = SPTEST(box_contain_pt, query, datum);
break;
default:
- elog(ERROR, "unrecognized strategy number: %d",
- in->scankeys[i].sk_strategy);
+ elog(ERROR, "unrecognized strategy number: %d", sk->sk_strategy);
break;
}
--
2.13.7
[PATCH 3/3] Add initial support for spgist quadtree @<(point,circle) operator
От
"Matwey V. Kornilov"
Дата:
Signed-off-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>
---
src/backend/access/spgist/spgquadtreeproc.c | 65 ++++++++++++++++---
src/include/catalog/pg_amop.dat | 3 +
src/test/regress/expected/create_index.out | 96 +++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 32 ++++++++++
4 files changed, 189 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/spgist/spgquadtreeproc.c b/src/backend/access/spgist/spgquadtreeproc.c
index 4904dbbe7b..034e706812 100644
--- a/src/backend/access/spgist/spgquadtreeproc.c
+++ b/src/backend/access/spgist/spgquadtreeproc.c
@@ -142,6 +142,36 @@ static int spg_quad_inner_consistent_box_helper(ScanKey sk, Point *centroid)
return r;
}
+static int spg_quad_inner_consistent_circle_helper(ScanKey sk, Point *centroid)
+{
+ CIRCLE *circleQuery = DatumGetCircleP(sk->sk_argument);
+ int r = 0;
+
+ const Point cp = {
+ .x = centroid->x - circleQuery->center.x,
+ .y = centroid->y - circleQuery->center.y
+ };
+ const Point cp2 = {
+ .x = cp.x * cp.x,
+ .y = cp.y * cp.y
+ };
+ const float8 R2 = circleQuery->radius * circleQuery->radius;
+
+ const float8 x_p0 = (0. > cp.x ? 0. : cp2.x);
+ const float8 y_p0 = (0. > cp.y ? 0. : cp2.y);
+ const float8 x_n0 = (0. < cp.x ? 0. : cp2.x);
+ const float8 y_n0 = (0. < cp.y ? 0. : cp2.y);
+
+ const float8 d[4] = {x_p0 + y_p0, x_p0 + y_n0, x_n0 + y_n0, x_n0 + y_p0};
+
+ for (int i = 0; i < 4; i++) {
+ if (d[i] <= R2)
+ r |= (1 << (i + 1));
+ }
+
+ return r;
+}
+
Datum
spg_quad_choose(PG_FUNCTION_ARGS)
{
@@ -355,7 +385,18 @@ spg_quad_inner_consistent(PG_FUNCTION_ARGS)
which &= (1 << 1) | (1 << 4);
break;
case RTContainedByStrategyNumber:
- which &= spg_quad_inner_consistent_box_helper(sk, centroid);
+
+ switch (sk->sk_subtype) {
+ case BOXOID:
+ which &= spg_quad_inner_consistent_box_helper(sk, centroid);
+ break;
+ case CIRCLEOID:
+ which &= spg_quad_inner_consistent_circle_helper(sk, centroid);
+ break;
+ default:
+ elog(ERROR, "unrecognized right type OID: %d", sk->sk_subtype);
+ break;
+ }
break;
default:
elog(ERROR, "unrecognized strategy number: %d", sk->sk_strategy);
@@ -442,12 +483,22 @@ spg_quad_leaf_consistent(PG_FUNCTION_ARGS)
break;
case RTContainedByStrategyNumber:
- /*
- * For this operator, the query is a box not a point. We
- * cheat to the extent of assuming that DatumGetPointP won't
- * do anything that would be bad for a pointer-to-box.
- */
- res = SPTEST(box_contain_pt, query, datum);
+ switch (sk->sk_subtype) {
+ case BOXOID:
+ /*
+ * For this operator, the query is a box not a point. We
+ * cheat to the extent of assuming that DatumGetPointP won't
+ * do anything that would be bad for a pointer-to-box.
+ */
+ res = SPTEST(box_contain_pt, query, datum);
+ break;
+ case CIRCLEOID:
+ res = SPTEST(circle_contain_pt, query, datum);
+ break;
+ default:
+ elog(ERROR, "unrecognized right type OID: %d", sk->sk_subtype);
+ break;
+ }
break;
default:
elog(ERROR, "unrecognized strategy number: %d", sk->sk_strategy);
diff --git a/src/include/catalog/pg_amop.dat b/src/include/catalog/pg_amop.dat
index 0ab95d8a24..fb7157aa44 100644
--- a/src/include/catalog/pg_amop.dat
+++ b/src/include/catalog/pg_amop.dat
@@ -1373,6 +1373,9 @@
amoprighttype => 'box', amopstrategy => '8', amopopr => '<@(point,box)',
amopmethod => 'spgist' },
{ amopfamily => 'spgist/quad_point_ops', amoplefttype => 'point',
+ amoprighttype => 'circle', amopstrategy => '8', amopopr => '<@(point,circle)',
+ amopmethod => 'spgist' },
+{ amopfamily => 'spgist/quad_point_ops', amoplefttype => 'point',
amoprighttype => 'point', amopstrategy => '15', amoppurpose => 'o',
amopopr => '<->(point,point)', amopmethod => 'spgist',
amopsortfamily => 'btree/float_ops' },
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 46deb55c67..4ff8745792 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -271,6 +271,102 @@ SELECT count(*) FROM quad_point_tbl WHERE box '(200,200,1000,1000)' @> p;
1057
(1 row)
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,399),1.41>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,399),1.42>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,401),1.41>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,401),1.42>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,399),1.41>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,399),1.42>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,401),1.41>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,401),1.42>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,399),0.99>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,399),1.01>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,401),0.99>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,401),1.01>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,400),0.99>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,400),1.01>';
+ count
+-------
+ 1000
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,400),0.99>';
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,400),1.01>';
+ count
+-------
+ 1000
+(1 row)
+
SELECT count(*) FROM quad_point_tbl WHERE p << '(5000, 4000)';
count
-------
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 59da6b6592..90d0f54792 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -188,6 +188,38 @@ SELECT count(*) FROM quad_point_tbl WHERE p <@ box '(200,200,1000,1000)';
SELECT count(*) FROM quad_point_tbl WHERE box '(200,200,1000,1000)' @> p;
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,399),1.41>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,399),1.42>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,401),1.41>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,401),1.42>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,399),1.41>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,399),1.42>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,401),1.41>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,401),1.42>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,399),0.99>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,399),1.01>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,401),0.99>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(333,401),1.01>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,400),0.99>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(332,400),1.01>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,400),0.99>';
+
+SELECT count(*) FROM quad_point_tbl WHERE p <@ circle '<(334,400),1.01>';
+
SELECT count(*) FROM quad_point_tbl WHERE p << '(5000, 4000)';
SELECT count(*) FROM quad_point_tbl WHERE p >> '(5000, 4000)';
--
2.13.7
Hi! On Fri, Feb 1, 2019 at 7:08 PM Matwey V. Kornilov <matwey.kornilov@gmail.com> wrote: > This patch series is to add support for spgist quadtree @<(point,circle) > operator. The first two patches are to refactor existing code before > implemention the new feature. The third commit is the actual implementation > provided with a set of simple unit tests. Cool! > Matwey V. Kornilov (3): > Introduce helper variable in spgquadtreeproc.c > Introduce spg_quad_inner_consistent_box_helper() in spgquadtreeproc.c > Add initial support for spgist quadtree @<(point,circle) operator At first, I have to note that it's not necessary to post every patch in separate message. It would be both easier and comfortable for readers if you just put your patches as multiple attachments to the same email message. Regarding the patchset itself * spg_quad_inner_consistent_circle_helper() definitely needs comments. * In PostgreSQL we require that index scan produce exactly same results as sequence scan. Can we ensure this is so for @<(point,circle) operator even in corner cases of rounding error? * In our coding style we have function name is the separate line from its return type. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
сб, 2 мар. 2019 г. в 12:14, Alexander Korotkov <a.korotkov@postgrespro.ru>: > > Hi! Thanks for reply. Comments and questions are below. > > On Fri, Feb 1, 2019 at 7:08 PM Matwey V. Kornilov > <matwey.kornilov@gmail.com> wrote: > > This patch series is to add support for spgist quadtree @<(point,circle) > > operator. The first two patches are to refactor existing code before > > implemention the new feature. The third commit is the actual implementation > > provided with a set of simple unit tests. > > Cool! > > > Matwey V. Kornilov (3): > > Introduce helper variable in spgquadtreeproc.c > > Introduce spg_quad_inner_consistent_box_helper() in spgquadtreeproc.c > > Add initial support for spgist quadtree @<(point,circle) operator > > At first, I have to note that it's not necessary to post every patch > in separate message. It would be both easier and comfortable for > readers if you just put your patches as multiple attachments to the > same email message. Ok, sorry, git send-email does embedded patches by default. Would it be even easier for you next time to receive github repo url and branch to pull from? > > Regarding the patchset itself > * spg_quad_inner_consistent_circle_helper() definitely needs comments. Ok > * In PostgreSQL we require that index scan produce exactly same > results as sequence scan. Can we ensure this is so for > @<(point,circle) operator even in corner cases of rounding error? I am not sure that fully understand the issue here. The operator is registered to have amopfamily = 'spgist/quad_point_ops', how could we use it without the index to do sequence scan? > * In our coding style we have function name is the separate line from > its return type. Ok > > ------ > Alexander Korotkov > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company -- With best regards, Matwey V. Kornilov