Invalid SQL emitted for over(partition_by=[]) and over(order_by=[])
For queries like this:
select([func.sum(tab.c.id).over(partition_by=[](func.sum(tab.c.id).over(partition_by=[))])
invalid SQL is emitted:
SELECT sum(foo.id) OVER (PARTITION BY ) AS anon_1
FROM foo
Passing empty lists has a reasonably well defined meaning (don't partition/order by anything), and may arise as a consequence of code like partition_by=list(determine_partition_expressions())
, where restructuring the code to not pass the parameter if its value will be empty is an unnecessary inconvenience.
Submitting a patch with tests.
Comments (11)
-
Account Deleted -
Account Deleted Oh, patch also includes test for
over()
with no parameters — that was already working, but no harm having a test for it. -
Account Deleted Patch against default branch
-
repo owner - changed milestone to 0.7.9
can this patch be done more simply by just asking for "if clause.order_by is not None and len(clause.order_by)" ? would be more succinct.
-
Account Deleted Sure. Attaching two alternative implementations, one that changes things minimally the way you describe, and another that eliminates redundancy but reads more functionally. Both pass the tests.
-
Account Deleted Alternative patch, minimally changed to be more succinct. Against rel_0_7
-
Account Deleted Another alternative patch, using ' '.join and a generator expression
-
Account Deleted These are both against rel_0_7 but they merge trivially to the default branch
-
repo owner - marked as critical
-
repo owner - changed status to resolved
-
repo owner - removed milestone
Removing milestone: 0.7.9 (automated comment)
- Log in to comment
Patch against rel_0_7