Re: [sqlalchemy] sqlalchemy.select().group_by(expr) doesn't use label of expression, while .order_by(expr) does

[Mike Bayer]

On Wed, Oct 25, 2017 at 8:55 AM, Gijs Molenaar  wrote:
>
>
> Op vrijdag 20 oktober 2017 19:11:56 UTC+2 schreef Mike Bayer:
>>
>> On Thu, Oct 19, 2017 at 4:25 AM, Gijs Molenaar 
>> wrote:
>> >
>> >
>> > Op donderdag 19 oktober 2017 03:10:21 UTC+2 schreef Mike Bayer:
>> >>
>> >> On Wed, Oct 18, 2017 at 7:38 AM, Gijs Molenaar 
>> >> wrote:
>> >> > hi!
>> >> >
>> >> >
>> >> > Not sure if this a bug or something I should in my SQLAlchemy
>> >> > dialect,
>> >> > but
>> >> > currently
>> >> >
>> >> >
>> >> > expr = (table.c.x + table.c.y).label('lx')
>> >> > select([func.count(table.c.id), expr]).group_by(expr).order_by(expr)
>> >> >
>> >> > compiles to:
>> >> >
>> >> > SELECT count(some_table.id) AS count_1, some_table.x + some_table.y
>> >> > AS
>> >> > lx
>> >> > \nFROM some_table GROUP BY some_table.x + some_table.y ORDER BY lx;
>> >> >
>> >> >
>> >> > which works fine for for example sqlite, but MonetDB requires the use
>> >> > of
>> >> > the
>> >> > lx label in the GROUP BY, which I think makes sense? Should this be
>> >> > addressed on the SQLalchemy side or on the MonetDB dialect side?
>> >>
>> >> so this was overhauled in
>> >>
>> >>
>> >> http://docs.sqlalchemy.org/en/latest/changelog/migration_09.html#label-constructs-can-now-render-as-their-name-alone-in-an-order-by
>> >> where we changed ORDER BY to use the label name when the expression is
>> >> passed.
>> >>
>> >> so the immediate answer would be to not actually order by the label().
>> >
>> >
>> > I think i didn't formulate my e-mail correctly. The ORDER BY is not the
>> > problem, it is the GROUP BY that requires a label. If the GROUP BY
>> > doesn't
>> > use a label (just like ORDER BY), MonetDB doesn't want to eat the query.
>>
>> this is my fault because I read/reply to these emails very fast and
>> focus mainly on the code I see.If you want it the other way
>> around, there is the feature described at:
>>
>>
>> https://docs.sqlalchemy.org/en/latest/changelog/migration_10.html#order-by-and-group-by-are-special-cases
>>
>> that is, say group_by("somelabel").
>>
>> If you need this to be more automatic for this backend, we can look
>> into seeing how the "ORDER BY" version of the feature can be more
>> generalized on a backend-specific basis but this would be longer-term.
>>
>
> Hi Mike,
>
> My main concern is to get the test suite pass for now :) What you propose
> doesn't make the dialect work with the current test suite right?

I assume this is the single test test_group_by_composed, for the
immediate case you can override it in your local test_suite.py file.
 We can add an @exclusion for it too.

>
> greetings,
>
>  - Gijs
>
> --
> 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+unsubscr...@googlegroups.com.
>
> To post to this group, send email to sqlalchemy@googlegroups.com.
> Visit this group at https://groups.google.com/group/sqlalchemy.
> For more options, visit https://groups.google.com/d/optout.

-- 
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+unsubscr...@googlegroups.com.
To post to this group, send email to sqlalchemy@googlegroups.com.
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.

Re: [sqlalchemy] sqlalchemy.select().group_by(expr) doesn't use label of expression, while .order_by(expr) does

[Gijs Molenaar]

