[DBAL-44] nullable is not working for all datatypes Created: 30/Aug/10  Updated: 16/May/14  Resolved: 30/Aug/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Critical
Reporter: Daniel Freudenberger Assignee: Benjamin Eberlei
Resolution: Invalid Votes: 0
Labels: None

Attachments: Text File nullable.patch    
Issue Links:
Reference
is referenced by DDC-1045 Schema-tool update missbehavior: Not ... Closed

 Description   

The nullable=true annotation is ignored from at least following Types:

  • SmallIntType
  • DecimalType
  • BooleanType (not sure if nullable makes sense here)


 Comments   
Comment by Roman S. Borschel [ 30/Aug/10 ]

This looks like it has been fixed for a while already. See:

http://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/SmallIntType.php
http://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/DecimalType.php
http://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/BooleanType.php

Comment by Daniel Freudenberger [ 30/Aug/10 ]

I'll take a look at the master next time before submitting a bug report. Anyway I think it would be better to not fix bugs without an existing ticket for the fixed bug

Comment by Daniel Freudenberger [ 30/Aug/10 ]

has already been fixed





[DBAL-34] MySql getListTableForeignKeysSQL doesn't work for 5.0.xx Created: 19/Jul/10  Updated: 15/Aug/11  Resolved: 27/Jul/10

Status: Resolved
Project: Doctrine DBAL
Component/s: Schema Managers
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Ting Wang Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

MySQL Server version: 5.0.51a-24+lenny4 (Debian)



 Description   

The sql contains mysql specific code. And for the mysql 5.0.xxx the sql statement has syntax error.

in /Doctrine/DBAL/Platforms/MySqlPlatform.php:
public function getListTableForeignKeysSQL($table, $database = null)
{
$sql = "SELECT DISTINCT k.`CONSTRAINT_NAME`, k.`COLUMN_NAME`, k.`REFERENCED_TABLE_NAME`, ".
"k.`REFERENCED_COLUMN_NAME` /*!50116 , c.update_rule, c.delete_rule */ ".
"FROM information_schema.key_column_usage k /*!50116 ".
"INNER JOIN information_schema.referential_constraints c ON ".
" c.constraint_name = k.constraint_name AND ".
" c.table_name = '$table' */ WHERE k.table_name = '$table'";

if ($database)

{ $sql .= " AND k.table_schema = '$database' AND c.constraint_schema = '$database'"; }

$sql .= " AND `REFERENCED_COLUMN_NAME` is not NULL";

return $sql;
}

For the mysql lower as 5.1.16 the SQL could be as the following:

SELECT DISTINCT k.`CONSTRAINT_NAME`, k.`COLUMN_NAME`, k.`REFERENCED_TABLE_NAME`, k.`REFERENCED_COLUMN_NAME` FROM information_schema.key_column_usage k WHERE k.table_name = 'some_table' AND k.table_schema = 'some database' AND c.constraint_schema = 'some database' AND `REFERENCED_COLUMN_NAME` is not NULL

In this statement there is no reference of c



 Comments   
Comment by Benjamin Eberlei [ 27/Jul/10 ]

Fixed

Comment by Aigars Gedroics [ 15/Aug/11 ]

Pity that because of this the Doctrine schema-tool update action reports incorrect change list.
It tries to drop/add foreign keys because "ON DELETE CASCADE" option isn't read from the database at all.

Comment by Benjamin Eberlei [ 15/Aug/11 ]

5.0.x has no way to export the CASCADE details. Its just not possible to get this data in 5.0





[DBAL-37] auto increment can not be added to a column, when postgresql is used. Created: 22/Jul/10  Updated: 25/Jul/10  Resolved: 25/Jul/10

Status: Resolved
Project: Doctrine DBAL
Component/s: Platforms
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Ting Wang Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

Linux


Attachments: File autoincrementPatch.diff    

 Description   

To add auto increment to on column using the following code in the
migration class:

$systemSettingsTable->changeColumn('setting_id',
array('autoincrement' => true));
or
$settingIdColumn = $systemSettingsTable->getColumn('setting_id');
$settingIdColumn->setAutoincrement(true);

This works fine when MySQL is used. But when PostGreSQL is used, this
statement doesn't create any SQL statement.

The patch in attachment could perhaps resolve this problem some way.



 Comments   
Comment by Benjamin Eberlei [ 25/Jul/10 ]

Fixed in current master





[DBAL-33] Doctrine2 fails handling microseconds from PostgreSQL record Created: 17/Jul/10  Updated: 24/Jul/10  Resolved: 24/Jul/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Jan Tichý Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

Doctrine - GIT trunk (night build from 2010-07-17), Windows 7, PHP 5.3.2, PostgreSQL 8.4.3, Apache 2.2.15



 Description   

A column in database may be defined as both TIMESTAMP WITH TIMEZONE and TIMESTAMP WITHOUT TIMEZONE. If I insert a new value directly to the database through NOW() function, the value is stored including microseconds.

But then, when I am trying to load the record to Doctrine entity, following exception is thrown:

Doctrine\DBAL\Types\ConversionException

Could not convert database value "2010-07-17 15:29:57.762+02" to Doctrine Type datetimetz

File: C:\dev\etrener\library\Doctrine\DBAL\Types\ConversionException.php Line: 46

The same with both datetime and datetimetz columns.

The problem is probably in PostgreSqlPlatform::getDateTimeTzFormatString(), where is the following row:
public function getDateTimeTzFormatString()

{ return 'Y-m-d H:i:sO'; }

But PostgreSQL stores timestamps with microseconds, so format should be maybe something like:

{ return 'Y-m-d H:i:s.uO'; }

The problem is already posted in http://www.doctrine-project.org/jira/browse/DBAL-22?focusedCommentId=13574&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_13574, but that issue is already closed so maybe it was overlooked already then.

Thank you for fix!



 Comments   
Comment by Benjamin Eberlei [ 20/Jul/10 ]

Both DateTime and DateTimeTz assume that TIMESTAMP(0) is the definition, not the TIMESTAMP that implicitly degrades to TIMESTAMP(6). I am not sure how to handle this, I find this Postgres DateTime stuff rather annoying

Is there a global client side option for Postgres users to configure this?

Comment by Benjamin Eberlei [ 24/Jul/10 ]

@Jan: Did you use create the column manually yourself (TIMESTAMP WITHOUT TIMEZONE) or use the Doctrine Schema-Tool which defines TIMESTAMP(0) WITHOUT TIMEZONE.

Comment by Benjamin Eberlei [ 24/Jul/10 ]

Fixed in DBAL Trunk, see the following section of the DBAL manual to understand the workaround for PostgreSQL TIMESTAMP( n ) types where n > 0.

ORM always creates TIMESTAMP(0), so this is handled as a legacy database schema.

Comment by Benjamin Eberlei [ 24/Jul/10 ]
++ PostgreSQL

+++ DateTime, DateTimeTz and Time Types

Postgres has a variable return format for the datatype TIMESTAMP(n) and TIME(n)
if microseconds are allowed (n > 0). Whenever you save a value with microseconds = 0.
PostgreSQL will return this value in the format:

    2010-10-10 10:10:10 (Y-m-d H:i:s)

However if you save a value with microseconds it will return the full representation:

    2010-10-10 10:10:10.123456 (Y-m-d H:i:s.u)

Using the DateTime, DateTimeTz or Time type with microseconds enabled columns
can lead to errors because internally types expect the exact format 'Y-m-d H:i:s'
in combination with `DateTime::createFromFormat()`. This method is twice a fast
as passing the date to the constructor of `DateTime`.

This is why Doctrine always wants to create the time related types without microseconds:

* DateTime to `TIMESTAMP(0) WITHOUT TIME ZONE`
* DateTimeTz to `TIMESTAMP(0) WITH TIME ZONE`
* Time to `TIME(0) WITHOUT TIME ZONE`

If you do not let Doctrine create the date column types and rather use types with microseconds
you have replace the "DateTime", "DateTimeTz" and "Time" types with a more liberal DateTime parser
that detects the format automatically:

    [php]
    use Doctrine\DBAL\Types\Type;

    Type::overrideType('datetime', 'Doctrine\DBAL\Types\VarDateTime');
    Type::overrideType('datetimetz', 'Doctrine\DBAL\Types\VarDateTime');
    Type::overrideType('time', 'Doctrine\DBAL\Types\VarDateTime');
Comment by Jan Tichý [ 24/Jul/10 ]

@Benjamin: Yes, I have created the column manualy directly in database using my own CREATE TABLE definition.





[DBAL-22] Saving UTC Offset in Postgres DATETIME messes with PHP DateTime and Timezones Created: 14/Jun/10  Updated: 13/Jul/10  Resolved: 26/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: Platforms
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Guilherme Blanco Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

The DateTime type is currently unusable in PostgreSQL due to a wrong parsing format.

PostgreSqlPlatform::getDateTimeFormatString() should be changed from:

Y-m-d H:i:sO

to

Y-m-d H:i:s.uO

According to http://twitter.com/auroraeosrose/statuses/16154268629

Also, there's no way to check if a convert From and To PHP/Database worked correctly. Maybe we should introduce an exception when a DBAL\Type conversion fails, otherwise, there's no way to figure it out what happened.



 Comments   
Comment by Benjamin Eberlei [ 14/Jun/10 ]

throwing an exception is a no-go in my opinion, it would completely cripple an application if there is some change in the DB.

There are no checks if an integer field is really in int, or if the db has a varchar field making the (int) conversion fail. This should be ignored and is responsibility of the user to get right.

I am looking into the format, however we have several unit-tests for this working correctly, i am a bit confused.

Comment by Benjamin Eberlei [ 14/Jun/10 ]

The question is rather, should we deprecate the usage of timezones alltogether? This is a good read on the topic: http://derickrethans.nl/storing-date-time-in-database.html

Comment by Benjamin Eberlei [ 14/Jun/10 ]

ok the current state is, we use "TIMESTAMP(0) WITH TIMEZONE", which matches the currently given format "Y-m-d H:i:sO"

Comment by Elizabeth M Smith [ 14/Jun/10 ]

The "TIMESTAMP(0) WITH TIMEZONE" is a very UNCOMMON time field

Usually timstamp without time zone is used

Also the difference between integer field is really in int and db has a varchar field making the (int) is different for one reason

The timestamp is returning a datetime object - not a scalar

What happens when you try to use something in PHP that's not an object as an object? It fatal errors

If the integer field is not really an integer when you get your data back from the db and you attempt to use it, PHP is not going to throw a fatal error at you. Your application might be angry, but that's another story.

I would almost rather you simply use strtotime and pass that to the datetime class for postgresql since it will work on all 'versions" of postgresql's timestamp, regardless if it has microseconds or timezone information. Then you're not locking all users of doctrine into using one timestamp format.

Comment by Elizabeth M Smith [ 14/Jun/10 ]
$full = '2010-06-11 17:18:39.808397-04';
$no_micro = '2010-06-11 17:18:39-04';
$no_tz = '2010-06-11 17:18:39';
var_dump(DateTime::createFromFormat('Y-m-d H:i:sO', $full));
var_dump(DateTime::createFromFormat('Y-m-d H:i:s.uO', $full));

var_dump(new DateTime($full));
var_dump(new DateTime($no_micro));
var_dump(new DateTime($no_tz));

Quick test - why rely on createFromFormat when the datetime constructor is smart enough to handle all formats?

Comment by Benjamin Eberlei [ 14/Jun/10 ]

One reason is probably performance, using the constructor takes double the time:

$full = '2010-06-11 17:18:39.808397-04';
$no_micro = '2010-06-11 17:18:39-04';
$no_tz = '2010-06-11 17:18:39';

$n = 5000;

$ts = microtime(true);
for ($i = 0; $i < $n; $i++) {
    DateTime::createFromFormat('Y-m-d H:i:sO', $no_micro);
    DateTime::createFromFormat('Y-m-d H:i:s.uO', $full);
}
echo "DateTime::createFromFormat: " . number_format(microtime(true) - $ts, 4)."\n";

for ($i = 0; $i < $n; $i++) {
    new DateTime($full);
    new DateTime($no_micro);
}
echo "new DateTime: " . number_format(microtime(true) - $ts, 4)."\n";
DateTime::createFromFormat: 0.1173
new DateTime: 0.2496

Since the DateFormat is pretty static, therefore using createFromFormat() seems an obvious choice. This is of course mainly a good reason for MySQL where you cannot change the format, and Postgres, Oracle and DB2 allow changes in precision and formatting of the date which make this decision a little bit more complicated.

Comment by Elizabeth M Smith [ 14/Jun/10 ]

I think everyone BUT mysql allow changes in the precision and formatting of the date - heck SQL Server is the worst for "make the date be whatever you want"

I think this would fall into "premature optimization" - yes it's a great speed improvement for mysql, but it's at the expense of all other DBs...

Comment by Benjamin Eberlei [ 14/Jun/10 ]

A Doctrine\DBAL\DBALException due to a wrong date in the database probably leads to a fatal error in any userland code, or would anybody catch all the errors? how would you handle them?

We could think of changing the type to be without timezone, that would be a pretty massive BC though since it affects users database schemas.

Another option would be a configurable format per Postgres/Oracle/DB2 Platform:

$config = new \Doctrine\ORM\Configuration();
$config->setDateTimeGlobalFormat('Y-m-d H:i:s.uO');

However Type instances are flyweight instances, that means it would have to be the format of all Doctrine DateTime typed columns. However this way you would at least have full control over the format, not depending on any Doctrine 2 interpretation.

You can of course add your own datetime type and overwrite the existing or use different ones. Maybe we should supply two DateTime types.

Comment by Elizabeth M Smith [ 14/Jun/10 ]

