Fix SQLite query compatibility inside WITH clause

Rename model methods to #succeeding.

app/controllers/quantities_controller.rb

@@ -19,7 +19,7 @@ class QuantitiesController < ApplicationController
   def create
     @quantity = current_user.quantities.new(quantity_params)
     if @quantity.save
-      @before = @quantity.successive
+      @before = @quantity.succeeding
       @ancestors = @quantity.ancestors
       flash.now.notice = t('.success', quantity: @quantity)
     else
@@ -45,8 +45,9 @@ class QuantitiesController < ApplicationController
 
     if @quantity.update(permitted)
       @ancestors = @quantity.ancestors
-      @self_and_progenies = @quantity.with_progenies
-      @before = @self_and_progenies.last.successive
+      # TODO: simplify into 1 query (see #succeeding comment)
+      @quantities = @quantity.with_progenies
+      @before = @quantities.last.succeeding
     else
       # Until UI blocks all disallowed reparents, render error messages if present.
       flash.now.alert = @quantity.errors.full_messages

app/controllers/units_controller.rb

@@ -18,7 +18,7 @@ class UnitsController < ApplicationController
   def create
     @unit = current_user.units.new(params.expect(Unit::ATTRIBUTES))
     if @unit.save
-      @before = @unit.successive
+      @before = @unit.succeeding
       flash.now.notice = t('.success', unit: @unit)
     else
       render_errors @unit
@@ -45,7 +45,7 @@ class UnitsController < ApplicationController
     @previous_base = @unit.base
     @unit.update!(unit_params)
 
-    @before = @unit.successive
+    @before = @unit.succeeding
     if @unit.multiplier_previously_changed?
       flash.now.notice = t(".multiplier_reset", unit: @unit)
     end

app/models/quantity.rb

@@ -18,6 +18,9 @@ class Quantity < ApplicationRecord
   end
   validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
     length: {maximum: type_for_attribute(:name).limit}
+  # FIXME: length validation for :text type column is probably wrong; reported
+  # limit is in bytes (at least for MySQL) and check is for #length (i.e.
+  # characters); this doesn't seem to apply to :string (implemented as varchar)
   validates :description, length: {maximum: type_for_attribute(:description).limit}
 
   # Update :depths of progenies after parent change
@@ -62,26 +65,42 @@ class Quantity < ApplicationRecord
   scope :defaults, ->{ where(user: nil) }
 
   # Return: ordered [sub]hierarchy
-  scope :ordered, ->(root: nil, include_root: true) {
-    numbered = Arel::Table.new('numbered')
+  scope :ordered, ->(**kwargs) {
+    with_path(**kwargs).order(arel_table[:path])
+  }
 
-    self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
-      numbered.project(
-        numbered[Arel.star],
-        numbered.cast(numbered[:child_number], 'BINARY').as('path')
-      ).where(numbered[root && include_root ? :id : :parent_id].eq(root)),
-      numbered.project(
-        numbered[Arel.star],
-        arel_table[:path].concat(numbered[:child_number])
-      ).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id]))
-    ]).order(arel_table[:path])
+  # TODO: sort recursive CTE using ORDER BY; it's supported by SQLite, so
+  # drop MySQL until it is available (?)
+  # Only ORDER BY allows proper i.e. not placing limits on user data and
+  # efficient with regard to paging (LIMIT/OFFSET) depth-first search.
+  # Sorting based on numbering is less elegant/efficient (MySQL: ROW_NUMBER cannot
+  # be used inside recursive CTE and needs separate SELECT,
+  # https://bugs.mysql.com/bug.php?id=96538).
+  # Sorting based on pathname requires I) padding to maximum column length or
+  # II) delimiter which a) cannot be used in sorted column and b) sorts before
+  # anything else (this adds complexity; NULL cannot be used when sorting).
+  # Sorting based on ARRAY is only available on PostgreSQL.
+  scope :with_path, ->(root: nil, include_root: true) {
+    with_number = Arel::Table.new('with_number')
+
+    self.model.with(with_number: with_number(:parent_id, :name))
+      .with_recursive(arel_table.name => [
+        with_number.project(
+          with_number[Arel.star],
+          with_number.cast(with_number[:number], 'BINARY').as('path')
+        ).where(with_number[root && include_root ? :id : :parent_id].eq(root)),
+        with_number.project(
+          with_number[Arel.star],
+          arel_table[:path].concat(with_number[:number])
+        ).join(arel_table).on(with_number[:parent_id].eq(arel_table[:id]))
+      ])
   }
 
   # TODO: extract named functions to custom Arel extension
   # NOTE: once recursive queries allow use of window functions, this scope can
   # be merged with :ordered
   # https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel
-  scope :numbered, ->(parent_column, order_column) {
+  scope :with_number, ->(parent_column, order_column) {
     select(
       arel_table[Arel.star],
       Arel::Nodes::NamedFunction.new(
@@ -94,7 +113,7 @@ class Quantity < ApplicationRecord
           ).from(arel_table),
           Arel::Nodes.build_quoted('0')
         ],
-      ).as('child_number')
+      ).as('number')
     )
   }
 
@@ -132,15 +151,17 @@ class Quantity < ApplicationRecord
       .order(arel_table[:depth].desc)
   }
 
-  # Return: successive record in order of appearance; used for partial view reload
-  def successive
+  # Return: succeeding record in order of appearance, used for partial view reload.
+  # TODO: add scope returning succeeding element for multiple records
+  def succeeding
     quantities = Quantity.arel_table
-    Quantity.with(
-      quantities: user.quantities.ordered.select(
+    Quantity.with(lagging_quantities: user.quantities.with_path.select(
       quantities[Arel.star],
-        Arel::Nodes::NamedFunction.new('LAG', [quantities[:id]]).over.as('lag_id')
-      )
-    ).where(quantities[:lag_id].eq(id)).first
+      # For SQLite 'OVER () ORDER BY x' is not equivalent to 'OVER (ORDER BY x)'.
+      Arel::Nodes::NamedFunction.new('LAG', [quantities[:id]])
+        .over(Arel::Nodes::Window.new.order(Quantity.ordered.order_values)).as('lag_id')
+    )).from(Arel::Table.new(:lagging_quantities).as(:quantities))
+      .where(quantities[:lag_id].eq(id)).take
   end
 
   def with_progenies

app/models/unit.rb

@@ -3,6 +3,7 @@ class Unit < ApplicationRecord
 
   belongs_to :user, optional: true
   belongs_to :base, optional: true, class_name: "Unit"
+  # TODO: use #alias_attribute after Rails upgrade?
   alias_method :parent, :base
   has_many :subunits, class_name: "Unit", inverse_of: :base,
     dependent: :restrict_with_error
@@ -111,13 +112,13 @@ class Unit < ApplicationRecord
       .update!(base: recipient_base, **params)
   end
 
-  def successive
+  def succeeding
     units = Unit.arel_table
-    Unit.with(units_with_lag: user.units.left_outer_joins(:base).select(
+    Unit.with(lagging_units: user.units.left_outer_joins(:base).select(
       units[Arel.star],
       Arel::Nodes::NamedFunction.new('LAG', [units[:id]])
         .over(Arel::Nodes::Window.new.order(Unit.ordered.order_values)).as('lag_id')
-    )).from(Arel::Table.new(:units_with_lag).as(:units))
+    )).from(Arel::Table.new(:lagging_units).as(:units))
       .where(units[:lag_id].eq(id)).take
   end
 end

app/views/quantities/reparent.turbo_stream.erb

@@ -1,7 +1,7 @@
 <% @previous_ancestors.union(@ancestors).each do |ancestor| %>
   <%= turbo_stream.replace ancestor %>
 <% end %>
-<% @self_and_progenies.each do |quantity| %>
+<% @quantities.each do |quantity| %>
   <%= turbo_stream.remove quantity %>
   <%= turbo_stream.before @before || :endmark, quantity %>
 <% end %>