Op vrijdag 20 oktober 2017 19:11:56 UTC+2 schreef Mike Bayer:
>
> On Thu, Oct 19, 2017 at 4:25 AM, Gijs Molenaar  > wrote: 
> > 
> > 
> > Op donderdag 19 oktober 2017 03:10:21 UTC+2 schreef Mike Bayer: 
> >> 
> >> On Wed, Oct 18, 2017 at 7:38 AM, Gijs Molenaar  
> >> wrote: 
> >> > hi! 
> >> > 
> >> > 
> >> > Not sure if this a bug or something I should in my SQLAlchemy 
> dialect, 
> >> > but 
> >> > currently 
> >> > 
> >> > 
> >> > expr = (table.c.x + table.c.y).label('lx') 
> >> > select([func.count(table.c.id), expr]).group_by(expr).order_by(expr) 
> >> > 
> >> > compiles to: 
> >> > 
> >> > SELECT count(some_table.id) AS count_1, some_table.x + some_table.y 
> AS 
> >> > lx 
> >> > \nFROM some_table GROUP BY some_table.x + some_table.y ORDER BY lx; 
> >> > 
> >> > 
> >> > which works fine for for example sqlite, but MonetDB requires the use 
> of 
> >> > the 
> >> > lx label in the GROUP BY, which I think makes sense? Should this be 
> >> > addressed on the SQLalchemy side or on the MonetDB dialect side? 
> >> 
> >> so this was overhauled in 
> >> 
> >> 
> http://docs.sqlalchemy.org/en/latest/changelog/migration_09.html#label-constructs-can-now-render-as-their-name-alone-in-an-order-by
>  
> >> where we changed ORDER BY to use the label name when the expression is 
> >> passed. 
> >> 
> >> so the immediate answer would be to not actually order by the label(). 
> > 
> > 
> > I think i didn't formulate my e-mail correctly. The ORDER BY is not the 
> > problem, it is the GROUP BY that requires a label. If the GROUP BY 
> doesn't 
> > use a label (just like ORDER BY), MonetDB doesn't want to eat the query. 
>
> this is my fault because I read/reply to these emails very fast and 
> focus mainly on the code I see.If you want it the other way 
> around, there is the feature described at: 
>
>
> https://docs.sqlalchemy.org/en/latest/changelog/migration_10.html#order-by-and-group-by-are-special-cases
>  
>
> that is, say group_by("somelabel"). 
>
> If you need this to be more automatic for this backend, we can look 
> into seeing how the "ORDER BY" version of the feature can be more 
> generalized on a backend-specific basis but this would be longer-term. 
>
>
Hi Mike,

My main concern is to get the test suite pass for now :) What you propose 
doesn't make the dialect work with the current test suite right?

greetings,

 - Gijs 

-- 
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+unsubscr...@googlegroups.com.
To post to this group, send email to sqlalchemy@googlegroups.com.
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.

Re: [sqlalchemy] sqlalchemy.select().group_by(expr) doesn't use label of expression, while .order_by(expr) does

[Mike Bayer]

On Thu, Oct 19, 2017 at 4:25 AM, Gijs Molenaar  wrote:
>
>
> Op donderdag 19 oktober 2017 03:10:21 UTC+2 schreef Mike Bayer:
>>
>> On Wed, Oct 18, 2017 at 7:38 AM, Gijs Molenaar 
>> wrote:
>> > hi!
>> >
>> >
>> > Not sure if this a bug or something I should in my SQLAlchemy dialect,
>> > but
>> > currently
>> >
>> >
>> > expr = (table.c.x + table.c.y).label('lx')
>> > select([func.count(table.c.id), expr]).group_by(expr).order_by(expr)
>> >
>> > compiles to:
>> >
>> > SELECT count(some_table.id) AS count_1, some_table.x + some_table.y AS
>> > lx
>> > \nFROM some_table GROUP BY some_table.x + some_table.y ORDER BY lx;
>> >
>> >
>> > which works fine for for example sqlite, but MonetDB requires the use of
>> > the
>> > lx label in the GROUP BY, which I think makes sense? Should this be
>> > addressed on the SQLalchemy side or on the MonetDB dialect side?
>>
>> so this was overhauled in
>>
>> http://docs.sqlalchemy.org/en/latest/changelog/migration_09.html#label-constructs-can-now-render-as-their-name-alone-in-an-order-by
>> where we changed ORDER BY to use the label name when the expression is
>> passed.
>>
>> so the immediate answer would be to not actually order by the label().
>
>
> I think i didn't formulate my e-mail correctly. The ORDER BY is not the
> problem, it is the GROUP BY that requires a label. If the GROUP BY doesn't
> use a label (just like ORDER BY), MonetDB doesn't want to eat the query.

this is my fault because I read/reply to these emails very fast and
focus mainly on the code I see.If you want it the other way
around, there is the feature described at:

https://docs.sqlalchemy.org/en/latest/changelog/migration_10.html#order-by-and-group-by-are-special-cases

that is, say group_by("somelabel").

If you need this to be more automatic for this backend, we can look
into seeing how the "ORDER BY" version of the feature can be more
generalized on a backend-specific basis but this would be longer-term.



>
>>
>> At the compiler level, this behavior can be disabled across the board like
>> this:
>>
>> diff --git a/lib/sqlalchemy/dialects/sqlite/base.py
>> b/lib/sqlalchemy/dialects/sqlite/base.py
>> index d8ce7f394..389d9bb02 100644
>> --- a/lib/sqlalchemy/dialects/sqlite/base.py
>> +++ b/lib/sqlalchemy/dialects/sqlite/base.py
>> @@ -800,6 +800,10 @@ class SQLiteCompiler(compiler.SQLCompiler):
>>  'week': '%W',
>>  })
>>
>> +def visit_label(self, elem, **kw):
>> +kw.pop('render_label_as_label', None)
>> +return super(SQLiteCompiler, self).visit_label(elem, **kw)
>> +
>>  def visit_now_func(self, fn, **kw):
>>  return "CURRENT_TIMESTAMP"
>>
>>
>>
>> however no dialect does that right now, you'd need to watch this
>> behavior closely across SQLAlchemy releases to make sure it doesn't
>> break, unless we make this behavior official.
>>
>>
>>
>>
>> >
>> >
>> > related issue:
>> >
>> >
>> > https://github.com/gijzelaerr/sqlalchemy-monetdb/issues/21
>> >
>> > greetings,
>> >
>> >  - Gijs
>> >
>> > --
>> > 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 post to this group, send email to sqlal...@googlegroups.com.
>> > Visit this group at https://groups.google.com/group/sqlalchemy.
>> > For more options, visit https://groups.google.com/d/optout.
>
> --
> 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+unsubscr...@googlegroups.com.
> To post to this group, send email to sqlalchemy@googlegroups.com.
> Visit this group at https://groups.google.com/group/sqlalchemy.
> For more options, visit https://groups.google.com/d/optout.

-- 
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+unsubscr...@googlegroups.com.
To post to this group, 

Re: [sqlalchemy] sqlalchemy.select().group_by(expr) doesn't use label of expression, while .order_by(expr) does

[Gijs Molenaar]

Op donderdag 19 oktober 2017 03:10:21 UTC+2 schreef Mike Bayer:
>
> On Wed, Oct 18, 2017 at 7:38 AM, Gijs Molenaar  > wrote: 
> > hi! 
> > 
> > 
> > Not sure if this a bug or something I should in my SQLAlchemy dialect, 
> but 
> > currently 
> > 
> > 
> > expr = (table.c.x + table.c.y).label('lx') 
> > select([func.count(table.c.id), expr]).group_by(expr).order_by(expr) 
> > 
> > compiles to: 
> > 
> > SELECT count(some_table.id) AS count_1, some_table.x + some_table.y AS 
> lx 
> > \nFROM some_table GROUP BY some_table.x + some_table.y ORDER BY lx; 
> > 
> > 
> > which works fine for for example sqlite, but MonetDB requires the use of 
> the 
> > lx label in the GROUP BY, which I think makes sense? Should this be 
> > addressed on the SQLalchemy side or on the MonetDB dialect side? 
>
> so this was overhauled in 
>
> http://docs.sqlalchemy.org/en/latest/changelog/migration_09.html#label-constructs-can-now-render-as-their-name-alone-in-an-order-by
>  
> where we changed ORDER BY to use the label name when the expression is 
> passed. 
>
> so the immediate answer would be to not actually order by the label(). 
>

I think i didn't formulate my e-mail correctly. The ORDER BY is not the 
problem, it is the GROUP BY that requires a label. If the GROUP BY doesn't 
use a label (just like ORDER BY), MonetDB doesn't want to eat the query. 


