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

adding TYPE_ENUM to Phalcon\Db\Column #11368

Merged
merged 3 commits into from Oct 21, 2018
Merged

adding TYPE_ENUM to Phalcon\Db\Column #11368

merged 3 commits into from Oct 21, 2018

Conversation

mattvb91
Copy link
Contributor

@mattvb91 mattvb91 commented Feb 4, 2016

PR to devtools will follow

@sergeyklay
Copy link
Member

Hi @mattvb91
What about other databases?
If someone moves from MySQL to Postgresql what would happen with those enum types.

In my opinion this PR this is not completed.

We have to support that type virtually for the other database engines supported.

@mattvb91
Copy link
Contributor Author

mattvb91 commented Feb 4, 2016

@sergeyklay do you mean just defaulting it to something like varchar for all the other adapters?

@sergeyklay
Copy link
Member

As I said above we have to support that type for the other database. For example we can transparent convert from Myslq's enum to Postgres' varchar. I'm not saying what you need to do exactly. I only express my opinion. Perhaps you'll propose a better idea

@mattvb91
Copy link
Contributor Author

mattvb91 commented Feb 4, 2016

@sergeyklay all the adapters default to either varchar or text if a column type isn't supported which in this case would still be ok as the result would still allow insertion of the data into the column.

@sergeyklay
Copy link
Member

And back?

@mattvb91
Copy link
Contributor Author

mattvb91 commented Feb 4, 2016

Im not sure if im missing something in your question but if porting data back INTO mysql from a data source not supporting enum it would also default the column to varchar.

@Dublerq
Copy link

Dublerq commented Mar 10, 2016

Please consider merging this pull request to 2.x. We're waiting for enums since 2014

@sergeyklay
Copy link
Member

@mattvb91 Could you please rebase?

if preg_match(sizePattern, columnType, matches) {

if definition["type"] == Column::TYPE_ENUM {
let definition["size"] = substr(columnType, 5, -1);
Copy link
Member

Choose a reason for hiding this comment

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

@mattvb91 Could you please explain this a bit more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sergeyklay this is where we were trying to use a regex but its easier to simply get the contents out of enum(CONTENTS) by doing a substr select.

So essentially if you have ENUM('val1', 'val2', 'val3') then definition["size"] = "'val1', 'val2', 'val3'"

Does that make more sense?

@sergeyklay
Copy link
Member

#10216 #11139

@sergeyklay
Copy link
Member

@mattvb91

@dcosta-ptc
Copy link

Any news regarding this topic?

@sergeyklay sergeyklay added this to the 3.1.0 milestone Aug 16, 2016
@sergeyklay sergeyklay self-assigned this Aug 16, 2016
@sergeyklay
Copy link
Member

@dcosta-ptc Will be part of 3.1.x

@Dublerq
Copy link

Dublerq commented Aug 16, 2016

I need to find some time to configure zephir and learn how to test/contribute. Looks like I'll need to speed up needed features development myself because waiting more than half a year to get a simple feature or fix is a bit too long.

@sergeyklay
Copy link
Member

sergeyklay commented Aug 16, 2016

@Dublerq sounds fine!

@mattvb91 mattvb91 changed the base branch from 2.1.x to 3.0.x October 8, 2016 13:13
@mattvb91 mattvb91 changed the base branch from 3.0.x to master October 8, 2016 13:14
@mattvb91 mattvb91 changed the base branch from master to 3.0.x October 8, 2016 13:24
@sergeyklay sergeyklay changed the base branch from 3.0.x to 3.1.x October 15, 2016 16:12
@sergeyklay sergeyklay modified the milestones: 4.0.0, 3.1.0 Jan 4, 2017
@sergeyklay sergeyklay force-pushed the 3.1.x branch 9 times, most recently from 4d3c73f to 6ef079c Compare April 5, 2017 15:25
@sjinks sjinks closed this Apr 26, 2017
@sjinks sjinks reopened this Apr 26, 2017
@denkurbatov
Copy link

And what now with ENUM ? I use Phalcon 3.1.2 and have problems with ENUM migrations.

@mattvb91 mattvb91 changed the base branch from 3.1.x to 4.0.x August 5, 2017 11:26
@mattvb91
Copy link
Contributor Author

mattvb91 commented Aug 5, 2017

@sergeyklay @dcosta-ptc @Dublerq sorry guys im not sure why im only now receiving notifications about this... I've rebased the branch and repushed. Looks like your builds are failing?

Surprised this hadn't been merged 2 years ago as its a pretty minor change from code side.

@sergeyklay
Copy link
Member

@mattvb91 I'll take a look a bit later. Thank you for contributing.

@jesugmz
Copy link

jesugmz commented Sep 6, 2017

3.2.1 and problems too. Any plan when it could be?

@aat2703
Copy link

aat2703 commented Sep 6, 2017

4.0.0 according to Sergey... Not usable without ENUM type for me... sadly... Migrations are pretty rough atm...

@ruudboon
Copy link
Member

ruudboon commented Sep 7, 2017

Hoping that this and some other types like TIME will get in at 4.0

@niden
Copy link
Sponsor Member

niden commented Oct 19, 2018

@mattvb91 Could you be so kind as to resolve the conflict with the changelog and have a look at the tests? It appears there is a syntax error somewhere. After that I can merge this.

I will need to add more tests on this one

@niden niden merged commit 6847fcc into phalcon:4.0.x Oct 21, 2018
@niden
Copy link
Sponsor Member

niden commented Oct 21, 2018

Thank you!

@niden niden added this to Done in 4.0.0 Release Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
4.0.0 Release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

10 participants