Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with field named timestamp and index with include #51428

Closed
raphaelmatteoni opened this issue Mar 26, 2024 · 2 comments · Fixed by #51429
Closed

Problem with field named timestamp and index with include #51428

raphaelmatteoni opened this issue Mar 26, 2024 · 2 comments · Fixed by #51429

Comments

@raphaelmatteoni
Copy link

raphaelmatteoni commented Mar 26, 2024

Steps to reproduce

When you have a field named "timestamp" and want to add an index including this field, the schema is generated with escape characters, making it impossible to run the load schema

class ChangeIndexStatusToEvent < ActiveRecord::Migration[7.1]
  def change
    add_index :events, [:status, :name, :app_id], name: 'index_events_on_status_name_app_id', include: [:id, :timestamp]
  end
end

This schema is generated when running the migrate above:

t.index ["status", "name", "app_id", "timestamp"], name: "index_events_on_status_name_app_id", include: ["id", "\"timestamp\""]

Test case:

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'rails'

  gem 'pg'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# `CREATE DATABASE rails_51428_test;`
ActiveRecord::Base.establish_connection(adapter: 'postgresql', database: 'rails_51428_test')
ActiveRecord::Base.logger = Logger.new($stdout)

ActiveRecord::Schema.define do
  create_table :events, force: true do |t|
    t.decimal :amount, precision: 10, scale: 0, default: 0, null: false
    t.datetime :timestamp
  end

  add_index :events, [:amount], name: 'index_events_on_timestamp', include: [:timestamp]
end

ActiveRecord::SchemaDumper.dump

class Event < ActiveRecord::Base; end

class EventTest < Minitest::Test
  def test_stuff
    assert_equal 0, Event.count
    Event.create!(amount: 1)
    assert_equal 1, Event.count

    # Note: this fails silently. I later discovered that neither this nor
    # `ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:ruby, ENV['SCHEMA'])`
    # fail. See comments below.
    puts `rails db:schema:load`
  end
end

I am not sure if the word "timestamp" is reserved for the system, but I started having this problem after upgrading the rails version from 7.0.3 to 7.1.0

Expected behavior

Don't escape the characters at include in index or add this explanation to take care to use field named "timestamp" in Rails Guide.

Actual behavior

Using field named "timestamp" and adding index with this field in include, results in the schema with escaped characters, making it impossible to run the load schema.

System configuration

Rails version: 7.1.0

Ruby version: 3.3.0

@danielolivaresd
Copy link

danielolivaresd commented Mar 26, 2024

I tested this locally and would like to share my findings. TLDR; I confirmed that trying to load the schema that was generated as described in this issue, causes an error.

I updated your test case script to use PostgreSQL and actually add the index:

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'rails'

  gem 'pg'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# `CREATE DATABASE rails_51428_test;`
ActiveRecord::Base.establish_connection(adapter: 'postgresql', database: 'rails_51428_test')
ActiveRecord::Base.logger = Logger.new($stdout)

ActiveRecord::Schema.define do
  create_table :events, force: true do |t|
    t.decimal :amount, precision: 10, scale: 0, default: 0, null: false
    t.datetime :timestamp
  end

  add_index :events, [:amount], name: 'index_events_on_timestamp', include: [:timestamp]
end

ActiveRecord::SchemaDumper.dump

class Event < ActiveRecord::Base; end

class EventTest < Minitest::Test
  def test_stuff
    assert_equal 0, Event.count
    Event.create!(amount: 1)
    assert_equal 1, Event.count

    # Note: this fails silently. I later discovered that neither this nor
    # `ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:ruby, ENV['SCHEMA'])`
    # fail. See comments below.
    puts `rails db:schema:load`
  end
end

Here is what I get from running it:

# ruby script.rb
Fetching gem metadata from https://rubygems.org/...........
Resolving dependencies.....
Using rake 13.1.0
Using base64 0.2.0
Using erubi 1.12.0
Using racc 1.7.3
Using crass 1.0.6
Using rack 3.0.10
Using nio4r 2.7.1
Using websocket-extensions 0.1.5
Using bigdecimal 3.1.7
Using connection_pool 2.4.1
Using marcel 1.0.4
Using mini_mime 1.1.5
Using date 3.3.4
Using bundler 2.3.19
Using minitest 5.22.3
Using mutex_m 0.2.0
Using stringio 3.1.0
Using pg 1.5.6
Using webrick 1.8.1
Using thor 1.3.1
Using builder 3.2.4
Using concurrent-ruby 1.2.3
Using zeitwerk 2.6.13
Using timeout 0.4.1
Using tzinfo 2.0.6
Using nokogiri 1.16.3 (x86_64-darwin)
Using rack-session 2.0.0
Using rack-test 2.1.0
Using websocket-driver 0.7.6
Using drb 2.2.1
Using io-console 0.7.2
Using net-protocol 0.2.2
Using psych 5.1.2
Using rackup 2.1.0
Using i18n 1.14.4
Using loofah 2.22.0
Using reline 0.5.0
Using net-imap 0.4.10
Using net-pop 0.1.2
Using net-smtp 0.5.0
Using rdoc 6.6.3.1
Using mail 2.8.1
Using rails-html-sanitizer 1.6.0
Using activesupport 7.1.3.2
Using irb 1.12.0
Using rails-dom-testing 2.2.0
Using globalid 1.2.1
Using actionview 7.1.3.2
Using activejob 7.1.3.2
Using activemodel 7.1.3.2
Using actionpack 7.1.3.2
Using activerecord 7.1.3.2
Using actioncable 7.1.3.2
Using activestorage 7.1.3.2
Using railties 7.1.3.2
Using actionmailer 7.1.3.2
Using actionmailbox 7.1.3.2
Using actiontext 7.1.3.2
Using rails 7.1.3.2


