From 0e0cfeff10af5e6593b1586d061282fcc6cb9f7f Mon Sep 17 00:00:00 2001 From: cryptogopher Date: Sat, 23 May 2026 17:02:01 +0200 Subject: [PATCH] Change Unit.multiplier type from decimal to float Remove remnants related to BigDecimal. --- DESIGN.md | 20 +++++-- README.md | 6 +-- app/controllers/units_controller.rb | 14 ++--- app/helpers/application_helper.rb | 20 ++++--- app/models/unit.rb | 10 ++-- app/views/units/_form.html.erb | 2 +- app/views/units/_unit.html.erb | 2 +- app/views/units/rebase.turbo_stream.erb | 2 +- config/initializers/core_ext.rb | 6 +-- config/locales/en.yml | 4 -- db/migrate/20230602185352_create_units.rb | 2 +- db/schema.rb | 2 +- db/seeds/templates/units.erb | 2 +- db/sqlite3_schema.rb | 2 +- ...ericality_validates_precision_and_scale.rb | 19 ------- .../big_decimal_scientific_notation.rb | 36 ------------- lib/core_ext/float.rb | 54 +++++++++++++++++++ lib/core_ext/range.rb | 31 +++++++++++ 18 files changed, 134 insertions(+), 100 deletions(-) delete mode 100644 lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb delete mode 100644 lib/core_ext/big_decimal_scientific_notation.rb create mode 100644 lib/core_ext/float.rb create mode 100644 lib/core_ext/range.rb diff --git a/DESIGN.md b/DESIGN.md index d8abb33..f652328 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -22,18 +22,32 @@ whenever a change is considered, to avoid regressions. and scale approximately half of that - 9, * double precision floating point guarantees 15 digits of precision, which is more than enough for all expected use cases, + * if a decimal string with at most 15 significant digits is converted to + the IEEE 754 double-precision format, giving a normal number, and then + converted back to a decimal string with the same number of digits, the + final result should match the original string, + * if an IEEE 754 double-precision number is converted to a decimal + string with at least 17 significant digits, and then converted back to + double-precision representation, the final result must match the + original number, * single precision floating point only guarntees 6 digits of precision, which is estimated to be too low for some use cases (e.g. storing - latitude/longitude with a resolution grater than 100m) + latitude/longitude with a resolution grater than 100m), * double precision floating point (IEEE 754) is a standard that ensures - compatibility with all database engines, + compatibility with majority of database engines, * the same data format is used internally by Ruby as a `Float`; it guarantees no conversions between storage and computation, * as a standard with hardware implementations ensures both: computing efficiency and hardware/3rd party library compatibility as opposed to Ruby - custom `BigDecimal` type + custom `BigDecimal` type, + * at present, only normalized numbers are used and considered sufficient, in + order to avoid potential issues with the cross-platform compatibility of + subnormal (denormal) numbers, +* in the future, the IEEE 754 decimal64 data type may be considered once it is + supported by database engines. ### Database layer vs application layer data model constraints + * database constraints are the final guard against data integrity corruption, * they should safeguard against data referential integrity loss under _all_ data (not schema) manipulation scenarios, including application level diff --git a/README.md b/README.md index 8df5a38..b97bbea 100644 --- a/README.md +++ b/README.md @@ -16,14 +16,10 @@ environment, see the _Contributing_ section below. * Server side: * Ruby interpreter, depending on the version of Rails used (see _Gemfile_), * https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#ruby-versions - * database (e.g. MySQL >= 8.0) supporting: + * database (e.g. SQLite, MySQL >= 8.0) supporting: * recursive Common Table Expressions (CTE) for SELECT/UPDATE/DELETE, * MariaDB does not support CTE for UPDATE/DELETE (https://jira.mariadb.org/browse/MDEV-18511) - * decimal datatype with precision of at least 30, - * SQLite3 _flexible typing_ decimal will work, but precision - will be limited to 16, making it practical mostly for testing - purposes * for testing: browser as specified in _Client side_ requirements * Client side: * browser (e.g. Firefox >= 121) supporting: diff --git a/app/controllers/units_controller.rb b/app/controllers/units_controller.rb index d3c6b30..c876cbc 100644 --- a/app/controllers/units_controller.rb +++ b/app/controllers/units_controller.rb @@ -17,7 +17,7 @@ class UnitsController < ApplicationController end def create - @unit = current_user.units.new(unit_params) + @unit = current_user.units.new(params.expect(Unit::ATTRIBUTES)) if @unit.save @before = @unit.successive flash.now[:notice] = t('.success', unit: @unit) @@ -30,7 +30,7 @@ class UnitsController < ApplicationController end def update - if @unit.update(unit_params.except(:base_id)) + if @unit.update(params.except(:base_id).expect(Unit::ATTRIBUTES)) flash.now[:notice] = t('.success', unit: @unit) else render :edit @@ -40,11 +40,11 @@ class UnitsController < ApplicationController # TODO: Avoid double table width change by first un-hiding table header, # then displaying index, e.g. by re-displaying header in index def rebase - permitted = params.require(:unit).permit(:base_id) - permitted.merge!(multiplier: 1) if permitted[:base_id].blank? && @unit.multiplier != 1 + unit_params = params.expect(unit: :base_id) + unit_params.merge!(multiplier: 1.0) if unit_params[:base_id].blank? @previous_base = @unit.base - @unit.update!(permitted) + @unit.update!(unit_params) @before = @unit.successive if @unit.multiplier_previously_changed? @@ -59,10 +59,6 @@ class UnitsController < ApplicationController private - def unit_params - params.require(:unit).permit(Unit::ATTRIBUTES) - end - def find_unit @unit = current_user.units.find_by!(id: params[:id]) end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 57b58e9..ea3c348 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -103,15 +103,19 @@ module ApplicationHelper def number_field(method, options = {}) attr_type = object.type_for_attribute(method) case attr_type.type - when :decimal + when :float, :double options[:value] = object.public_send(method)&.to_scientific - options[:step] ||= BigDecimal(10).power(-attr_type.scale) - options[:max] ||= BigDecimal(10).power(attr_type.precision - attr_type.scale) - - options[:step] - options[:min] = options[:min] == :step ? options[:step] : options[:min] - options[:min] ||= -options[:max] - options[:size] ||= attr_type.precision / 2 - when :float + options[:step] ||= :any + options[:min] ||= Float::MIN_15 + options[:max] ||= Float::MAX_15 + # Longest possible number (written not using exponent): + # sign (1), leading 0 (1), dot (1), exponent 0s (307), digits (15). + # This is only upper bound, which cannot guarantee the number won't fall + # out of range. + # TODO: add `[pattern]` to limit precision and (possibly) replace `[maxlength]`? + # NOTE: `[pattern]` is unavailable on `input[type=number]` and `[min]/[max]` is + # unavailable on `input[type=text]`. + options[:maxlength] ||= 3 + Float::MIN_10_EXP + Float::DIG options[:size] ||= 6 end super diff --git a/app/models/unit.rb b/app/models/unit.rb index 5f5865c..e53f311 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -1,5 +1,5 @@ class Unit < ApplicationRecord - ATTRIBUTES = [:symbol, :description, :multiplier, :base_id] + ATTRIBUTES = {unit: [:symbol, :description, :multiplier, :base_id]} belongs_to :user, optional: true belongs_to :base, optional: true, class_name: "Unit" @@ -14,8 +14,8 @@ class Unit < ApplicationRecord validates :symbol, presence: true, uniqueness: {scope: :user_id}, length: {maximum: type_for_attribute(:symbol).limit} validates :description, length: {maximum: type_for_attribute(:description).limit} - validates :multiplier, numericality: {equal_to: 1}, unless: :base - validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base + validates :multiplier, numericality: {equal_to: 1.0}, unless: :base + validates :multiplier, numericality: {greater_than: 0.0}, if: :base scope :defaults, ->{ where(user: nil) } scope :defaults_diff, ->{ @@ -102,10 +102,10 @@ class Unit < ApplicationRecord user_id.nil? end - # Should only by invoked on Units returned from #defaults_diff which are #portable + # Should only by invoked on Units returned from #defaults_diff which are #portable. def port!(recipient) recipient_base = base && Unit.find_by!(symbol: base.symbol, user: recipient) - params = slice(ATTRIBUTES - [:symbol, :base_id]) + params = slice(ATTRIBUTES[:unit] - [:symbol, :base_id]) Unit.find_or_initialize_by(user: recipient, symbol: symbol) .update!(base: recipient_base, **params) end diff --git a/app/views/units/_form.html.erb b/app/views/units/_form.html.erb index a5b1a3e..ce219ef 100644 --- a/app/views/units/_form.html.erb +++ b/app/views/units/_form.html.erb @@ -9,7 +9,7 @@ <%= form.text_area :description, cols: 30, rows: 1, escape: false %> - <%= form.number_field :multiplier, required: true, size: 10, min: :step if @unit.base_id? %> + <%= form.number_field :multiplier, required: true if @unit.base_id? %> diff --git a/app/views/units/_unit.html.erb b/app/views/units/_unit.html.erb index d5e726e..db656d9 100644 --- a/app/views/units/_unit.html.erb +++ b/app/views/units/_unit.html.erb @@ -11,7 +11,7 @@ data: {turbo_stream: true} %> <%= unit.description %> - <%= unit.multiplier.to_html %> + <%= unit.multiplier.to_html if unit.base_id? %> <% if current_user.at_least(:active) %> diff --git a/app/views/units/rebase.turbo_stream.erb b/app/views/units/rebase.turbo_stream.erb index 36c369d..91ab985 100644 --- a/app/views/units/rebase.turbo_stream.erb +++ b/app/views/units/rebase.turbo_stream.erb @@ -1,4 +1,4 @@ <%= turbo_stream.remove @unit %> <%= turbo_stream.replace @previous_base if @previous_base %> -<%= turbo_stream.replace @unit.base if @unit.base_id? && (@previous_base.id != @unit.base_id) %> +<%= turbo_stream.replace @unit.base if @unit.base_id? && (@previous_base&.id != @unit.base_id) %> <%= @before ? turbo_stream.before(@before, @unit) : turbo_stream.append(:units, @unit) %> diff --git a/config/initializers/core_ext.rb b/config/initializers/core_ext.rb index 1233f1b..288fc16 100644 --- a/config/initializers/core_ext.rb +++ b/config/initializers/core_ext.rb @@ -1,5 +1,6 @@ require 'core_ext/array_delete_bang' -require 'core_ext/big_decimal_scientific_notation' +require 'core_ext/float' +require 'core_ext/range' ActiveSupport.on_load :action_dispatch_system_test_case do prepend CoreExt::ActionDispatch::SystemTesting::TestHelpers::ScreenshotHelperUniqueId @@ -10,9 +11,6 @@ ActiveSupport.on_load :action_view do end ActiveSupport.on_load :active_record do - ActiveModel::Validations::NumericalityValidator - .prepend CoreExt::ActiveModel::Validations::NumericalityValidatesPrecisionAndScale - # Temporary patch for https://github.com/rails/rails/pull/54658 Arel::TreeManager::StatementMethods .prepend CoreExt::Arel::TreeManager::StatementMethodsCteUpdateAndDelete diff --git a/config/locales/en.yml b/config/locales/en.yml index 3382167..53cbd41 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4,10 +4,6 @@ en: # Format contains non-breaking space: 160.chr(Encoding::UTF_8) default: "%Y-%m-%d %H:%M %Z" without_tz: "%Y-%m-%d %H:%M" - errors: - messages: - precision_exceeded: must not exceed %{value} significant digits - scale_exceeded: must not exceed %{value} decimal digits activerecord: attributes: quantity: diff --git a/db/migrate/20230602185352_create_units.rb b/db/migrate/20230602185352_create_units.rb index 5dcea1a..75684fd 100644 --- a/db/migrate/20230602185352_create_units.rb +++ b/db/migrate/20230602185352_create_units.rb @@ -4,7 +4,7 @@ class CreateUnits < ActiveRecord::Migration[8.1] t.references :user, foreign_key: {on_delete: :cascade} t.string :symbol, null: false, limit: 15 t.text :description - t.decimal :multiplier, null: false, precision: 30, scale: 15, default: 1.0 + t.float :multiplier, null: false, limit: Float::MANT_DIG, default: 1.0 t.references :base, foreign_key: {to_table: :units, on_delete: :cascade} t.timestamps diff --git a/db/schema.rb b/db/schema.rb index a87e257..84650cc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -63,7 +63,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_21_230456) do t.bigint "user_id" t.string "symbol", limit: 15, null: false t.text "description" - t.decimal "multiplier", precision: 30, scale: 15, default: "1.0", null: false + t.float "multiplier", limit: 53, default: 1.0, null: false t.bigint "base_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false diff --git a/db/seeds/templates/units.erb b/db/seeds/templates/units.erb index cfec373..c875a60 100644 --- a/db/seeds/templates/units.erb +++ b/db/seeds/templates/units.erb @@ -4,7 +4,7 @@ Unit.transaction do <% Unit.defaults.ordered.each do |unit| %> <%= "\n" if unit.base.nil? %> units['<%= unit.symbol %>'] = - Unit.create symbol: '<%= unit.symbol %>',<% unless unit.base.nil? %> base: units['<%= unit.base.symbol %>'], multiplier: '<%= unit.multiplier.to_scientific %>',<% end %> + Unit.create symbol: '<%= unit.symbol %>',<% if unit.base_id? %> base: units['<%= unit.base.symbol %>'], multiplier: '<%= unit.multiplier.to_scientific %>',<% end %> description: '<%= unit.description %>' <% end %> end diff --git a/db/sqlite3_schema.rb b/db/sqlite3_schema.rb index fe10f75..ea32ab5 100644 --- a/db/sqlite3_schema.rb +++ b/db/sqlite3_schema.rb @@ -61,7 +61,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_21_230456) do t.integer "user_id" t.string "symbol", limit: 15, null: false t.text "description", limit: 65535 - t.decimal "multiplier", precision: 30, scale: 15, default: "1.0", null: false + t.float "multiplier", limit: 53, default: 1.0, null: false t.integer "base_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false diff --git a/lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb b/lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb deleted file mode 100644 index 42be128..0000000 --- a/lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb +++ /dev/null @@ -1,19 +0,0 @@ -module CoreExt::ActiveModel::Validations::NumericalityValidatesPrecisionAndScale - def validate_each(record, attr_name, value, ...) - super(record, attr_name, value, ...) - - if options[:precision] || options[:scale] - attr_type = record.class.type_for_attribute(attr_name) - # For conversion of 'value' to BigDecimal 'ndigits' is not supplied intentionally, - # to avoid silent rounding. It is only required for conversion from Float and - # Rational, which should not happen. - value = BigDecimal(value) unless value.is_a? BigDecimal - if options[:precision] && (value.precision > attr_type.precision) - record.errors.add(attr_name, :precision_exceeded, **filtered_options(attr_type.precision)) - end - if options[:scale] && (value.scale > attr_type.scale) - record.errors.add(attr_name, :scale_exceeded, **filtered_options(attr_type.scale)) - end - end - end -end diff --git a/lib/core_ext/big_decimal_scientific_notation.rb b/lib/core_ext/big_decimal_scientific_notation.rb deleted file mode 100644 index 0a45c90..0000000 --- a/lib/core_ext/big_decimal_scientific_notation.rb +++ /dev/null @@ -1,36 +0,0 @@ -module CoreExt - module BigDecimalScientificNotation - def to_scientific - return 'NaN' unless finite? - - sign, coefficient, base, exponent = split - (sign == -1 ? '-' : '') + - (coefficient.length > 1 ? coefficient.insert(1, '.') : coefficient) + - (exponent != 1 ? "e#{exponent-1}" : '') - end - - # Converts value to HTML formatted scientific notation - def to_html - sign, coefficient, base, exponent = split - return 'NaN' unless sign - - result = (sign == -1 ? '-' : '') - unless coefficient == '1' && sign == 1 - if coefficient.length > 1 - result += coefficient.insert(1, '.') - elsif - result += coefficient - end - if exponent != 1 - result += "×" - end - end - if exponent != 1 - result += "10% d" % [exponent-1] - end - result.html_safe - end - end -end - -BigDecimal.prepend CoreExt::BigDecimalScientificNotation diff --git a/lib/core_ext/float.rb b/lib/core_ext/float.rb new file mode 100644 index 0000000..e42189a --- /dev/null +++ b/lib/core_ext/float.rb @@ -0,0 +1,54 @@ +# If a decimal string with at most 15 significant digits is converted to the +# IEEE 754 double-precision format, giving a normal number, and then converted +# back to a decimal string with the same number of digits, the final result +# should match the original string. +# If an IEEE 754 double-precision number is converted to a decimal string with +# at least 17 significant digits, and then converted back to double-precision +# representation, the final result must match the original number: +# ("%.16e" % self).to_f == self +class Float + def to_scientific + sign, significand, exponent = split + sign + significand + (exponent != 0 ? "e#{exponent}" : '') + end + + # Converts value to HTML formatted scientific notation. + def to_html(show_unity: true) + result, significand, exponent = split + result += significand if significand != '1' || (exponent == 0 && show_unity) + if exponent != 0 + result += "×" if significand != '1' + result += "10% d" % exponent + end + result.html_safe + end + + # Assume #finite? is true. + def limit(precision = DIG) + return 0.0 if precision.zero? + sign, significand, exponent = split(DIG_MAX) + "#{sign}#{significand[..precision]}e#{exponent}".to_f + end + + private + + SPLIT_FLOAT = /(-?)(.*?)\.?0*e(.*)/ + + # Format `%e` displays starting from significant digit (not 0). + def split(digits = DIG) + return ['', to_s[..2], 0] unless finite? + return ['', '0', 0] if digits.zero? + ("%.#{digits - 1}e" % self).match(SPLIT_FLOAT).captures + .then { |sign, significand, exponent| [sign, significand, exponent.to_i] } + end + + # The maximum number of significant decimal digits in a double-precision + # floating point number. + DIG_MAX = 17 + # Smallest and largest double-precision floating point numbers with DIG + # precision. + # TODO: change MIN_15 to MIN.ceil(MIN_10_EXP - DIG) after #ceil fix in Ruby + # v4.0.5: https://bugs.ruby-lang.org/issues/22079 + MIN_15 = MIN.ceil(-(MIN_10_EXP - 1)) + MAX_15 = MAX.floor(-(MAX_10_EXP - DIG + 1)) +end diff --git a/lib/core_ext/range.rb b/lib/core_ext/range.rb new file mode 100644 index 0000000..58657e6 --- /dev/null +++ b/lib/core_ext/range.rb @@ -0,0 +1,31 @@ +class Range + # TODO: cleanup comments after commit + # * < nil, true < false + #if a.end == b.end + # a.exclude_end? ^ b.exclude_end? ? (a.exclude_end? ? -1 : 1) : 0 + #else + # a.end <=> b.end || (a.end.nil? ? 1 : -1) + #end + #a.end == b.end ? (b.exclude_end? ? b : a) : [a, b].to_h.except(nil).min + #*(l[0] == r[0] ? (r[1] ? r : l) : [l, r].reject{ |e| e[0].nil? }.min) + def &(other) + case other + when Range + return nil unless self.overlap?(other) + + both = [self, other] + return Range.new( + both.map(&:begin).compact.max, + *if self.end == other.end + other.exclude_end? ? [other] : [self] + else + both.select(&:end) + end.map { |r| [r.end, r.exclude_end?] }.min + ) + when Array + return other.map { |o| self & o }.compact + else + return self.member?(other) ? other : nil + end + end +end