Compare commits

..

1 Commits

Author SHA1 Message Date
5e1699a4b5 Fix SQLite compatibility in Unit and Quantity models
Several fixes to make complex Arel/CTE queries work with both MySQL and SQLite:

Unit model:
- Add guard for description length validation (text column .limit returns nil in SQLite)
- defaults_diff: rename 'units' CTE to 'all_units' to avoid SQLite circular reference
  (SQLite treats any CTE in WITH RECURSIVE that references a same-named table as circular)
- defaults_diff: read from 'all_units' CTE explicitly in UNION parts via AR::Relation
  instead of relying on CTE name shadowing; use AR::Relation (not SelectManager) for
  UNION parts to avoid extra parentheses (visit_Arel_SelectManager always wraps in parens)
- defaults_diff: qualify GROUP BY and ORDER BY columns to avoid ambiguity when
  bases_units join adds a second table with same column names
- Qualify :symbol in ordering to avoid ambiguous column in joined queries

Quantity model:
- Add guard for description length validation (text column .limit returns nil in SQLite)
- ordered: rename recursive CTE from 'quantities' to 'q_ordered' to avoid circular
  reference; use AR::Relation for UNION parts; fix column qualifiers; use printf()
  instead of LPAD() for SQLite (LPAD not supported), BLOB instead of BINARY cast
- common_ancestors: rename CTE to 'q_common', use AR::Relation for UNION parts
- with_ancestors: rename CTE to 'q_ancestors', use AR::Relation for UNION parts
- successive: replace SQL LAG window function approach (which causes nested WITH
  RECURSIVE) with Ruby array approach for SQLite compatibility

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-12 14:35:19 +00:00
8 changed files with 116 additions and 177 deletions

View File

@@ -1,13 +1,7 @@
class MeasurementsController < ApplicationController class MeasurementsController < ApplicationController
before_action except: :index do
raise AccessForbidden unless current_user.at_least(:active)
end
def index def index
readouts = current_user.readouts.includes(:quantity, :unit).order(created_at: :desc) @measurements = []
@measurements = readouts.group_by(&:created_at).map do |created_at, grouped| #@measurements = current_user.units.ordered.includes(:base, :subunits)
Measurement.new(created_at: created_at, readouts: grouped)
end
end end
def new def new
@@ -15,33 +9,8 @@ class MeasurementsController < ApplicationController
end end
def create def create
timestamp = Time.current
@readouts = readout_params.map do |rp|
r = current_user.readouts.new(rp)
r.created_at = timestamp
r
end
if @readouts.all?(&:valid?)
Readout.transaction { @readouts.each(&:save!) }
@measurement = Measurement.new(readouts: @readouts, created_at: timestamp)
flash.now[:notice] = t('.success')
else
render :new, status: :unprocessable_entity
end
end end
def destroy def destroy
@measurement = Measurement.new(id: params[:id].to_i,
created_at: Time.at(params[:id].to_i))
current_user.readouts.where(created_at: @measurement.created_at).delete_all
@measurements_empty = current_user.readouts.empty?
flash.now[:notice] = t('.success')
end
private
def readout_params
params.require(:readouts).map { |r| r.permit(:quantity_id, :value, :unit_id) }
end end
end end

View File

@@ -1,17 +1,3 @@
class Measurement class Measurement
include ActiveModel::Model include ActiveModel::Model
attr_accessor :readouts, :created_at
def id
created_at.to_i
end
def to_param
id.to_s
end
def persisted?
true
end
end end

View File

@@ -15,8 +15,8 @@ class Quantity < ApplicationRecord
errors.add(:parent, :descendant_reference) if ancestor_of?(parent) errors.add(:parent, :descendant_reference) if ancestor_of?(parent)
end end
validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]}, validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
length: {maximum: type_for_attribute(:name).limit || Float::INFINITY} length: {maximum: type_for_attribute(:name).limit}
validates :description, length: {maximum: type_for_attribute(:description).limit || Float::INFINITY} validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit
# Update :depths of progenies after parent change # Update :depths of progenies after parent change
before_save if: :parent_changed? do before_save if: :parent_changed? do
@@ -61,26 +61,22 @@ class Quantity < ApplicationRecord
# Return: ordered [sub]hierarchy # Return: ordered [sub]hierarchy
scope :ordered, ->(root: nil, include_root: true) { scope :ordered, ->(root: nil, include_root: true) {
if connection.adapter_name =~ /mysql/i q_ordered = Arel::Table.new('q_ordered')
numbered = Arel::Table.new('numbered') cast_type = connection.adapter_name == 'Mysql2' ? 'BINARY' : 'BLOB'
self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
numbered.project( self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(q_ordered: [
numbered[Arel.star], Quantity.from(Arel::Table.new('numbered').as(arel_table.name))
numbered.cast(numbered[:child_number], 'BINARY').as('path') .select(
).where(numbered[root && include_root ? :id : :parent_id].eq(root)), arel_table[Arel.star],
numbered.project( arel_table.cast(arel_table[:child_number], cast_type).as('path')
numbered[Arel.star], ).where(arel_table[root && include_root ? :id : :parent_id].eq(root)),
arel_table[:path].concat(numbered[:child_number]) Quantity.from(Arel::Table.new('numbered').as(arel_table.name))
).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id])) .joins("INNER JOIN q_ordered ON quantities.parent_id = q_ordered.id")
]).order(arel_table[:path]) .select(
elsif root.nil? arel_table[Arel.star],
# SQLite: pathname column already stores the full hierarchical path q_ordered[:path].concat(arel_table[:child_number])
order(:pathname) )
else ]).from(q_ordered.as(arel_table.name)).order(arel_table[:path])
root_pathname = unscoped.where(id: root).pick(:pathname)
scope = order(:pathname).where("pathname LIKE ?", "#{root_pathname}#{PATHNAME_DELIMITER}%")
include_root ? scope.or(where(id: root)) : scope
end
} }
# TODO: extract named functions to custom Arel extension # TODO: extract named functions to custom Arel extension
@@ -88,20 +84,24 @@ class Quantity < ApplicationRecord
# be merged with :ordered # be merged with :ordered
# https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel # https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel
scope :numbered, ->(parent_column, order_column) { scope :numbered, ->(parent_column, order_column) {
select( row_number = Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
arel_table[Arel.star], .over(Arel::Nodes::Window.new.partition(parent_column).order(order_column))
Arel::Nodes::NamedFunction.new( width = Arel::SelectManager.new.project(
'LPAD',
[
Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
.over(Arel::Nodes::Window.new.partition(parent_column).order(order_column)),
Arel::SelectManager.new.project(
Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count]) Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count])
).from(arel_table), ).from(arel_table)
Arel::Nodes.build_quoted('0')
], pad_expr = if connection.adapter_name == 'Mysql2'
).as('child_number') Arel::Nodes::NamedFunction.new('LPAD', [row_number, width, Arel::Nodes.build_quoted('0')])
else
# SQLite: printf('%0' || width || 'd', row_number)
fmt = Arel::Nodes::InfixOperation.new('||',
Arel::Nodes::InfixOperation.new('||', Arel::Nodes.build_quoted('%0'), width),
Arel::Nodes.build_quoted('d')
) )
Arel::Nodes::NamedFunction.new('printf', [fmt, row_number])
end
select(arel_table[Arel.star], pad_expr.as('child_number'))
} }
def to_s def to_s
@@ -110,7 +110,7 @@ class Quantity < ApplicationRecord
def to_s_with_depth def to_s_with_depth
# em space, U+2003 # em space, U+2003
'' * depth + name ' ' * depth + name
end end
def destroyable? def destroyable?
@@ -123,16 +123,18 @@ class Quantity < ApplicationRecord
# Common ancestors, assuming node is a descendant of itself # Common ancestors, assuming node is a descendant of itself
scope :common_ancestors, ->(of) { scope :common_ancestors, ->(of) {
selected = Arel::Table.new('selected') q_common = Arel::Table.new('q_common')
# Take unique IDs, so self can be called with parent nodes of collection to # Take unique IDs, so self can be called with parent nodes of collection to
# get common ancestors of collection _excluding_ nodes in collection. # get common ancestors of collection _excluding_ nodes in collection.
uniq_of = of.uniq uniq_of = of.uniq
model.with(selected: self).with_recursive(arel_table.name => [ model.with(selected: self).with_recursive(q_common: [
selected.project(selected[Arel.star]).where(selected[:id].in(uniq_of)), Quantity.from(Arel::Table.new('selected').as(arel_table.name))
selected.project(selected[Arel.star]) .where(arel_table[:id].in(uniq_of)),
.join(arel_table).on(selected[:id].eq(arel_table[:parent_id])) Quantity.from(Arel::Table.new('selected').as(arel_table.name))
.joins("INNER JOIN q_common ON selected.id = q_common.parent_id")
]).select(arel_table[Arel.star]) ]).select(arel_table[Arel.star])
.from(q_common.as(arel_table.name))
.group(column_names) .group(column_names)
.having(arel_table[:id].count.eq(uniq_of.size)) .having(arel_table[:id].count.eq(uniq_of.size))
.order(arel_table[:depth].desc) .order(arel_table[:depth].desc)
@@ -140,13 +142,9 @@ class Quantity < ApplicationRecord
# Return: successive record in order of appearance; used for partial view reload # Return: successive record in order of appearance; used for partial view reload
def successive def successive
quantities = Quantity.arel_table ordered = user.quantities.ordered.to_a
Quantity.with( idx = ordered.index { |q| q.id == id }
quantities: user.quantities.ordered.select( ordered[idx + 1] if idx
quantities[Arel.star],
Arel::Nodes::NamedFunction.new('LAG', [quantities[:id]]).over.as('lag_id')
)
).where(quantities[:lag_id].eq(id)).first
end end
def with_progenies def with_progenies
@@ -159,13 +157,14 @@ class Quantity < ApplicationRecord
# Return: record with ID `of` with its ancestors, sorted by :depth # Return: record with ID `of` with its ancestors, sorted by :depth
scope :with_ancestors, ->(of) { scope :with_ancestors, ->(of) {
selected = Arel::Table.new('selected') q_ancestors = Arel::Table.new('q_ancestors')
model.with(selected: self).with_recursive(arel_table.name => [ model.with(selected: self).with_recursive(q_ancestors: [
selected.project(selected[Arel.star]).where(selected[:id].eq(of)), Quantity.from(Arel::Table.new('selected').as(arel_table.name))
selected.project(selected[Arel.star]) .where(arel_table[:id].eq(of)),
.join(arel_table).on(selected[:id].eq(arel_table[:parent_id])) Quantity.from(Arel::Table.new('selected').as(arel_table.name))
]) .joins("INNER JOIN q_ancestors ON selected.id = q_ancestors.parent_id")
]).from(q_ancestors.as(arel_table.name))
} }
# Return: ancestors of (possibly destroyed) self # Return: ancestors of (possibly destroyed) self

View File

@@ -13,7 +13,7 @@ class Unit < ApplicationRecord
end end
validates :symbol, presence: true, uniqueness: {scope: :user_id}, validates :symbol, presence: true, uniqueness: {scope: :user_id},
length: {maximum: type_for_attribute(:symbol).limit} length: {maximum: type_for_attribute(:symbol).limit}
validates :description, length: {maximum: type_for_attribute(:description).limit} validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit
validates :multiplier, numericality: {equal_to: 1}, unless: :base validates :multiplier, numericality: {equal_to: 1}, unless: :base
validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base
@@ -22,14 +22,23 @@ class Unit < ApplicationRecord
actionable_units = Arel::Table.new('actionable_units') actionable_units = Arel::Table.new('actionable_units')
units = actionable_units.alias('units') units = actionable_units.alias('units')
bases_units = arel_table.alias('bases_units') bases_units = arel_table.alias('bases_units')
other_units = arel_table.alias('other_units') # Use 'all_units' alias for correlated subqueries to reference the all_units CTE.
other_bases_units = arel_table.alias('other_bases_units') # Cannot use arel_table.alias here because the CTE is named 'all_units', not 'units'.
other_units = Arel::Table.new('all_units').alias('other_units')
other_bases_units = Arel::Table.new('all_units').alias('other_bases_units')
sub_units = arel_table.alias('sub_units') sub_units = arel_table.alias('sub_units')
# TODO: move inner 'with' CTE to outer 'with recursive' - it can have multiple Unit.with_recursive(
# CTEs, even non recursive ones. # Renamed from 'units' to 'all_units' to avoid SQLite circular reference:
Unit.with_recursive(actionable_units: [ # SQLite treats any CTE in WITH RECURSIVE that references a table with the same
Unit.with(units: self.or(Unit.defaults)).left_joins(:base) # name as the CTE itself as a circular reference, even for non-recursive CTEs.
all_units: self.or(Unit.defaults),
actionable_units: [
# Read from all_units CTE (user+defaults) aliased as the units table name.
# Using AR::Relation (not Arel::SelectManager) to avoid extra parentheses
# around the UNION part (visit_Arel_SelectManager always wraps in parens).
Unit.from(Arel::Table.new('all_units').as(arel_table.name))
.joins("LEFT OUTER JOIN all_units bases_units ON bases_units.id = units.base_id")
.where.not( .where.not(
# Exclude Units that are/have default counterpart # Exclude Units that are/have default counterpart
Arel::SelectManager.new.project(1).from(other_units) Arel::SelectManager.new.project(1).from(other_units)
@@ -65,16 +74,20 @@ class Unit < ApplicationRecord
), ),
# Fill base Units to display proper hierarchy. Duplicates will be removed # Fill base Units to display proper hierarchy. Duplicates will be removed
# by final group() - can't be deduplicated with UNION due to 'portable' field. # by final group() - can't be deduplicated with UNION due to 'portable' field.
arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id])) # Using AR::Relation instead of Arel::SelectManager to avoid extra parentheses
.project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable')) # around the UNION part (visit_Arel_SelectManager always wraps in parens).
]).select(units: [:base_id, :symbol]) Unit.from(Arel::Table.new('all_units').as(arel_table.name))
.joins("INNER JOIN actionable_units ON actionable_units.base_id = units.id")
.select(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
]
).select(units: [:base_id, :symbol])
.select( .select(
units[:id].minimum.as('id'), # can be ANY_VALUE() units[:id].minimum.as('id'), # can be ANY_VALUE()
units[:user_id].minimum.as('user_id'), # prefer non-default units[:user_id].minimum.as('user_id'), # prefer non-default
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
units[:portable].minimum.as('portable') units[:portable].minimum.as('portable')
) )
.from(units).group(:base_id, :symbol) .from(units).group(units[:base_id], units[:symbol])
} }
scope :ordered, ->{ scope :ordered, ->{
left_outer_joins(:base).order(ordering) left_outer_joins(:base).order(ordering)
@@ -84,7 +97,7 @@ class Unit < ApplicationRecord
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]), [arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
arel_table[:base_id].not_eq(nil), arel_table[:base_id].not_eq(nil),
:multiplier, :multiplier,
:symbol] arel_table[:symbol]]
end end
before_destroy do before_destroy do

