Skip to content

Error in table definitions for MySQL #543

Closed
shefik opened this Issue Oct 19, 2012 · 3 comments

2 participants

@shefik
shefik commented Oct 19, 2012

I get "Error in table definition", when trying to use this in tables.php (for the definition "year"):

"YEAR DEFAULT 'NULL'"

This is for MySQL. It worked before in Zikula 1.2.x, but no longer for Zikula 1.3.x. Looking at the code for DBUtil::getTableDefinition(), I see that there is no type map for "year". As a result, the preg_match throws an invalid exception. Is there any particular reason why "year" is not being mapped in Zikula 1.3.x? Or, was it just unintentionally overlooked?


I get "Error in table definition", when trying to use this in tables.php (for the definition "number"):

"N(3,1) DEFAULT 'NULL'"

This is for MySQL. It worked before in Zikula 1.2.x, but no longer for Zikula 1.3.x. Looking at the code for DBUtil::getTableDefinition(), I see that there is type map for "number". However, the preg_match throws an invalid exception, since it is now checking for a decimal ".", instead of a comma ",".Is there any particular reason why it was changed from a comma to a decimal?

When I change my module code to:

"N(3.1) DEFAULT 'NULL'"

The table successfully gets created with the column having the type:

"decimal(3,1)"


When trying to use this in tables.php (for the definition "decimal"):

"DECIMAL(10,2) DEFAULT 'NULL'"

The table successfully gets create. However, the column incorrectly has the column type:

"date"

This is for MySQL. It worked before in Zikula 1.2.x, but no longer for Zikula 1.3.x. However, I suppose I could just change my code to use:

"N(10.2) DEFAULT 'NULL'"

Since my intent is to have the column with the type:

"decimal(10,2)"


Based on the issues I have described above, I do have a fix to DBUtil support "year". But, before I submit a pull request, let me know, if there was a particular reason why it wasn't added in the first place.

Regarding the "decimal" problem, I recommend changing the code back to support something like this (using a comma):

"N(3,1) DEFAULT 'NULL'"

Instead of this (using a decimal point):

"N(3.1) DEFAULT 'NULL'"

Since that structure better matches the final MySQL result, which ends up being this (using a comma):

"decimal(3,1)"


Also the type DBUtil mapping currently has this code:

'N' => 'number',

However, it seems to me that the correct mapping should be:

'N' => 'decimal',

I modified the code, to test how it works both ways, and in either instance, it results in the intended type:

"decimal(3,1)"

So, for consistency in the code, shouldn't it be proper to have it changed to this

'N' => 'decimal',

Or, was there a particular reason why it was coded as:

'N' => 'number',


I can submit a pull request to fix all of the above issues I mentioned, but let's discuss first. Let me know, if there are any objections to the DBUtil code modifications I have in mind.

@phaidon
phaidon commented Jul 20, 2013

@cmfcmf @craigh @drak

Is that a dbutil code problem? There is no DBUtil in 1.3.x. So can we close that?

@ghost
ghost commented Jul 20, 2013

I would not bother with this, DBUtil is deprecated and nothing new should be written using it - we've been clear about this already. Use Doctrine 2 or you will suffer a lot very soon.

@phaidon
phaidon commented Jul 20, 2013

I will close it. If some one is against it feel free to reopen it.

@phaidon phaidon closed this Jul 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.