Hi,

I wanted to use this module on Postgres, but your SQL query to fetch the next sequence number is MySQL-specific.

Patch attached for Postgres support.

Thanks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

Title: PostgreSQL support (patch) » PostgreSQL support for email_registration (patch)
Status: Active » Needs review

I don't have a test bed for this, but here's a better status ;)

greggles’s picture

Title: PostgreSQL support for email_registration (patch) » Rewrite name unique check so it is cross-database compatible
Status: Needs review » Needs work

This is not a real solution to the problem because it uses a switch. We should use standard sql.

As far as I can tell, this code is trying to find 1) if the name is unique, 2) if not, what is the highest "index" for the name. While it may be slightly slower, we should be able to do more of this in php and make it cross-database compatible.

dspring0021’s picture

Is there a solution to this. I'm running Drupal 6.19 and Email Registration 6.x-1.3 on PostgreSQL. I'm getting the errors below when registering a username of an email address that already exists in the db--as in the scenario outlined in this issue. The patch above failed. So, then, I tried to manually make the changes, but the code seems to be always using the mysql version of the query. I'm kind of stuck. Any advice on resolving this for pgsql would be much appreciated.

warning: pg_query() [function.pg-query]: Query failed: ERROR: syntax error at or near "REGEXP" LINE 1: ...UBSTRING_INDEX(name,'_',-1) FROM users WHERE name REGEXP '^d... ^ in /home/dspring/public_html/cert2/includes/database.pgsql.inc on line 139.
user warning: query: SELECT SUBSTRING_INDEX(name,'_',-1) FROM users WHERE name REGEXP '^dspring_[0-9]+$' ORDER BY CAST(SUBSTRING_INDEX(name,'_',-1) AS UNSIGNED) DESC LIMIT 1 in /home/dspring/public_html/cert2/sites/all/modules/email_registration/email_registration.module on line 34.
warning: pg_query() [function.pg-query]: Query failed: ERROR: duplicate key value violates unique constraint "users_name_key" in /home/dspring/public_html/cert2/includes/database.pgsql.inc on line 139.
user warning: query: UPDATE users SET name = 'dspring_1' WHERE uid = '307' in /home/dspring/public_html/cert2/sites/all/modules/email_registration/email_registration.module on line 45.

dspring0021’s picture

***UPDATE***

I lied in my post above. The patch does work for me (user error before). It will suffice as a workaround for now. Thank you!

dwightaspinwall’s picture

Subscribing. An ANSI sql solution would be nice. BTW, thanks for a great module - we use it on hci.org - 200,000 member site.

dwightaspinwall’s picture

Status: Needs work » Needs review
FileSize
2.9 KB

Here's a patch for review. Should be cross-db compatible, eliminating the need for db-specific solutions.

dwightaspinwall’s picture

Version: 6.x-1.1 » 6.x-1.x-dev
thePanz’s picture

Status: Needs review » Reviewed & tested by the community

Tested, working fine!

YK85’s picture

thePanz’s picture

@yaz085: to clarify your comment: the #657472-23: Add setting to allow users to login with email address or username patch includes this patch (it's not a dependency) ;) Maybe someone could work to split them (and then commit both) :)

greggles’s picture

+  // Strip leading and trailing spaces
+  $name = preg_replace('/^ +/', '', $name);
+  $name = preg_replace('/ +$/', '', $name);

Why not just use trim?

greggles’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.76 KB

Here's a rewrite/reroll for Drupal 7.

Using $new_name instead of $namenew - the standard in Drupal is to separate words in a variable by an underscore.

Using trim instead of preg_replace - seems likely that trim is faster and more thorough - http://maettig.com/code/php/php-performance-benchmarks.php backs up that perspective.

Perform the uniqueness check regardless of where the name comes from - this seems less likely to let a site-specific module or other contrib screw up the process.

Added some documentation about the need to validate the input to the function.

Flipped the do {} while loop so that a username can potentially come in with notting appended to the end. I don't see why the username should have to have something appended to the end.

greggles’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
FileSize
3.59 KB

Let's try that again :)

Updated for full d7 query style.

I realized that the existing code already handled letting through the username itself on the first pass when $i = 0 and is therefore empty.

greggles’s picture

It would be great to get feedback on the patch in #13. I plan to commit it in ~2 weeks.

greggles’s picture

Title: Rewrite name unique check so it is cross-database compatible » Abstract name unique check for re-use, make it is cross-database compatible, make it more robust, us it on external names

Updated title, this is about more than just cross-db compatible business.

Status: Needs review » Needs work

The last submitted patch, 421078_unique_usernames_13.patch, failed testing.

greggles’s picture

Status: Needs work » Needs review
FileSize
3.58 KB

It applied with "patch -p1" but not with "git apply". Rerolled and passes tests locally.

dwightaspinwall’s picture

Thanks for your work on this greggles. Are you planning to roll a patch of same functionality for 6.x-1.x-dev or should I do it? Here or in separate issue?

greggles’s picture

@dwightaspinwall - It would be lovely if you could review this and re-roll for 6.x-1.x. See #1878166: Create 7.x-1.1, 6.x-1.4 release for my general plan for 6.x. Most of these should be straightforward to re-roll for 6.x-1.x. It would be great if you could do some (or all) of that work.

dwightaspinwall’s picture

Here's a patch for 6.x-1.x based on #19. I've tested it, but probably deserves an independent test.

Status: Needs review » Needs work

The last submitted patch, email_registration-6.x-1.x-unique_usernames-421078-20.patch, failed testing.

greggles’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs work » Needs review

OK, committed to 7.x-1.x http://drupalcode.org/project/email_registration.git/commit/29d37d2

Let's see if the last one will pass the 6.x-1.x tests.

greggles’s picture

greggles’s picture

Status: Needs review » Fixed

Thanks, dwightaspinwall! Now committed http://drupalcode.org/project/email_registration.git/commit/56a2315 giving you credit as the author :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.