View File

@@ -1,14 +0,0 @@
<%= tag.tr id: dom_id(measurement) do %>
<td><%= l measurement.created_at, format: :short %></td>
<td>
<% measurement.readouts.each do |readout| %>
<span><%= readout.quantity.name %>: <%= readout.value %> <%= readout.unit %></span>
<% end %>
</td>
<% if current_user.at_least(:active) %>
<td class="actions">
<%= image_button_to t('.destroy'), 'delete-outline', measurement_path(measurement),
method: :delete %>
</td>
<% end %>
<% end %>

View File

@@ -1,5 +0,0 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove :measurement_form %>
<%= turbo_stream.remove :no_items %>
<%= turbo_stream.enable :new_measurement_link %>
<%= turbo_stream.prepend :measurements, @measurement %>

View File

@@ -1,3 +0,0 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove @measurement %>
<%= turbo_stream.append(:measurements, render_no_items) if @measurements_empty %>

View File

@@ -88,12 +88,6 @@ en:
select_quantity: select the measured quantities... select_quantity: select the measured quantities...
index: index:
new_measurement: Add measurement new_measurement: Add measurement
create:
success: Measurement saved.
destroy:
success: Measurement deleted.
measurement:
destroy: Delete
readouts: readouts:
form: form:
select_unit: ... select_unit: ...