Skip to content

Added support for column inline comments in SQLite #819

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.

Sign up for GitHub

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

Jump to bottom
Merged
deeky666 merged 1 commit into from
Jan 10, 2016
Merged

Added support for column inline comments in SQLite #819

deeky666 merged 1 commit into from
Jan 10, 2016

Conversation

@hason
Copy link
Contributor

@hason hason commented Mar 18, 2015

No description provided.

@doctrinebot
Copy link

doctrinebot commented Mar 18, 2015

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-1176

We use Jira to track the state of pull requests and the versions they got
included in.

Ocramius
Ocramius reviewed Mar 18, 2015
View reviewed changes
lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php Outdated
Copy link
Member

@Ocramius Ocramius Mar 18, 2015

Choose a reason for hiding this comment

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

Uhm... why is DBAL not using a prepared statement here? :|

Copy link
Contributor Author

@hason hason Mar 18, 2015

Choose a reason for hiding this comment

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

Fixed

deeky666
deeky666 reviewed Mar 18, 2015
View reviewed changes
lib/Doctrine/DBAL/Platforms/SqlitePlatform.php Outdated
Copy link
Member

@deeky666 deeky666 Mar 18, 2015

Choose a reason for hiding this comment

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

Shouldn't we make sure here that $comment does not contain \n? Otherwise the generated SQL will be invalid. We could either strip newlines or throw exception...

Copy link
Contributor Author

@hason hason Mar 18, 2015

Choose a reason for hiding this comment

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

Fixed

@hason hason force-pushed the sqlite_comment branch 2 times, most recently from 4fca981 to c27e90f Compare March 18, 2015 12:29
deeky666
deeky666 reviewed Apr 30, 2015
View reviewed changes
lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php Outdated
Copy link
Member

@deeky666 deeky666 Apr 30, 2015

Choose a reason for hiding this comment

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

@hason this change is not part of your PR, is it? Can we try to not mix different things here?

@hason
Copy link
Contributor Author

hason commented Jun 16, 2015

@deeky666 This is necessary for correct handling custom types in ORM

@doctrinebot doctrinebot mentioned this pull request Dec 6, 2015
Closed
@deeky666 deeky666 merged commit a4dfce2 into doctrine:master Jan 10, 2016
deeky666 added a commit that referenced this pull request Jan 10, 2016
@deeky666
Merge pull request #819 from hason/sqlite_comment
3dcf0cc 
Added support for inline comments for SQLite

fixes #1118
@deeky666
Copy link
Member

deeky666 commented Jan 10, 2016

Merged manually via 3dcf0cc. Added more tests, made AbstractPlatform::getInlineColumnCommentSQL() throw exception if not supported and fixed some CS. Thanks @hason !

@deeky666 deeky666 added this to the 2.6 milestone Jan 10, 2016
@deeky666 deeky666 self-assigned this Jan 10, 2016
@Ocramius Ocramius changed the title Added support for inline comments for SQLite Added support for column inline comments in SQLite Jul 22, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@deeky666 deeky666

Labels

Columns Improvement Platforms Schema Definition Schema Introspection Schema Management SQLite Type Mapping

Projects

None yet

Milestone

2.6.0

Development

Successfully merging this pull request may close these issues.

4 participants

@hason @doctrinebot @deeky666 @Ocramius