DBAL-694: Extend AbstractSchemaManager::dropTable() signature #1916

Closed
doctrinebot opened this Issue Nov 29, 2013 · 6 comments

2 participants

@doctrinebot

Jira issue originally created by user velosipedist:

For now, dropTable($tableName) doesn't take into account useful IF EXISTS flag.

I can submit PR with myself if this proposal will be accepted.

@doctrinebot

Comment created by @deeky666:

[~velosipedist] can you please give some information of what you want to achieve with this addition? There are several problems I see here:

  1. I am quite sure a method signature changes are not possible here because of BC break (affects AbstractSchemaManager::dropTable and AbstractPlatform::getDropTableSQL).
  2. IF EXISTS clauses are not supported on all platforms and not SQL standard AFAIK.

Possible solutions depending on what you want to do:
1. If you just want to make sure that the DROP statement does not fail because of non-existence in DB you can use AbstractSchemaManager::tryMethod('dropTable', 'tablenamehere') which tries to drop the table but does not throw an exception if it failed. Drawbacks here are that other errors besides non-existence are silenced, too and you cannot evaluate if it really got deleted at database side.
2. Use AbstractSchemaManager::dropTable() and catch Doctrine\DBAL\DBALException. Check for the error code with DBALException::getCode() and evaluate it to the DBALException::ERRORUNKNOWNTABLE constant (currently only available in master branch and available in 2.5 when released - see #364).
3. Use AbstractSchemaManager::tableExists method to check if the table exists before calling AbstractSchemaManager::dropTable

You see there are lots of possibilities already and I am not quite sure if we should bloat the schema manager API with more alias methods (dropIfExists, createIfNotExists) that actually don't do anything special.

@doctrinebot

Comment created by velosipedist:

Thanks, while i used to wait for answer, i took solution #3 for iExists functionality purposes (test suite setUp & tearDown clean-ups).

And also thanks for DBALException::ERRORUNKNOWNTABLE hint — it solves problem finally, without API changes.

@doctrinebot

Comment created by velosipedist:

Found some workarounds, which worth to be documented

@doctrinebot

Issue was closed with resolution "Won't Fix"

@doctrinebot

Comment created by @deeky666:

[~velosipedist] The DBALException error constants are pretty new and need indeed documentation. I will provide that during the next days. Also these exception error codes are converted into an own exception class on the fly (see: #458). Not all DBALException::ERROR_* constants will get converted at this time though. Some are still missing but will be added soon. Then you are able to do something like this:

// Schema manager.

try {
    $sm->dropTable('mytable');
} catch (\Doctrine\DBAL\Excpetion\TableNotFoundException $e) { // exception class not yet available
    // do something
}

Besides that do you think there is still anything to do here? Or are those solutions acceptable for you?

@doctrinebot

Comment created by velosipedist:

Solution with tableExists() check is acceptable in my scenario, although it adds some db overhead in my tests.

Issue does not have sense now.

@beberlei beberlei was assigned by doctrinebot Dec 7, 2015
@doctrinebot doctrinebot closed this Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment