patch for between symmetric, asymmetric (from TODO)

Pavel Stehule wrote:

I did task from TODO: Add BETWEEN ASYMMETRIC/SYMMETRIC.

The implementation in this patch has the same problems as all the
previously rejected attempts: it evaluates its arguments twice. You
need to make BETWEEN SYMMETRIC into a separate node type that evaluates
each argument only once.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:

I did task from TODO: Add BETWEEN ASYMMETRIC/SYMMETRIC.
this patch is based on Robert's B. Easter work from 2001 year.
http://archives.postgresql.org/pgsql-patches/2001-01/msg00022.php

IIRC, that patch was rejected at the time because of performance issues
--- the optimizer could not turn it into an indexscan.  I think that
problem may have gone away by now, but did you check?

Also, you neglected to add the new keywords to the right keyword list
(so that they won't be any more reserved than necessary).

Also, a change like this is hardly just in the parser. Please include
the needed documentation changes. It's simple enough to probably not
need a regression test, but that's always something to think about when
completing a TODO, too.

regards, tom lane

Peter Eisentraut <peter_e@gmx.net> writes:

Pavel Stehule wrote:

I did task from TODO: Add BETWEEN ASYMMETRIC/SYMMETRIC.

The implementation in this patch has the same problems as all the
previously rejected attempts: it evaluates its arguments twice. You
need to make BETWEEN SYMMETRIC into a separate node type that evaluates
each argument only once.

Sure, but BETWEEN already does that; Pavel's not making that situation
any worse. The trouble with making it a separate node type is that that
raises the stakes a great deal concerning the amount of planner support
needed (assuming you still want indexability...)

regards, tom lane

On Wed, 1 Jun 2005, Tom Lane wrote:

Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:

I did task from TODO: Add BETWEEN ASYMMETRIC/SYMMETRIC.
this patch is based on Robert's B. Easter work from 2001 year.
http://archives.postgresql.org/pgsql-patches/2001-01/msg00022.php

I started working on own node type version. Between symmetric use index,
but twice :-(.

pokus=# explain analyze select * from fx where i between 1 and 10;
QUERY PLAN
------------------------------------------------------------------------------------------------------------------
Bitmap Heap Scan on fx (cost=6.00..475.59 rows=500 width=4) (actual
time=0.547..3.830 rows=1035 loops=1)
Recheck Cond: ((i >= 1) AND (i <= 10))
-> Bitmap Index Scan on fxxx (cost=0.00..6.00 rows=500 width=0)
(actual time=0.408..0.408 rows=1035 loops=1)
Index Cond: ((i >= 1) AND (i <= 10))
Total runtime: 6.373 ms
(5 rows)

pokus=# explain analyze select * from fx where i between symmetric 1 and
10;
QUERY PLAN
------------------------------------------------------------------------------------------------------------------------
Bitmap Heap Scan on fx (cost=12.00..473.00 rows=998 width=4) (actual
time=0.516..3.855 rows=1035 loops=1)
Recheck Cond: (((i >= 1) AND (i <= 10)) OR ((i >= 10) AND (i <= 1)))
-> BitmapOr (cost=12.00..12.00 rows=1000 width=0) (actual
time=0.377..0.377 rows=0 loops=1)
-> Bitmap Index Scan on fxxx (cost=0.00..6.00 rows=500 width=0)
(actual time=0.362..0.362 rows=1035 loops=1)
Index Cond: ((i >= 1) AND (i <= 10))
-> Bitmap Index Scan on fxxx (cost=0.00..6.00 rows=500 width=0)
(actual time=0.007..0.007 rows=0 loops=1)
Index Cond: ((i >= 10) AND (i <= 1))
Total runtime: 6.412 ms

IIRC, that patch was rejected at the time because of performance issues
--- the optimizer could not turn it into an indexscan.  I think that
problem may have gone away by now, but did you check?

better is sort operands before and next test. Without it, between produce
condition which is allways false. But planner dont recognize it.

See:

Index Cond: ((i >= 10) AND (i <= 1))

Also, you neglected to add the new keywords to the right keyword list
(so that they won't be any more reserved than necessary).

SYMMETRIC and ASYMMETRIC are SQL keywords, but its true so this are often
used identificators.

Also, a change like this is hardly just in the parser. Please include
the needed documentation changes. It's simple enough to probably not
need a regression test, but that's always something to think about when
completing a TODO, too.

I need help with documentation. I am sorry, but my language skills are
terrible. I can write basic part of documentation for it.

Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:

Bitmap Heap Scan on fx (cost=12.00..473.00 rows=998 width=4) (actual
time=0.516..3.855 rows=1035 loops=1)
Recheck Cond: (((i >= 1) AND (i <= 10)) OR ((i >= 10) AND (i <= 1)))
-> BitmapOr (cost=12.00..12.00 rows=1000 width=0) (actual
time=0.377..0.377 rows=0 loops=1)
-> Bitmap Index Scan on fxxx (cost=0.00..6.00 rows=500 width=0)
(actual time=0.362..0.362 rows=1035 loops=1)
Index Cond: ((i >= 1) AND (i <= 10))
-> Bitmap Index Scan on fxxx (cost=0.00..6.00 rows=500 width=0)
(actual time=0.007..0.007 rows=0 loops=1)
Index Cond: ((i >= 10) AND (i <= 1))
Total runtime: 6.412 ms

better is sort operands before and next test. Without it, between produce
condition which is allways false. But planner dont recognize it.

No, but the btree index code does --- notice the very small actual time
there. I don't think there's anything seriously wrong with the above
plan.

regards, tom lane

The implementation in this patch has the same problems as all the
previously rejected attempts: it evaluates its arguments twice. You
need to make BETWEEN SYMMETRIC into a separate node type that evaluates
each argument only once.

And that's also been submitted. The problem then is making the
optimizer recognise it as a range query...

Chris