Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for column collation #245

Merged
merged 3 commits into from
Feb 11, 2014

Conversation

hason
Copy link
Contributor

@hason hason commented Jan 8, 2013

No description provided.

@doctrinebot
Copy link

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-409

@@ -303,6 +312,11 @@ public function getDefault()
return $this->_default;
}

public function getCollation()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing comment

@guilhermeblanco
Copy link
Member

Patch seems good.
Still missing remaining drivers to deal with collation.
Patch could be merged once they're done.

@beberlei
Copy link
Member

Please dont add a new method getCollation()/setcollation() but use the options instead. Collatoin is sometihng database specific.

@hason
Copy link
Contributor Author

hason commented Mar 13, 2013

@beberlei, @adrienbrault Yours opinion?

* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong indentation of the license header

@hason
Copy link
Contributor Author

hason commented Apr 12, 2013

ping

@caponica
Copy link

Any chance this is going to make it into 2.4?

@Ocramius
Copy link
Member

@caponica hardly - we're in RC: only bugfixes I suppose...

@caponica
Copy link

Wasn't sure where we're at with 2.4. What more needs to be done to this one to make it ready for acceptance?

(i.e. if I can help just let me know how!)

@dazz
Copy link
Contributor

dazz commented Oct 14, 2013

Is there a state on this PR when it is going to be released?

@caponica
Copy link

Not that I've heard of, but lets hope it's soon!

public function postConnect(ConnectionEventArgs $args)
{
$connection = $args->getConnection();
$result = $connection->exec('SELECT VERSION()');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align = signs.

@till
Copy link
Contributor

till commented Nov 30, 2013

What's the status of this PR?

if ($column1->getUnsigned() != $column2->getUnsigned()) {
$changedProperties[] = 'unsigned';
foreach (array('type', 'notnull', 'default', 'unsigned', 'autoincrement') as $property) {
if ($properties1[$property] !== $properties2[$property]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strict comparison here changes the behaviour of the comparator. I don't know what side effects this creates. I would leave the loose comparison here. One example: You create a column with default value 0 (integer). Afterwards you reverse engineer the column from database and eventually it returns 0 as string. Then using strict comparison will generate unecessary changes.

@deeky666
Copy link
Member

@kimhemsoe I guess Drizzle also supports column collations. Can you maybe investigate and add support for it? This would make the support complete over all platforms.

@caponica
Copy link

@deeky666 Isn't it better to get this released with support for some platforms then add others later as enhancements? I know it's tantalising to be close to complete support, but I don't think it's a requirement.

@deeky666
Copy link
Member

@caponica Don't worry. I have done a PR hason#1 in @hason DBAL repo to add support for this :)

@hason
Copy link
Contributor Author

hason commented Jan 2, 2014

@beberlei @deeky666 Updated and rebased. Any chance to merge?

/**
* PostgreSQL 9.1 reserved keywords list.
*
* @author Martin HasOoň <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There something wrong here with your name ;) Awesome work on the keywords btw. You read my thoughts :)

@hason
Copy link
Contributor Author

hason commented Feb 11, 2014

@beberlei @guilhermeblanco @Ocramius The oldest PR begs you to merge :)

guilhermeblanco added a commit that referenced this pull request Feb 11, 2014
Added support for column collation
@guilhermeblanco guilhermeblanco merged commit 5b8f4f0 into doctrine:master Feb 11, 2014
@hason hason deleted the column_collation branch February 18, 2014 07:39
@till
Copy link
Contributor

till commented Feb 26, 2014

Late to this, just wanted to say thanks for getting this in!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.