Fix usage of psql in db:test:prepare #4834

Merged
merged 1 commit into from Feb 13, 2012

Conversation

Projects
None yet
3 participants
Contributor

sskirby commented Feb 1, 2012

Re-submit of #4810

According to http://www.postgresql.org/docs/9.1/static/app-psql.html (which hasn't changed since at least version 7.0), the psql command line tool optionally takes a username as its last argument. However, the rake db:test:prepare task is mistakenly passing in the template database from database.yml.

This bug probably hasn't been discovered until now because it requires users to be using a sql schema dump (by using in application.rb: config.active_record.schema_format = :sql) AND to have specified a template in their database.yml.

@sskirby sskirby fixes rake db:test:prepare when using postgresql with sql db structur…
…e file and specifying a template in database.yml
745fbd1
Contributor

chris-baynes commented Feb 3, 2012

Also spotted this, but the user argument is still missing.
set_psql_env should be handling this, but the rake tasks fail unless I provide an explicit user argument:

`psql -U #{abcs[env]['username']} -f "#{filename}" #{abcs[env]['database']}`
Contributor

sskirby commented Feb 4, 2012

That's very strange, Chris, because my change only removes passing in the template as the last argument. If what you are saying is correct, db:test:prepare should not be working at all in master. Which means no one using PostgreSQL is able to run tests via the standard rake task.

Now that I think of it, however, I did only test the change as it applies to the 3-2-stable branch and not master. I will re-test against master. If something is wrong, I suspect that it's not because of the changes in this patch.

Contributor

chris-baynes commented Feb 4, 2012

Well, by default psql uses the current OS user name if one isn't provided on the command line. Are you also running the rake tasks as a user that does not have direct access to the db? If so I'll have to check what's wrong with my environment.

Contributor

sskirby commented Feb 6, 2012

Just tested my patch against the latest master branch and (after applying #4910) everything seems to work just fine. I do not have a pg user that matches my username.

Contributor

sskirby commented Feb 7, 2012

Just wondering if the suspicion that a username needs to be explicitly passed in to psql is what's keeping this pull request from being accepted.

If that's the case, the username issue is really a completely different issue and has nothing to do with the changes in this pull request. It should not be blocking this pull request, it should be in it's own ticket.

Contributor

chris-baynes commented Feb 7, 2012

@sskirby agreed. I've tested your patch in another environment and everything's working fine.
+1 for the pull request

@tenderlove tenderlove added a commit that referenced this pull request Feb 13, 2012

@tenderlove tenderlove Merge pull request #4834 from sskirby/fix_usage_of_psql_in_db_test_pr…
…epare

Fix usage of psql in db:test:prepare
957da55

@tenderlove tenderlove merged commit 957da55 into rails:master Feb 13, 2012

@tenderlove tenderlove added a commit that referenced this pull request Feb 23, 2012

@tenderlove tenderlove Merge pull request #4834 from sskirby/fix_usage_of_psql_in_db_test_pr…
…epare

Fix usage of psql in db:test:prepare
ecff25c

@tenderlove tenderlove added a commit that referenced this pull request Feb 23, 2012

@tenderlove tenderlove Merge pull request #4834 from sskirby/fix_usage_of_psql_in_db_test_pr…
…epare

Fix usage of psql in db:test:prepare
6dbf6f3

@tenderlove tenderlove added a commit that referenced this pull request Mar 1, 2012

@tenderlove tenderlove Merge branch '3-2-2' into 3-2-stable
* 3-2-2:
  bumping to 3.2.2
  Ensure [] respects the status of the buffer.
  Merge pull request #4834 from sskirby/fix_usage_of_psql_in_db_test_prepare
  Merge pull request #5084 from johndouthat/patch-1
  updating RAILS_VERSION
  delete vulnerable AS::SafeBuffer#[]
  use AS::SafeBuffer#clone_empty for flushing the output_buffer
  add AS::SafeBuffer#clone_empty
  fix output safety issue with select options
c289790
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment