Posted by neilnz on April 1, 2009 at 9:33pm
8 followers
| Project: | Email Registration |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
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
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| email_registration.module.patch | 1.36 KB | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch email_registration.module_3.patch. Unable to apply patch. See the log in the details link for more information. | View details |
Comments
#1
I don't have a test bed for this, but here's a better status ;)
#2
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.
#3
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.
#4
***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!
#5
Subscribing. An ANSI sql solution would be nice. BTW, thanks for a great module - we use it on hci.org - 200,000 member site.
#6
Here's a patch for review. Should be cross-db compatible, eliminating the need for db-specific solutions.
#7
#8
Tested, working fine!
#9
fyi - #657472: Add setting to allow users to login with email address or username depends on this patch
#10
@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) :)
#11
<?php+ // Strip leading and trailing spaces
+ $name = preg_replace('/^ +/', '', $name);
+ $name = preg_replace('/ +$/', '', $name);
?>
Why not just use trim?
#12
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.
#13
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.
#14
It would be great to get feedback on the patch in #13. I plan to commit it in ~2 weeks.
#15
Updated title, this is about more than just cross-db compatible business.
#16
The last submitted patch, 421078_unique_usernames_13.patch, failed testing.
#17
It applied with "patch -p1" but not with "git apply". Rerolled and passes tests locally.
#18
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?
#19
@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.
#20
Here's a patch for 6.x-1.x based on #19. I've tested it, but probably deserves an independent test.
#21
The last submitted patch, email_registration-6.x-1.x-unique_usernames-421078-20.patch, failed testing.
#22
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.
#23
#20: email_registration-6.x-1.x-unique_usernames-421078-20.patch queued for re-testing.
#24
Thanks, dwightaspinwall! Now committed http://drupalcode.org/project/email_registration.git/commit/56a2315 giving you credit as the author :)
#25
Automatically closed -- issue fixed for 2 weeks with no activity.