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

drush_sql_register_post_sync_op() query with braces enclosing table does not work #1582

Closed
txemapamundi9l opened this issue Aug 29, 2015 · 5 comments

Comments

@txemapamundi9l
Copy link

Works when using drush sql-sanitize:
drush_sql_register_post_sync_op('password_policy_history', dt('Delete all Password Policy history table entries'), "DELETE FROM password_policy_history;");

Does not work:
drush_sql_register_post_sync_op('password_policy_history', dt('Delete all Password Policy history table entries'), "DELETE FROM {password_policy_history};");

The former, though, presumably does not work for sites using table prefixes. So there seems to be no single way to write the query that works for all configurations.

Using Drush Git master latest.

This issue arose from this issue:
https://www.drupal.org/node/2555053

@mstrelan
Copy link

sql_drush_sql_sync_sanitize() includes checks to determine if the table name should be wrapped in braces. For me this seems to wrap the table names regardless because my db prefix is apparently an array and therefore not empty. Regardless of whether or not it should be wrapped, ultimately the query is written to a temporary file and read directly by mysql, so if there are braces they will throw an error. Braces should be replaced by the actual prefix.

@weitzman
Copy link
Member

Drush sql-sanitize replaces braces when the --db-prefix option is provided.

@txemapamundi9l
Copy link
Author

Drush sql-sanitize replaces braces when the --db-prefix option is provided.

But it errors when --db-prefix is not provided and braces are used in the query. So it seems to be impossible to write a query for sql_drush_sql_sync_sanitize() that works for both sites using or not using database prefixes.

@weitzman
Copy link
Member

Oh crap. That does look like awkward. Today, you have to write your sanitize hook like Drush core does it in sql_drush_sql_sync_sanitize():

if (drush_get_option('db-prefix') || !empty($databases['default']['default']['prefix'])) {
    $wrap_table_name = TRUE;
  }
  else {
    $wrap_table_name = FALSE;
  }

Or more tersely:

$wrap_table_name = drush_get_option('db-prefix') || !empty($databases['default']['default']['prefix'])`. 

Then you add braces based on $wrap_table_name.

Ping @bjeavons since this went in with fdffc65.

I'm not sure why we don't unconditionally do the prefix replacement. This is such a useless part of Drupal that I hesitate to spend much time making it better. It could easily cause more harm than help.

@weitzman weitzman reopened this Feb 29, 2016
weitzman added a commit that referenced this issue Mar 28, 2016
…sanitize ... Database prefixes are still discouraged by Drush maintainers.
weitzman added a commit that referenced this issue Mar 29, 2016
commit 0438baf
Author: Moshe Weitzman <[email protected]>
Date:   Tue Mar 29 10:11:33 2016 -0400

    A fix to the implode() of sanitize queries.

commit 8ed8452
Author: Moshe Weitzman <[email protected]>
Date:   Mon Mar 28 08:59:03 2016 -0400

    Fix #1869 and #1582. Better support for database prefixes during sql-sanitize ... Database prefixes are still discouraged by Drush maintainers.
weitzman added a commit that referenced this issue Mar 29, 2016
commit 0438baf
Author: Moshe Weitzman <[email protected]>
Date:   Tue Mar 29 10:11:33 2016 -0400

    A fix to the implode() of sanitize queries.

commit 8ed8452
Author: Moshe Weitzman <[email protected]>
Date:   Mon Mar 28 08:59:03 2016 -0400

    Fix #1869 and #1582. Better support for database prefixes during sql-sanitize ... Database prefixes are still discouraged by Drush maintainers.
@weitzman
Copy link
Member

I updated the example in https://github.com/drush-ops/drush/blob/master/drush.api.php#L282 to show current table wrapping best practice.

mikeker pushed a commit to mikeker/drush that referenced this issue Aug 10, 2017
commit 0438baf
Author: Moshe Weitzman <[email protected]>
Date:   Tue Mar 29 10:11:33 2016 -0400

    A fix to the implode() of sanitize queries.

commit 8ed8452
Author: Moshe Weitzman <[email protected]>
Date:   Mon Mar 28 08:59:03 2016 -0400

    Fix drush-ops#1869 and drush-ops#1582. Better support for database prefixes during sql-sanitize ... Database prefixes are still discouraged by Drush maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants