On new native PG Range in SA 2.0

[Lele Gaifax]

Hi,

as stated in https://docs.sqlalchemy.org/en/20/dialects/postgresql.html#range-and-multirange-types,
in SA 2.0 the PG "range" (and "multirange") types are wrapped in a new
Range class, while the underlying driver-specific class is "hidden"
within the dialect implementation.

This means that many operations that were previously exposed to the
Python logic are not available anymore, and I'm not sure if that is
intentional, not-yet-implemented functionalities or instead just me
missing something.

For example, one of the trickiest things I would need to reimplement is
the Python-side equivalent of the "<@" operator, that checks whether an
element is contained in a range: in SA 1.x this was readily exposed by
the driver implementation, usually by a `__contains__` method
(see https://github.com/psycopg/psycopg2/blob/master/lib/_range.py#L121
for psycopg2).

What do you foresee, that the Range class should/will "proxies" those
capabilities, or instead that I should change (pseudo)code like::

class Contract:
active = Column(bool)
validity = Column(daterange)

def is_currently_active(self):
return self.active and date.today() in self.validity

to something different, that in some way "reaches" the underlying
psycopg (say) implementation and uses its logic?

Thanks in advance,
ciao, lele.
--
nickname: Lele Gaifax | Dire che Emacs è "conveniente" è come
real: Emanuele Gaifas | etichettare l'ossigeno come "utile"
le...@etour.tn.it | -- Rens Troost

[Mike Bayer]

On Sat, Oct 22, 2022, at 6:57 AM, Lele Gaifax wrote:

Hi,

as stated in https://docs.sqlalchemy.org/en/20/dialects/postgresql.html#range-and-multirange-types, 

in SA 2.0 the PG "range" (and "multirange") types are wrapped in a new

Range class, while the underlying driver-specific class is "hidden"

within the dialect implementation.

This means that many operations that were previously exposed to the

Python logic are not available anymore, and I'm not sure if that is

intentional, not-yet-implemented functionalities or instead just me

missing something.

we were going for the lowest common denominator of functionality, which was to be able to persist and load Range objects.   If you look at asyncpg's Range type, it's very basic: https://github.com/MagicStack/asyncpg/blob/eccdf61afb0116f9500f6fb2f832058ba8eb463e/asyncpg/types.py#L42

We looked at the methods that psycopg2's Range has and they seemed to be related related to psycopg2's own internal needs, such as the comparison operator implementations, which don't make any real sense and are documented as "arbitrary": https://github.com/psycopg/psycopg2/blob/20bb48666312e6f4747d1f6187f520c812f29591/lib/_range.py#L159

More obvious methods like issubset and issuperset (https://github.com/MagicStack/asyncpg/blob/master/asyncpg/types.py#L110) are easy to add and are just a pull request / issue away.   as for psycopg2's __contains__, that also looks very simple but I'd rather have it as an explicit name like "contains_value()".

As far as the Multirange type in psycopg3, I looked at that and the features it had seemed to be redundant vs. what you can do with a plain list, and indeed asyncpg implements multirange using lists of Ranges which IMO is much more natural and easy to work with.

For example, one of the trickiest things I would need to reimplement is

the Python-side equivalent of the "<@" operator, that checks whether an

element is contained in a range: in SA 1.x this was readily exposed by

the driver implementation, usually by a `__contains__` method

(see https://github.com/psycopg/psycopg2/blob/master/lib/_range.py#L121

for psycopg2).

Similar methods are a PR (with tests) away from being included.

What do you foresee, that the Range class should/will "proxies" those

capabilities, or instead that I should change (pseudo)code like::

Range is a really simple dataclass so adding a few simple value methods contains_value(), issubset(), issuperset() is a PR (with tests) away.   there's no need to "proxy" things, these are pretty obvious methods to just implement directly.   I would just disagree with psycopg2's more nonsensical methods like `__lt__`, `__gt__`, etc.

[Lele Gaifax]

"Mike Bayer" <mik...@zzzcomputing.com> writes:

> Range is a really simple dataclass so adding a few simple value
> methods contains_value(), issubset(), issuperset() is a PR (with
> tests) away. there's no need to "proxy" things, these are pretty
> obvious methods to just implement directly. I would just disagree with
> psycopg2's more nonsensical methods like `__lt__`, `__gt__`, etc.
>

Thank you for the directions: while I'm not 100% convinced about
avoiding __contains__(), which seems very intuitive and practical, I
will propose a PR soon.

ciao, lele.
--
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
le...@metapensiero.it | -- Fortunato Depero, 1929.

[Lele Gaifax]

Lele Gaifax <le...@metapensiero.it> writes:

> I will propose a PR soon.

An ethical concern: given that the proposed methods are so simple, is it
legal/acceptable to verbatim copy them, and their tests, from the
psycopg (LGPL) and asyncpg (Apache license)?

[Mike Bayer]

On Sun, Oct 23, 2022, at 4:13 AM, Lele Gaifax wrote:

"Mike Bayer" <mik...@zzzcomputing.com> writes:

> Range is a really simple dataclass so adding a few simple value

> methods contains_value(), issubset(), issuperset() is a PR (with

> tests) away. there's no need to "proxy" things, these are pretty

> obvious methods to just implement directly. I would just disagree with

> psycopg2's more nonsensical methods like `__lt__`, `__gt__`, etc.

>

Thank you for the directions: while I'm not 100% convinced about

avoiding __contains__(), which seems very intuitive and practical, I

will propose a PR soon.

Yes, I just like to avoid `__contains__()` at the outset until we can think about it some more, because once you implement the "in" operator, you can never really change it.   

ciao, lele.

-- 

nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri

real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.

le...@metapensiero.it  |                 -- Fortunato Depero, 1929.

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/87ilkb9cvm.fsf%40metapensiero.it.

[Mike Bayer]

On Sun, Oct 23, 2022, at 5:04 AM, Lele Gaifax wrote:

Lele Gaifax <le...@metapensiero.it> writes:

> I will propose a PR soon.

An ethical concern: given that the proposed methods are so simple, is it

legal/acceptable to verbatim copy them, and their tests, from the

psycopg (LGPL) and asyncpg (Apache license)?

I would say that it's not!    I wouldn't really look at the method bodies here and I wouldn't copy them and for tests we have our own testing formats and styles that wouldn't match those other libraries anyway.

in my view, these are very trivial methods with trivial tests, just comparing upper/lower bounds to some value.   It's not like we're copying a network protocol implementation or something.

ciao, lele.

-- 

nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri

real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.

le...@metapensiero.it  |                 -- Fortunato Depero, 1929.

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/878rl6ap2z.fsf%40metapensiero.it.

[Lele Gaifax]

Hi,

this is my first cut at the proposed methods:

https://github.com/lelit/sqlalchemy/commit/b9a2a7761ec292536a7be37703d20800edfab3f0

I'm not quite happy with how I wrote the tests, because I didn't find a
reasonable way to get the PG representation of a given Range instance,
and thus I manually coded those values.

For that reason I didn't (yet) created an issue and proposed a PR.

Mike, can you tell me if this is acceptable or how can I improve it?

Thanks&bye, lele.

[Mike Bayer]

these look super great!  We can make the text("") statements into something nicer in a second pass, is that the part you didn't like ?

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/878rl5uqbq.fsf%40metapensiero.it.

[Lele Gaifax]

"Mike Bayer" <mik...@zzzcomputing.com> writes:

> these look super great!

Thanks! I will then open an issue and a related PR shortly.

> We can make the text("") statements into something nicer in a second
> pass, is that the part you didn't like ?

Not exactly: I spent some time trying to avoid the "redundancy" in the
tests parametrization, ie

@testing.combinations(
(Range(empty=True), 'empty'),
(Range(None, None, bounds='()'), '(,)'),
(Range(None, 4, bounds='(]'), '(,4]'),
...

ideally simplifying it to

@testing.combinations(
Range(empty=True),
Range(None, None, bounds='()'),
Range(None, 4, bounds='(]'),
...

and then being able to "compute" the SQL query from the given Range
instance alone, but I couldn't figure out how to obtain the
"compilation" of just a single value from the connection.dialect.

But hey, I do not expected those arguments will be changing often... :-)

ciao, lele

[Mike Bayer]

On Mon, Oct 24, 2022, at 2:37 PM, Lele Gaifax wrote:

"Mike Bayer" <mik...@zzzcomputing.com> writes:

> these look super great!

Thanks! I will then open an issue and a related PR shortly.

> We can make the text("") statements into something nicer in a second

> pass, is that the part you didn't like ?

Not exactly: I spent some time trying to avoid the "redundancy" in the

tests parametrization, ie

    @testing.combinations(

        (Range(empty=True), 'empty'),

        (Range(None, None, bounds='()'), '(,)'),

        (Range(None, 4, bounds='(]'), '(,4]'),

        ...

ideally simplifying it to

    @testing.combinations(

        Range(empty=True),

        Range(None, None, bounds='()'),

        Range(None, 4, bounds='(]'),

        ...

and then being able to "compute" the SQL query from the given Range

instance alone, but I couldn't figure out how to obtain the

"compilation" of just a single value from the connection.dialect.

But hey, I do not expected those arguments will be changing often... :-)

OK, we usually go for what's expedient as long as it covers all cases and avoids major future-compatibility issues where possible

ciao, lele

-- 

nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri

real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.

le...@metapensiero.it  |                 -- Fortunato Depero, 1929.

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/874jvtukxz.fsf%40metapensiero.it.