-- create_table(:events, {:force=>true})
D, [2024-03-26T15:07:40.569488 #39539] DEBUG -- :    (2.5ms)  DROP TABLE IF EXISTS "events"
D, [2024-03-26T15:07:40.576022 #39539] DEBUG -- :    (5.4ms)  CREATE TABLE "events" ("id" bigserial primary key, "amount" decimal(10,0) DEFAULT 0.0 NOT NULL, "timestamp" timestamp(6))
   -> 0.0416s
-- add_index(:events, [:amount], {:name=>"index_events_on_timestamp", :include=>[:timestamp]})
D, [2024-03-26T15:07:40.578837 #39539] DEBUG -- :    (2.1ms)  CREATE INDEX "index_events_on_timestamp" ON "events" ("amount") INCLUDE ("timestamp")
   -> 0.0026s
D, [2024-03-26T15:07:40.606898 #39539] DEBUG -- :   ActiveRecord::InternalMetadata Load (0.9ms)  SELECT * FROM "ar_internal_metadata" WHERE "ar_internal_metadata"."key" = $1 ORDER BY "ar_internal_metadata"."key" ASC LIMIT 1  [[nil, "environment"]]
D, [2024-03-26T15:07:40.611838 #39539] DEBUG -- :   ActiveRecord::SchemaMigration Load (0.6ms)  SELECT "schema_migrations"."version" FROM "schema_migrations" ORDER BY "schema_migrations"."version" ASC


# This file is auto-generated from the current state of the database. Instead
# of editing this file, please use the migrations feature of Active Record to
# incrementally modify your database, and then regenerate this schema definition.
#
# This file is the source Rails uses to define your schema when running `bin/rails
# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to
# be faster and is potentially less error prone than running all of your
# migrations from scratch. Old migrations may fail to apply correctly if those
# migrations use external dependencies or application code.
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 0) do
  # These are extensions that must be enabled in order to support this database
  enable_extension "plpgsql"

  create_table "events", force: :cascade do |t|
    t.decimal "amount", precision: 10, default: "0", null: false
    t.datetime "timestamp"
    t.index ["amount", "timestamp"], name: "index_events_on_timestamp", include: ["\"timestamp\""]
  end

end


Run options: --seed 13706

# Running:

D, [2024-03-26T15:07:40.676610 #39539] DEBUG -- :   Event Count (0.6ms)  SELECT COUNT(*) FROM "events"
D, [2024-03-26T15:07:40.684533 #39539] DEBUG -- :   TRANSACTION (0.2ms)  BEGIN
D, [2024-03-26T15:07:40.685258 #39539] DEBUG -- :   Event Create (1.0ms)  INSERT INTO "events" ("amount") VALUES ($1) RETURNING "id"  [["amount", 1]]
D, [2024-03-26T15:07:40.686043 #39539] DEBUG -- :   TRANSACTION (0.5ms)  COMMIT
D, [2024-03-26T15:07:40.686686 #39539] DEBUG -- :   Event Count (0.2ms)  SELECT COUNT(*) FROM "events"

.

Finished in 8.925573s, 0.1120 runs/s, 0.2241 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips

While it is clear that the double quotes are added to the schema as you describe, it seems like the tests and rails db:schema:load run as expected, at least in this inline script. However, I can confirm that this causes an error with an actual app:

  1. Created a Rails app:
rails new rails_51428_test --database postgresql --skip-git --skip-docker \
  --skip-action-mailer --skip-action-text --skip-active-job --skip-active-storage \
  --skip-action-cable --skip-asset-pipeline --skip-js --skip-hotwire --skip-jbuilder \
  --skip-system-test --skip-dev-gems --api --minimal --skip-decrypted-diffs
  1. Configured the database
# config/database.yml
development:
  adapter: postgresql
  encoding: unicode
  pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
  database: rails_51428_test
  1. Created the Event model (rails g model Event amount:decimal timestamp:datetime) and ran the migration (rails db:migrate)

  2. Added the index in a new migration (rails g migration add_index_including_timestamp) and ran it (rails db:migrate)

class AddIndexIncludingTimestamp < ActiveRecord::Migration[7.1]
  def change
    add_index :events, [:amount], name: 'index_events_on_timestamp', include: [:timestamp]
  end
end
  1. Booted the app (rails console). This worked.

  2. Tried to run rails db:schema:load. This failed:

bin/rails aborted!
ActiveRecord::StatementInvalid: PG::UndefinedColumn: ERROR:  column ""timestamp"" does not exist
/redacted/rails_51428_test/db/schema.rb:17:in `block in <top (required)>'
/redacted/rails_51428_test/db/schema.rb:13:in `<top (required)>'

Caused by:
PG::UndefinedColumn: ERROR:  column ""timestamp"" does not exist
/redacted/rails_51428_test/db/schema.rb:17:in `block in <top (required)>'
/redacted/rails_51428_test/db/schema.rb:13:in `<top (required)>'
Tasks: TOP => db:schema:load
(See full trace by running task with --trace)

I also tested the PR by @fatkodima (#51429) and it generates the correct schema and works as expected

# db/schema.rb
ActiveRecord::Schema[7.2].define(version: 2024_03_26_230735) do
  # These are extensions that must be enabled in order to support this database
  enable_extension "plpgsql"

  create_table "events", force: :cascade do |t|
    t.decimal "amount"
    t.datetime "timestamp"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.index ["amount"], name: "index_events_on_timestamp", include: ["timestamp"]
  end
end

@raphaelmatteoni
Copy link
Author

raphaelmatteoni commented Mar 27, 2024

@danielolivaresd I updated my test case script too, thanks for the update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants