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 smallint support in Phalcon\Db #12523

Closed
wants to merge 2 commits into from
Closed

Added smallint support in Phalcon\Db #12523

wants to merge 2 commits into from

Conversation

Studentsov
Copy link
Contributor

This patch adds the capability to use the data type smallint, which is part of the SQL standard and is well supported by major DBMS. This data type uses 2 bytes instead of 4 bytes for type int, which will save memory if you don't need large numeric values.

Copy link
Contributor

@sergeyklay sergeyklay left a comment

Choose a reason for hiding this comment

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

@Studentsov Could you please cover new feature by unit tests?

@@ -142,6 +142,11 @@ class Column implements ColumnInterface
const TYPE_TIMESTAMP = 17;

/**
* Small integer abstract type
*/
const TYPE_SMALLINTEGER = 18;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about just TYPE_SMALLINT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have a type TYPE_BIGINTEGER. It would be very strange (though quite in the spirit of PHP 😆) to use the name TYPE_SMALLINT.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree

@sergeyklay sergeyklay added this to the 3.1.0 milestone Jan 3, 2017
@Jurigag
Copy link
Contributor

Jurigag commented Mar 14, 2017

Can you add here tinyint support too?

@Studentsov
Copy link
Contributor Author

Studentsov commented Mar 14, 2017 via email

@Jurigag
Copy link
Contributor

Jurigag commented Mar 14, 2017

Oh okay, so it could be at least added to mysql. Not sure about Column class.

@sjinks sjinks closed this Apr 26, 2017
@sjinks sjinks reopened this Apr 26, 2017
@sergeyklay sergeyklay modified the milestones: 3.2.0, 4.0.0 Jun 18, 2017
@niden
Copy link
Member

niden commented Oct 16, 2018

@Studentsov Could you be so kind as to rebase this for the 3.4.x branch so that we can merge this?

Apologies for letting this linger for this long

@niden niden self-assigned this Oct 22, 2018
@niden niden mentioned this pull request Oct 31, 2018
3 tasks
@niden
Copy link
Member

niden commented Oct 31, 2018

Addressed in #13562

@niden niden closed this Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants