OVER clause lost from row_number functions.

[pat]

I'm finding that - in both v3.3.0 and v3.3.1 - the OVER part of a ROW_NUMBER() function is lost from the generated SQL statement. I'm guessing that it's due to #373, but that's purely because it seems to be the only related change since v3.2.0. Perhaps it's something else? 🤷🏻‍♂️

The ROW_NUMBER() and alias both remain, but the OVER (...) part is lost somehow. Here's a script that reproduces the issue (and if you change the bundled version of rom-sql to 3.2.0, you'll find the tests pass:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "rom", "5.2.1"
  gem "rom-sql", "3.3.1"
  gem "sqlite3"
  gem "rspec"
end

RSpec.describe "over window persists" do
  let(:products) { rom.relations[:products] }
  let(:collections_products) { rom.relations[:collections_products] }

  let(:rom) do
    rom = ROM.container(:sql, "sqlite::memory") do |conf|
      conf.default.create_table(:products) do
        primary_key :id
        column :name, String, null: false
      end

      conf.default.create_table(:collections) do
        primary_key :id
        column :name, String, null: false
      end

      conf.default.create_table(:collections_products) do
        foreign_key :collection_id, :users, null: false
        foreign_key :product_id, :users, null: false
        column :position, Integer, null: false
      end

      conf.relation(:products) do
        schema(infer: true) do
          associations do
            has_many :collections_products
          end
        end
      end

      conf.relation(:collections_products) do
        schema(infer: true)
      end
    end
  end

  it "holds onto the over conditions for single table queries" do
    relation = products
      .select_append {
        [
          integer::row_number().over(
            order: :name,
          ).as(:row_numbers),
        ]
      }

    expect(relation.dataset.sql).to eq("SELECT `products`.`id`, `products`.`name`, ROW_NUMBER() OVER (ORDER BY `name`) AS 'row_numbers' FROM `products` ORDER BY `products`.`id`")

# expected: "SELECT `products`.`id`, `products`.`name`, ROW_NUMBER() OVER (ORDER BY `name`) AS 'row_numbers' FROM `products` ORDER BY `products`.`id`"
#      got: "SELECT `products`.`id`, `products`.`name`, ROW_NUMBER() AS 'row_numbers' FROM `products` ORDER BY `products`.`id`"
  end

  it "holds onto the over conditions for associations" do
    relation = products.join(:collections_products)
      .select_append { |collections_products:|
        [
          integer::row_number().over(
            partition: collections_products[:collection_id],
            order: collections_products[:position],
          ).as(:row_numbers),
        ]
      }

    expect(relation.dataset.sql).to eq("SELECT `products`.`id`, `products`.`name`, ROW_NUMBER() OVER (PARTITION BY `collections_products`.`collection_id` ORDER BY `collections_products`.`position`) AS 'row_numbers' FROM `products` INNER JOIN `collections_products` ON (`products`.`id` = `collections_products`.`product_id`) ORDER BY `products`.`id`")

# expected: "SELECT `products`.`id`, `products`.`name`, ROW_NUMBER() OVER (PARTITION BY `collections_products`.`c...tions_products` ON (`products`.`id` = `collections_products`.`product_id`) ORDER BY `products`.`id`"
#      got: "SELECT `products`.`id`, `products`.`name`, ROW_NUMBER() AS 'row_numbers' FROM `products` INNER JOIN ...tions_products` ON (`products`.`id` = `collections_products`.`product_id`) ORDER BY `products`.`id`"
  end
end

My environment

• Affects my production application: NO (CI + dependabot found the bug, so the PR isn't merged in).
• Ruby version: ruby 2.6.6p146 (2020-03-31 revision 67876) [x86_64-darwin19]
• OS: macOS 11.1 (though also occurs on Ubuntu for CI)

[solnic]

@pat hey, thanks for the report. It wasn't easy to fix but I'm working on it in #391. The trick is to figure out how to preserve the underlying Sequel function with its opts 😅

[pat]

Thanks for looking into it @solnic :) Hopefully it's not too much of a headache! 🤞🏻

[solnic]

@pat this should be fixed in 3.3.2 - please upgrade and let me know if this works for you now

[pat]

Yup, all green on my side - thanks for such a quick fix!

[pat]

Meant to report this sooner, but been caught up with other things - this fix seems to have been lost between the 3.3.2 and 3.3.3 releases? It's not present in 3.4.0 either.

[solnic]

@pat oh shit you're right 😭 I must've messed up release branches / master merging/rebasing 😭 I'll cherry-pick the fix commit into release-3.5 so it'll be available in version 3.5.0. Sorry about that...

[pat]

No worries, completely understandable how these things happen :)

[solnic]

@pat 🙂 OK I just pushed 3.5.0 which includes the fix again https://github.com/rom-rb/rom-sql/releases/tag/v3.5.0

[pat]

Many thanks, my build's green again with that upgrade :)