I don't think an exception is a good idea
A PHP warning/notice might be a better solution so you at least know what failed (it shouldn't kill the script though, as a fatal error and exception do)

However a configurable would probably be a good idea...
Or two datetime types - one that allows a format to be used
Or even the ability to define a format for your datetime field (with a fallback to the default format)

For databases that talk to other things, not just PHP via Doctrine, dictating the format of the datetime fields is not an option

Comment by Benjamin Eberlei [ 17/Jun/10 ]

I changed the title to reflect the real issue of this ticket, using Postgresql with WITH TIMEZONE is rather problematic with regard to DateTime and DateTimeZone Handling inside PHP, see the following code snippets (http://pastie.org/1009033) and Dericks post (http://derickrethans.nl/storing-date-time-in-database.html).

We should change the default behaviour in Postgres (and Oracle) to be WITHOUT TIMEZONE, since this is the 90% use-case. Additionally it may create less bugs when we don't fiddle with the Timezone used for creation.

Comment by Benjamin Eberlei [ 21/Jun/10 ]

Ok we also discussed errors when conversion failed and added a ConversionException. We also implemented a new type DateTimeTz.

This is both currently in my feature branch: http://github.com/doctrine/dbal/tree/DBAL-22

Comment by Benjamin Eberlei [ 21/Jun/10 ]

This is a copy of a mail going to the doctrine-dev and doctrine-user lists some minutes ago:

Hello Doctrine 2 + Postgres and Oracle Users,

Both Postgres and Oracle currently save the Date Offset for DateTime
instances they are handling from Doctrine 2. However Date Offsets should
not be confused with Timezones and this can cause considerable issues
with transitions, modifications and comparisons of dates.

As a result we have to change the DateTime type implementations of
Oracle/Postgres for Beta 3 to reduce the risk of users running into date
calculation problems.

Required changes inside Doctrine DBAL Package:

  • The column create statement on both platforms will be changed from
    "TIMESTAMP(0) WITH TIME ZONE" to "TIMESTAMP(0) WITHOUT TIME ZONE".
  • The supported PHP date format will change from "Y-m-d H:i:sO" to
    "Y-m-d H:i:s".
  • Schema-Tool will automatically detect dates with offset retrieved from
    the Database as "DateTimeTz" types.
  • Wrongly converted date values will throw a "Doctrine\DBAL\Types
    \ConversionException". This will stop any non-migrated from running, but
    more importantly from corrupting your data.

What does that mean to you as a user?

POSTGRES:

There are two solutions if you use Postgres:

1. The easy way out: We will introduce a new Type "DateTimeTz", which
will keep backwards compability. You will however have to deal with the
Timezone issues yourself. One way is by setting your application
"date.timezone" ini variable to be an offset instead of a timezone on
your server. You have to make transition calculations yourself in this
case.

2. Converting the columns: When upgrading from Beta 2 to the master or
Beta 3 the type will change its behavior, conversions from a Beta 2 WITH
TIME ZONE column to the new type will fail, leading to a
"ConversionException" being thrown. However Schema-Tool will recognize
the changes automatically and ask you to convert the column. First tests
of me showed that converting the TIMESTAMP column from WITH to WITHOUT
timezone works, it even corrects all dates for the offsets to UTC.

ORACLE:

Oracle does not permit changing the types when there is already data in
it (guessing from the preliminary tests I made).
You have to switch all your entity fields using the "DateTime" type to
use the new "DateTimeTz" type otherwise you will experience an
"ConversionException" being thrown.

Planed Schedule for the changes:

  • The complete changes are currently in a DBAL feature branch on Github
    http://github.com/doctrine/dbal/tree/DBAL-22
  • Merge into the DBAL project this week. This WONT affect you using the
    ORM just yet, its a DBAL change only!
  • The ORM Master on Github will still be linked against the DBAL Beta 2
    via a Git Submodule (doctrine2/lib/vendor/doctrine-dbal).
  • In the timestamp between the DBAL Beta3 and the ORM Beta3 release we
    will integrate the changes into the ORM package also.

I will send an additional notice to the lists when we will bump the ORM
dependency on the DBAL.

Sorry for the inconvenience regarding this issue, but we feel very
strongly about making this change. This will ultimately solve many
subtle issues that would have popped up here and there.

Thanks goes to Elisabeth Smith for bringing this issue to the table and
to Derick Rethans who helped us understand why timezones/offset in the
database are a pain to work with.

greetings,
Benjamin

Comment by Benjamin Eberlei [ 26/Jun/10 ]

Merged into Master now

Comment by Václav Novotný [ 13/Jul/10 ]

Hi, on master branch of DBAL package in PostgreSqlPlatform::getDateTimeTzFormatString() I see this:

public function getDateTimeTzFormatString()

{ return 'Y-m-d H:i:sO'; }

But PostgreSQL stores timestamps with microseconds, so format should be more likely:

public function getDateTimeTzFormatString()

{ return 'Y-m-d H:i:s.uO'; }

Yes, Doctrine converts DateTimeTz from PHP value to database value without microseconds, but if I will use some database function (for example now()) for get timestamp and I will store it to the database directly and than I will read this values through Doctrine, it will cause an error.

Is there any reason for not using default PostgreSQL format of timestamp with microseconds?





[DBAL-32] Ensure that error mode is EXCEPTION if existing PDO instance used Created: 07/Jul/10  Updated: 07/Jul/10  Resolved: 07/Jul/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Improvement Priority: Minor
Reporter: Roman S. Borschel Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

I am not sure whether we do that already but I remember having talked about it. It would probably be good to ensure that error mode is EXCEPTION if an existing PDO instance is used to create a DBAL Connection. Simply forcing the error mode by using setAttribute(PDO::ATTR_ERRMODE, PDO::ATTR_ERRMODE_EXCEPTION) should be sufficient.



 Comments   
Comment by Benjamin Eberlei [ 07/Jul/10 ]

fixed





[DBAL-29] Comparator: Changing one column name and dropping one leads to changing both into the new name. Created: 29/Jun/10  Updated: 30/Jun/10  Resolved: 30/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

If you have a table and in a new version rename one and drop another column both of exactly the same definition, the comparator will errorusly optimize the one to rename both columns to the new one.

In these cases where it is not detectable which is the renamed and which the dropped it should drop both columns and create the new one. The user has to optimize this himself.



 Comments   
Comment by Benjamin Eberlei [ 30/Jun/10 ]

Fixed.





[DBAL-30] Doctrine\DBAL\Connection::query() method does not connect automatically to the database Created: 29/Jun/10  Updated: 30/Jun/10  Resolved: 30/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Tomasz Jędrzejewski Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

Linux, PostgreSQL 8.4



 Description   

The database access methods in Doctrine\DBAL\Connection automatically connect to the database, if the connection has not been created yet. This is not true in case of Doctrine\DBAL\Connection::query() - if we forget to connect manually, we get:

Warning: call_user_func_array() expects parameter 1 to be a valid callback, first array member is not a valid class name or object in /.../Doctrine/DBAL/Connection.php on line 608

The problem is caused by the lack of

$this->connect();

line within the method body.



 Comments   
Comment by Benjamin Eberlei [ 30/Jun/10 ]

fixed





[DBAL-17] E_STRICT error in Doctrine\DBAL\Driver\OCI8\Driver::_constructDsn() Created: 05/Jun/10  Updated: 28/Jun/10  Resolved: 28/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Trivial
Reporter: Eriksen Costa Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

MacOS X 10.5, PHP 5.3.2


Attachments: File patch.diff    

 Description   

I get an E_STRICT error in Doctrine\DBAL\Driver\OCI8\Driver::_constructDsn() because I forgot to set a 'dbname' key in the params array to Doctrine\DBAL\DriverManager.

My Doctrine is a svn checkout, from 2.0.0-beta1 tag.

A patch is attached (patch -p0 < patch.diff on project root).



 Comments   
Comment by Benjamin Eberlei [ 06/Jun/10 ]

is specifying the database name optional during connection to oracle databases? otherwise we could also throw an exception.

Comment by Eriksen Costa [ 06/Jun/10 ]

It's optional, according to <http://php.net/oci_connect>:

"If not specified, PHP uses environment variables such as TWO_TASK (on Linux) or LOCAL (on Windows) and ORACLE_SID to determine the Oracle instance to connect to."

I just spotted the error, I don't have an Oracle database neither the oci8 extension in my machine.

Comment by Benjamin Eberlei [ 28/Jun/10 ]

fixed anyways





[DBAL-28] Better error messages while creating schema Created: 28/Jun/10  Updated: 28/Jun/10  Resolved: 28/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: Schema Managers
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Improvement Priority: Minor
Reporter: Tomasz Jędrzejewski Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

Linux, PostgreSQL 8.4, PHP 5.3.2



 Description   

I'd like to suggest some improvements in the schema builder concerning error messages, which do not tell the most important information needed to find the problem quickly and without irritation. Below, I present a simple case I encountered yesterday: I had a quite big schema to generate, but I made a mistake in index column names. Doctrine thrown me the following exception:

Doctrine\DBAL\Schema\SchemaException
An unknown column-name name was given.

What I miss, is the table name, where I try to define such an index. I took a look at the messages for SchemaException and I noticed that none of them provides it, except those ones which refer directly to tables.

Another small, but useful improvement is enclosing the table/column/index/sequence/anything-else names in quotes or double quotes in order to make them better distinguishable from the message body. My first impression from the message above was that I have a "column-name" name defined somewhere, not a column named "name".

To sum up, the expected message should look like this:

Doctrine\DBAL\Schema\SchemaException
An unknown column-name 'name' was given for index 'foo' in table 'bar'.

The problem is minor, but I would be grateful if you remembered about it in the near future, so that I could have more useful error messages at least in the stable releases.



 Comments   
Comment by Benjamin Eberlei [ 28/Jun/10 ]

fixed, changed on other exceptions also.





[DBAL-26] DateTime type column can't be nullable Created: 27/Jun/10  Updated: 27/Jun/10  Resolved: 27/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Jakub Husák Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

After upgrade from B1 to B2 my app stopped working, throwing ConversionException on nullable DateTime field, value of which is null.
There is part of DateTimeType.php which causes it - if (!$val) matches even the valid null value.

public function convertToPHPValue($value, AbstractPlatform $platform)
{
$val = ($value !== null)
? \DateTime::createFromFormat($platform->getDateTimeFormatString(), $value) : null;
if (!$val)

{ throw ConversionException::conversionFailed($value, $this->getName()); }

return $val;
}



 Comments   
Comment by Jakub Husák [ 27/Jun/10 ]

maybe there should be a null check in _gatherRowData method, before the value is passed to convertToPHPValue of any type class

Comment by Benjamin Eberlei [ 27/Jun/10 ]

This is a bug in the ConversionException code, sorry for that i will fix it tonight.

Btw, you are not using Beta2 but trunk, the ConversionException code was just committed yesterday. Additionally you should use the ORM with the Beta2 not with trunk of DBAL, they don't work together currently in some aspects (SchemaTool)

Comment by Benjamin Eberlei [ 27/Jun/10 ]

This was introduced with DBAL-22 and is now fixed





[DBAL-11] Improve API of Sqllogger Created: 06/Nov/09  Updated: 20/Jun/10  Resolved: 20/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Improvement Priority: Minor
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 3
Labels: None


 Description   

The SqlLogger currently has a very simple API, which lacks for example the possibilty to log the execution time of queries. This would require a second call of the logger however, but i made some performance tests on the current implementation. Its using the following check to look if logging is enabled:

if($this->_config->getSqlLogger())

However:

if($this->_hasLogger)

Only takes half the time. See the test at the bottom.

We could switch to a boolean flag and use the "saved" time to enhance the SQL Logger API to include the following information:

1. SQL (Have already)
2. Params (Have already)
3. Elapsed Time (Start Time and End Time)
4. Query Type (SELECT, INSERT, UPDATE, DELETE, OTHER)

This would benefit both Symfony and ZF since they could hook into the logger for their respective debugging facilities (sf Web Toolbar, Zend Firebug SQL Profiling).

<?php                                                                                             
class Config                                                                                      
{                                                                                                 
    protected $_logger;                                                                           
    public function __construct()                                                                 
    {                                                                                             
        $this->_logger = new stdClass();
    }
    public function getLogger()
    {
        return $this->_logger;
    }
}

class Foo
{
    protected $_config = null;
    public function __construct()
    {
         $this->_config = new Config();
    }
    public function execute()
    {
         if($this->_config->getLogger()) {
         }
    }
}

class Bar
{
    protected $_config = null;
    protected $_loggerExists = false;
    public function __construct()
    {
         $this->_config = new stdClass();
         $this->_loggerExists = true;
    }
    public function execute()
    {
        if($this->_loggerExists) {

        }
    }
}

$f = new Foo();
$s = microtime(true);
for($i = 0; $i < 100; $i++) {
    $f->execute();
}
echo (microtime(true)-$s)."s\n";

$f = new Bar();
$s = microtime(true);
for($i = 0; $i < 100; $i++) {
    $f->execute();
}
echo (microtime(true)-$s)."s\n"
benny@benny-pc:~/code/php/wsnetbeans/Doctrine/trunk/tests$ php /tmp/boolvsmethod.php                
0.00055313110351562s                                                                                  
0.00025200843811035s                                                                                  
benny@benny-pc:~/code/php/wsnetbeans/Doctrine/trunk/tests$ php /tmp/boolvsmethod.php                
0.00051999092102051s                                                                                
0.00025200843811035s                                                                                
benny@benny-pc:~/code/php/wsnetbeans/Doctrine/trunk/tests$ php /tmp/boolvsmethod.php                
0.00036001205444336s                                                                                
0.00017309188842773s                                                                                
benny@benny-pc:~/code/php/wsnetbeans/Doctrine/trunk/tests$ php /tmp/boolvsmethod.php                
0.0005180835723877s                                                                                 
0.00025010108947754s 


 Comments   
Comment by Roman S. Borschel [ 07/Nov/09 ]

Yea, its currently a known limitation that the times do not get logged. However, I have real doubts as to whether that is actually useful as such measurements from PHP are very inaccurate. Its much better to copy+paste a SQL query to a SQL console and run it with ANALYZE/EXPLAIN ... to see its performance.

sfDoctrine2Plugin already uses this logger for its debug toolbar.

Comment by David Abdemoulaie [ 10/Apr/10 ]

Such timings are very useful. The accuracy isn't really the point. If you have N queries executing on a slow request, you have to check N queries to find the problem. With a simple "inaccurate" time measurement you'd be able to quickly identify the problem query(ies) and take further action.

Comment by Roman S. Borschel [ 14/May/10 ]

I think timings can be added.

Comment by Benjamin Eberlei [ 13/Jun/10 ]

Schedule for BETA 3

Comment by Benjamin Eberlei [ 20/Jun/10 ]

Implemented this change, the interface of the SQL Logger changed to:

function logSQL($sql, array $params = null, $executionMS = null);

Additionally the contract of loggers changed, they are now invoked AFTER a query is executed therefore queries that cause failures do not appear on the logs.





[DBAL-21] Pgsql: getListTableColumnsSQL: read domains info (patch provided) Created: 13/Jun/10  Updated: 20/Jun/10  Resolved: 20/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: 2.0.0-BETA2
Fix Version/s: 2.0.0-BETA3

Type: Improvement Priority: Major
Reporter: Raphaël Dehousse Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None

Attachments: Text File pgsql_domains.patch    

 Description   

Hello,

If you use DOMAIN (CREATE DOMAIN) in pgsql, PostgreSqlPlatform generate a "new \Doctrine\DBAL\DBALException("Unknown database type ".$dbType." requested, " . get_class($this) . " may not support it.");" exception because it does not know the type "domain_name" (CREATE DOMAIN domain_name AS VARCHAR(80); )

With this patch, it checks if the given type (either normal type, either domain) is a domain, and if yes, takes the right info (domain_name is type varchar and complete_type character varying(80)). If it's not a domain, it takes the normal type.

(patch -p0 doctrine2/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php < pgsql_domains.patch)

Also, it's not the goal here, but the queries could be optimized using joins in place of auto join + where, but, as said, the speed is not the goal since the queries are there only for entities mapping.

Hope it helps.

Best regards,

Raphaël Dehousse



 Comments   
Comment by Benjamin Eberlei [ 13/Jun/10 ]

Let me understand this, this patch automatically detects the underlying type of a new DOMAIN Type created in postgresql?

So say i create a MONEY type and have it be a decimal or something, your code would make doctrine automatically detect it being a decimal and create the decimal doctrine type?

Comment by Raphaël Dehousse [ 13/Jun/10 ]

Indeed, it will detect the "natural" type under MONEY and use this type for doctrine.

Comment by Benjamin Eberlei [ 20/Jun/10 ]

Implemented and tested in http://github.com/doctrine/dbal/commit/93ad4c2a24adf93d450c9eefbc966ccf3fe87414





[DBAL-2] Schema Comparator - Changes of only Identifier Strategy are not detected Created: 15/May/10  Updated: 13/Jun/10  Resolved: 13/Jun/10

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.0-BETA3

Type: Bug Priority: Major
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Duplicate
is duplicated by DDC-632 Lost auto_increment in all primary ke... Resolved

 Description   

When changing metdata from a NONE to an IDENTITY generator only for example on MySQL, SchemaComparator does not pick up the changes.



 Comments   
Comment by Benjamin Eberlei [ 06/Jun/10 ]

This should be fixed now

Comment by Benjamin Eberlei [ 10/Jun/10 ]

Introduced a regression

Comment by Benjamin Eberlei [ 13/Jun/10 ]

Fixed in current master, scheduled for Beta 3





Generated at Wed Oct 22 09:51:22 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.