Closed
Bug 557536
Opened 14 years ago
Closed 10 years ago
checksetup.pl fails on ALTER DATABASE if database name contains hyphen
Categories
(Bugzilla :: Database, defect)
Bugzilla
Database
Tracking
()
RESOLVED
FIXED
Bugzilla 6.0
People
(Reporter: chris-bugzilla.mozilla.org, Assigned: selsky)
Details
Attachments
(1 file, 2 obsolete files)
1.85 KB,
patch
|
gerv
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Opera/9.80 (Windows NT 5.1; U; de) Presto/2.5.22 Version/10.51 Build Identifier: bugzilla-3.4.6.tar.gz I created an empty database named 'bugzilla-test' using phpMyAdmin 3.3.1, modified localconfig to use this database and run checksetup.pl. This script failed with: #1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-test CHARACTER SET utf8' at line 1 The reason is that "ALTER DATABASE bugzilla-test CHARACTER SET utf8" isn't a valid mysql query, due to missing quotes. Reproducible: Always Steps to Reproduce: 1. Create new database with a hyphen in its name 2. choose another charset than utf8 3. run checksetup.pl
Reporter | ||
Comment 1•14 years ago
|
||
After changing the ALTER DATABASE line there were no further issues with my this database name.
Comment 2•14 years ago
|
||
Hey Christopher. Thank you for your patch! If you'd like to get it into Bugzilla, see our development process, here: http://wiki.mozilla.org/Bugzilla:Developers
Reporter | ||
Updated•14 years ago
|
Attachment #437314 -
Flags: review?(mkanat)
Comment 3•14 years ago
|
||
Comment on attachment 437314 [details] [diff] [review] Patch to add quotes You probably also need to fix CREATE DATABASE and any other DATABASE commands that we use. You might want to use $dbh->quote_identifier.
Attachment #437314 -
Flags: review?(mkanat) → review-
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Bugzilla 3.6
Comment 4•13 years ago
|
||
Bugzilla 3.6 is now restricted to security fixes only, and this bug got no traction for several months. We will retarget this bug once it has a patch ready for checkin.
Target Milestone: Bugzilla 3.6 → ---
Assignee | ||
Comment 5•10 years ago
|
||
Assignee: database → selsky
Attachment #437314 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8530757 -
Flags: review?(gerv)
Comment 6•10 years ago
|
||
Comment on attachment 8530757 [details] [diff] [review] v2 >+ $self->do("ALTER DATABASE `$db_name` CHARACTER SET utf8"); Is ` legal here? And is it universal (MySQL, Pg, Oracle, SQLite)?
Assignee | ||
Comment 7•10 years ago
|
||
` is used by OBDC and MySQL (unless MySQL is in ANSI mode) per http://www.nntp.perl.org/group/perl.dbi.users/2003/11/msg20931.html It looks like everything else uses double-quotes.
Comment 8•10 years ago
|
||
Comment on attachment 8530757 [details] [diff] [review] v2 So you cannot use ` here as it's not supported everywhere.
Attachment #8530757 -
Flags: review?(gerv) → review-
Assignee | ||
Comment 9•10 years ago
|
||
But I'm only patching the MySQL module. And there's no support for obdc. Or do you want the quoting done always(even if not needed) in the parent module?
Comment 10•10 years ago
|
||
(In reply to Matt Selsky [:selsky] from comment #9) > But I'm only patching the MySQL module. And there's no support for obdc. Or > do you want the quoting done always(even if not needed) in the parent module? What's the behavior of Pg, SQLite and Oracle when DB names contain whitespaces or hyphens? If they require no quoting, then your patch would be fine. Else they would need to be fixed too.
Assignee | ||
Comment 11•10 years ago
|
||
SQLite works without any patch when the db name contain hyphens or whitespace. I don't have Oracle handy to test with. I can attempt to set up Pg unless someone else has a test environment handy.
Assignee | ||
Comment 12•10 years ago
|
||
Only mysql accesses $db_name outside of the DBI connect string (in the ALTER DATABASE query) so that's why only it needs the special quoting.
Comment 13•10 years ago
|
||
But all DB have their own get_create_database_sql() sub.
Comment 14•10 years ago
|
||
LpSolit: Matt's point is that this is MySQL-specific code (it's in Mysql.pm) and so it doesn't matter if he's using a MySQL-specific construct. That's precisely what such DB-specific files are for, isn't it? What's wrong with that logic? Gerv
Comment 15•10 years ago
|
||
Comment 10 hasn't been fully answered. If Oracle and Pg are affected too, they must also be fixed.
Assignee | ||
Comment 16•10 years ago
|
||
I've added the double-quoting for postgres and other databases since that's the standard. Only mysql uses the backticks.
Attachment #8536223 -
Flags: review?(gerv)
Assignee | ||
Updated•10 years ago
|
Attachment #8530757 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8536223 -
Flags: review?(gerv) → review+
Updated•10 years ago
|
Flags: approval?
Target Milestone: --- → Bugzilla 6.0
Comment 17•10 years ago
|
||
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git fe5bd2c..b93e6fe master -> master Gerv
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•