Skip to content

Commit f376782

Browse files
authored
Merge pull request rails#50838 from fractaledmind/ar-fix-sqlite-table-structure-parsing
Fix SQLite table definition parsing bug to handle commas in default function definitions
2 parents 3acf87e + 46ad541 commit f376782

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -477,11 +477,7 @@ def bind_params_length
477477
end
478478

479479
def table_structure(table_name)
480-
structure = if supports_virtual_columns?
481-
internal_exec_query("PRAGMA table_xinfo(#{quote_table_name(table_name)})", "SCHEMA")
482-
else
483-
internal_exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA")
484-
end
480+
structure = table_info(table_name)
485481
raise ActiveRecord::StatementInvalid.new("Could not find table '#{table_name}'", connection_pool: @pool) if structure.empty?
486482
table_structure_with_collation(table_name, structure)
487483
end
@@ -684,7 +680,7 @@ def table_structure_with_collation(table_name, basic_structure)
684680
auto_increments = {}
685681
generated_columns = {}
686682

687-
column_strings = table_structure_sql(table_name)
683+
column_strings = table_structure_sql(table_name, basic_structure.map { |column| column["name"] })
688684

689685
if column_strings.any?
690686
column_strings.each do |column_string|
@@ -719,7 +715,15 @@ def table_structure_with_collation(table_name, basic_structure)
719715
end
720716
end
721717

722-
def table_structure_sql(table_name)
718+
UNQUOTED_OPEN_PARENS_REGEX = /\((?![^'"]*['"][^'"]*$)/
719+
FINAL_CLOSE_PARENS_REGEX = /\);*\z/
720+
721+
def table_structure_sql(table_name, column_names = nil)
722+
unless column_names
723+
column_info = table_info(table_name)
724+
column_names = column_info.map { |column| column["name"] }
725+
end
726+
723727
sql = <<~SQL
724728
SELECT sql FROM
725729
(SELECT * FROM sqlite_master UNION ALL
@@ -729,16 +733,30 @@ def table_structure_sql(table_name)
729733

730734
# Result will have following sample string
731735
# CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
732-
# "password_digest" varchar COLLATE "NOCASE");
736+
# "password_digest" varchar COLLATE "NOCASE",
737+
# "o_id" integer,
738+
# CONSTRAINT "fk_rails_78146ddd2e" FOREIGN KEY ("o_id") REFERENCES "os" ("id"));
733739
result = query_value(sql, "SCHEMA")
734740

735741
return [] unless result
736742

737743
# Splitting with left parentheses and discarding the first part will return all
738744
# columns separated with comma(,).
739-
columns_string = result.split("(", 2).last
745+
result.partition(UNQUOTED_OPEN_PARENS_REGEX)
746+
.last
747+
.sub(FINAL_CLOSE_PARENS_REGEX, "")
748+
# column definitions can have a comma in them, so split on commas followed
749+
# by a space and a column name in quotes or followed by the keyword CONSTRAINT
750+
.split(/,(?=\s(?:CONSTRAINT|"(?:#{Regexp.union(column_names).source})"))/i)
751+
.map(&:strip)
752+
end
740753

741-
columns_string.split(",").map(&:strip)
754+
def table_info(table_name)
755+
if supports_virtual_columns?
756+
internal_exec_query("PRAGMA table_xinfo(#{quote_table_name(table_name)})", "SCHEMA")
757+
else
758+
internal_exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA")
759+
end
742760
end
743761

744762
def arel_visitor

activerecord/test/cases/adapters/sqlite3/virtual_column_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ def setup
1717
t.virtual :upper_name, type: :string, as: "UPPER(name)", stored: true
1818
t.virtual :lower_name, type: :string, as: "LOWER(name)", stored: false
1919
t.virtual :octet_name, type: :integer, as: "LENGTH(name)"
20+
t.virtual :mutated_name, type: :string, as: "REPLACE(name, 'l', 'L')"
2021
t.integer :column1
2122
end
2223
VirtualColumn.create(name: "Rails", column1: 10)
@@ -58,6 +59,14 @@ def test_implicit_virtual_column
5859
assert_equal 5, VirtualColumn.take.octet_name
5960
end
6061

62+
def test_virtual_column_with_comma_in_definition
63+
column = VirtualColumn.columns_hash["mutated_name"]
64+
assert_predicate column, :virtual?
65+
assert_not_predicate column, :virtual_stored?
66+
assert_not_nil column.default_function
67+
assert_equal "RaiLs", VirtualColumn.take.mutated_name
68+
end
69+
6170
def test_change_table_with_stored_generated_column
6271
@connection.change_table :virtual_columns do |t|
6372
t.virtual :decr_column1, type: :integer, as: "column1 - 1", stored: true

0 commit comments

Comments
 (0)