Respect 'SchemaDumper.ignore_tables' in databases structure dump #29077
Conversation
|
Will squash some commits and add CHANGELOG to this after of getting review 🙌 |
| ignore_tables = ActiveRecord::SchemaDumper.ignore_tables | ||
| if ignore_tables.any? | ||
| tables = `sqlite3 #{dbfile} .tables`.split - ignore_tables | ||
| condition = tables.map { |table| "tbl_name = '#{table}'" }.join(" OR ") |
There was a problem hiding this comment.
It feels like it might be more consistent, if nothing else, to use a NOT IN (..) here, instead of doing the subtraction ourselves.
There was a problem hiding this comment.
We should also use the proper string-quoting method.
| tables = `sqlite3 #{dbfile} .tables`.split - ignore_tables | ||
| condition = tables.map { |table| "tbl_name = '#{table}'" }.join(" OR ") | ||
| statement = "SELECT sql FROM sqlite_master WHERE #{condition} ORDER BY tbl_name, type DESC, name" | ||
| `sqlite3 #{flags} #{dbfile} "#{statement}" > #{filename}` |
There was a problem hiding this comment.
We should be anyway, but this level of control over the contents of the string makes it even more important for this to be using the array form, instead of the string-construction form, of command invocation.
guilleiguaran
force-pushed
the
ignore-tables-in-sql-dump
branch
from
e845ed8
to
1f2cfa7
Compare
| @@ -64,6 +66,18 @@ def configuration | |||
| def root | |||
| @root | |||
| end | |||
|
|
|||
| def run_cmd(cmd, args, out) | |||
| #fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, out: out) | |||
There was a problem hiding this comment.
Not sure if I'm doing something wrong but this fails:
Kernel.system(cmd, *args, out: out)
while this works correctly:
Kernel.system("#{cmd} #{args.join(' ')}", out: out)
There was a problem hiding this comment.
My guess would be the quoting on statement, which isn't required for the splat form.
There was a problem hiding this comment.
yup, that worked 👍
guilleiguaran
force-pushed
the
ignore-tables-in-sql-dump
branch
from
eed369a
to
b50f32c
Compare
|
@matthewd I've done all the suggested changes in SQLite rake task 🙌 |
guilleiguaran
force-pushed
the
ignore-tables-in-sql-dump
branch
from
b50f32c
to
271a695
Compare
When using `sql` as the schema format, or even just doing `rake db:structure:dump`, it would be good to respect the list of ignored tables that has been configured.
* Use NOT IN in SQL query * Quote table names propertly * Use array form of command invocation
guilleiguaran
force-pushed
the
ignore-tables-in-sql-dump
branch
from
271a695
to
f98c97d
Compare
Follow up of rails#29077. Before: ```sql SELECT sql FROM sqlite_master WHERE tbl_name NOT IN ("foo") ORDER BY tbl_name, type DESC, name ``` After: ```sql SELECT sql FROM sqlite_master WHERE tbl_name NOT IN ('foo') ORDER BY tbl_name, type DESC, name ``` > If a keyword in double quotes (ex: "key" or "glob") is used in a context where it cannot be resolved to an identifier but where a string literal is allowed, then the token is understood to be a string literal instead of an identifier. http://www.sqlite.org/lang_keywords.html
This includes changes from #29060, fix a broken test and adds support for MySQL and SQLite3 as well.