> At the compiler level, this behavior can be disabled across the board like 
> this: 
>
> diff --git a/lib/sqlalchemy/dialects/sqlite/base.py 
> b/lib/sqlalchemy/dialects/sqlite/base.py 
> index d8ce7f394..389d9bb02 100644 
> --- a/lib/sqlalchemy/dialects/sqlite/base.py 
> +++ b/lib/sqlalchemy/dialects/sqlite/base.py 
> @@ -800,6 +800,10 @@ class SQLiteCompiler(compiler.SQLCompiler): 
>  'week': '%W', 
>  }) 
>
> +def visit_label(self, elem, **kw): 
> +kw.pop('render_label_as_label', None) 
> +return super(SQLiteCompiler, self).visit_label(elem, **kw) 
> + 
>  def visit_now_func(self, fn, **kw): 
>  return "CURRENT_TIMESTAMP" 
>
>
>
> however no dialect does that right now, you'd need to watch this 
> behavior closely across SQLAlchemy releases to make sure it doesn't 
> break, unless we make this behavior official. 
>
>
>
>
> > 
> > 
> > related issue: 
> > 
> > 
> > https://github.com/gijzelaerr/sqlalchemy-monetdb/issues/21 
> > 
> > greetings, 
> > 
> >  - Gijs 
> > 
> > -- 
> > 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 post to this group, send email to sqlal...@googlegroups.com 
> . 
> > Visit this group at https://groups.google.com/group/sqlalchemy. 
> > For more options, visit https://groups.google.com/d/optout. 
>

-- 
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+unsubscr...@googlegroups.com.
To post to this group, send email to sqlalchemy@googlegroups.com.
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.

Re: [sqlalchemy] sqlalchemy.select().group_by(expr) doesn't use label of expression, while .order_by(expr) does

[Mike Bayer]

On Wed, Oct 18, 2017 at 7:38 AM, Gijs Molenaar  wrote:
> hi!
>
>
> Not sure if this a bug or something I should in my SQLAlchemy dialect, but
> currently
>
>
> expr = (table.c.x + table.c.y).label('lx')
> select([func.count(table.c.id), expr]).group_by(expr).order_by(expr)
>
> compiles to:
>
> SELECT count(some_table.id) AS count_1, some_table.x + some_table.y AS lx
> \nFROM some_table GROUP BY some_table.x + some_table.y ORDER BY lx;
>
>
> which works fine for for example sqlite, but MonetDB requires the use of the
> lx label in the GROUP BY, which I think makes sense? Should this be
> addressed on the SQLalchemy side or on the MonetDB dialect side?

so this was overhauled in
http://docs.sqlalchemy.org/en/latest/changelog/migration_09.html#label-constructs-can-now-render-as-their-name-alone-in-an-order-by
where we changed ORDER BY to use the label name when the expression is
passed.

so the immediate answer would be to not actually order by the label().

At the compiler level, this behavior can be disabled across the board like this:

diff --git a/lib/sqlalchemy/dialects/sqlite/base.py
b/lib/sqlalchemy/dialects/sqlite/base.py
index d8ce7f394..389d9bb02 100644
--- a/lib/sqlalchemy/dialects/sqlite/base.py
+++ b/lib/sqlalchemy/dialects/sqlite/base.py
@@ -800,6 +800,10 @@ class SQLiteCompiler(compiler.SQLCompiler):
 'week': '%W',
 })

+def visit_label(self, elem, **kw):
+kw.pop('render_label_as_label', None)
+return super(SQLiteCompiler, self).visit_label(elem, **kw)
+
 def visit_now_func(self, fn, **kw):
 return "CURRENT_TIMESTAMP"



however no dialect does that right now, you'd need to watch this
behavior closely across SQLAlchemy releases to make sure it doesn't
break, unless we make this behavior official.




>
>
> related issue:
>
>
> https://github.com/gijzelaerr/sqlalchemy-monetdb/issues/21
>
> greetings,
>
>  - Gijs
>
> --
> 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+unsubscr...@googlegroups.com.
> To post to this group, send email to sqlalchemy@googlegroups.com.
> Visit this group at https://groups.google.com/group/sqlalchemy.
> For more options, visit https://groups.google.com/d/optout.

-- 
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+unsubscr...@googlegroups.com.
To post to this group, send email to sqlalchemy@googlegroups.com.
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.