DDC-133: Complete implementation for schema support. #1945

Closed
doctrinebot opened this Issue Nov 9, 2009 · 16 comments

2 participants

@doctrinebot

Jira issue originally created by user romanb:

@doctrinebot

Comment created by romanb:

This mainly affects SqlWalker but probably many other parts also.

@doctrinebot

Comment created by romanb:

This shoudl wait for ddc-335 to finish.

@doctrinebot

Comment created by romanb:

How (if) his should be supported is still open to discussion. Maybe just @Table(name="schema.table") can work fine mostly and we dont need to bloat our implementation.

@doctrinebot

Comment created by romanb:

I tested this a bit with PostgreSQL and @Table(name="schema.table") and it seems to work fine. Even DDL generation/export works fine as long as the schema is created beforehand, just like databases. I think this is acceptable but still need to test further.

@doctrinebot

Comment created by andyajadeh:

Hello, I use postgresql, with PHP mapping driver like this:

<?php

use Doctrine\ORM\Mapping\ClassMetadataInfo;

$metadata->setInheritanceType(ClassMetadataInfo::INHERITANCE*TYPE*NONE);
$metadata->setPrimaryTable(array(
    'name' => 'sales.mdp',
    'schema' => 'sales',
));

$metadata->setChangeTrackingPolicy(ClassMetadataInfo::CHANGETRACKING*DEFERRED*IMPLICIT);
$metadata->setIdGeneratorType(ClassMetadataInfo::GENERATOR*TYPE*SEQUENCE);
$metadata->setSequenceGeneratorDefinition(array(
        'sequenceName'   => 'mdp*id*seq',
        'allocationSize' => 10,
        'initialValue'   => 1,
    ));

$metadata->mapField(array(
   'id'         => true,
   'fieldName'  => 'id',
   'columnName' => 'id',
   'type'       => 'integer',
  ));
$metadata->mapField(array(
   'fieldName'  => 'rrn',
   'columnName' => 'rrn',
   'type'       => 'string',
   'length'     => 50,
   'unique'     => true,
  ));

When I create database schema from CLI, it raises exception:

[Doctrine\DBAL\Schema\SchemaException]
Invalid index-name sales.mdp*rrn_uniq given, has to be [a-zA-Z0-9*]
@doctrinebot

Comment created by andyajadeh:

I try to remove unique => true, then it works. The mdp table is generated in sales schema.
Now when I I try to drop the database from CLI, I get an exception:

[PDOException]
SQLSTAT[42P01] Undefined table: 7 Error: table "mdp" does not exist.

it seems like dropping improper sequence or table name.

@doctrinebot

Comment created by romanb:

Ben, do you have an opinion on this? I think we can try to address this on the DBAL level by inspecting the table names whether they are of the form "foo.bar". The alternative would require full extra schema support on the ORM + DBAL level, so I'd rather try to do this the "hacky" way if possible (i.e. no explicit schema support, just "schema.table" as table name implicitly works) since most of the stuff works already with this.

The DBAL does not need to support creating schemas or stuff like that. Just ensuring that "schema.table" as a table name does not mess up DDL statements like constraint or index names in this example.

@doctrinebot

Comment created by @beberlei:

Fixed the auto-naming of indexes and constraints on the latest DBAL trunk, so this should continue to work @Andy can you verify?

@doctrinebot

Comment created by @beberlei:

Move to Beta3 to gather more feedback on the implicit definition

@doctrinebot

Comment created by andyajadeh:

Thanks Benjamin, Now it works. When I run orm:schema-tool:create, it creates the tables & indexes & constraints in the proper schema.
But, I still have problems when update / dropping the tables.

when I run orm:schema-tool:drop, I get an exception:

[PDOException]

  SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "mdp*id*seq" does not exist
  LINE 1: SELECT min*value, increment_by FROM mdp_id*seq

                                              ^

I use this in my map:

<?php

use Doctrine\ORM\Mapping\ClassMetadataInfo;

$metadata->setInheritanceType(ClassMetadataInfo::INHERITANCE*TYPE*NONE);
$metadata->setPrimaryTable(array(
    'name' => 'sales.mdp',
    'schema' => 'sales',
));

$metadata->setChangeTrackingPolicy(ClassMetadataInfo::CHANGETRACKING*DEFERRED*IMPLICIT);
$metadata->setIdGeneratorType(ClassMetadataInfo::GENERATOR*TYPE*SEQUENCE);
$metadata->setSequenceGeneratorDefinition(array(
        'sequenceName'   => 'sales.mdp*id*seq',
        'allocationSize' => 10,
        'initialValue'   => 1,
    ));

I want my sequence name to be also in sales schema. But when I drop it, I get that exception.

@doctrinebot

Comment created by ss10sb:

Since I was trying to break the schemaname.tablename by overcomplicating it and not doing a very good search here first (DDC-639, sorry Roman!), I happened to notice that the quoteIdentifier method in AbstractPlatform doesn't appear to be correctly quoting the schema and table name if quotes are needed. I'm not using any quoting but just glancing at the code, shouldn't $str be exploded on '.' in case of a schema.tablename string passed in? The current method looks like it will return (quote)schemaname.tablename(quote) instead of (quote)schemaname(quote).(quote)tablename(quote).
Maybe something along the lines of:

$expStr = explode('.', $str);
return $c . join("$c.$c", $expStr) . $c;

Awesome piece of work! It rocks!!

@doctrinebot

Comment created by @jwage:

The only problem I can think of is the identifier quoting like Scott mentions above. I had to "hack" this same thing in Doctrine 1. I don't like it :)

@doctrinebot

Comment created by romanb:

I think the limitation of not supporting identifier quoting in cases like this is not problematic so this hack is not needed. Identifier quoting is not supported everywhere anyway (i.e. not for join columns and discriminator columns).

@doctrinebot

Comment created by romanb:

Pushing back to beta4 to wait for more potential feedback. If anyone has issues with the current support for schemas via the table name, please speak up.

@doctrinebot

Comment created by romanb:

Closing this case for now as no further negative feedback was submitted. If new major issues arise that can not be addressed easily this issue needs to be reconsidered in a post 2.0 release.

@doctrinebot

Issue was closed with resolution "Fixed"

@beberlei beberlei was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot added this to the 2.0-BETA4 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment