camertron / scuttle-rb

A library for transforming raw SQL statements into ActiveRecord/Arel queries. Ruby wrapper and tests for scuttle-java.
86 stars 2 forks source link

Window function OVER () not parsed correctly #10

Closed timkrins closed 5 years ago

timkrins commented 5 years ago

Just noticed an issue with the parser and had to dig a little deeper into the ActiveRecord/Arel source to figure it out the solution.

Problem arose when using window functions in Postgres, and giving each of my results the row number:

SELECT
row_number() OVER () AS id,
COUNT(*) as session_count,
FROM session_traffic_sources
GROUP BY session_traffic_sources.traffic_source

Scuttle is reporting the Arel equivalent as:

SessionTrafficSource.select(
  Arel::Nodes::NamedFunction.new('row_number', []).as('OVER')
).group(SessionTrafficSource.arel_table[:traffic_source])

1) It translates the named function incorrectly 2) It forgets about the count

Handwritten Arel that works correctly is:

SessionTrafficSource.select(
  [
    Arel::Nodes::NamedFunction.new('row_number', []).over(()).as('id'),
    Arel.star.count.as('session_count')
  ]
).group(Analytics::SessionTrafficSource.arel_table[:traffic_source])

Not a common one - but leaving this here in case any others run across a similar issue.

Thanks very much for the tool!

camertron commented 5 years ago

Hey @timkrins, thanks for the bug report! I think I was able to fix this, although I don't believe Arel supports anything other than a bare .over, i.e. no PARTITION BY. Should be live now.

See: https://github.com/camertron/scuttle-rb/pull/11 and https://github.com/camertron/scuttle-java/pull/3