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

boolean in MSSQL2Adapter #62

Closed
ilvalle opened this issue Feb 17, 2015 · 9 comments
Closed

boolean in MSSQL2Adapter #62

ilvalle opened this issue Feb 17, 2015 · 9 comments

Comments

@ilvalle
Copy link
Contributor

ilvalle commented Feb 17, 2015

The boolean data type is mapped to bit or boolean for all mssql adapters but MSSQL2Adapter.
Is there a special reason?

@niphlod
Copy link
Member

niphlod commented Feb 17, 2015

historical ones. mssql2:// is rather an experiment.

@ilvalle
Copy link
Contributor Author

ilvalle commented Feb 17, 2015

Given that, I'd set boolean as bit.

@mdipierro
Copy link
Contributor

No. That would break existing code. MSSQL2Adapter was created for those users who wanted a non-boolean mapping. We do not know that somebody is not relying on it. Why do you want to change it?

On Feb 17, 2015, at 9:45 AM, Paolo Valleri notifications@github.com wrote:

Given that, I'd set boolean as bit.


Reply to this email directly or view it on GitHub.

@ilvalle
Copy link
Contributor Author

ilvalle commented Feb 17, 2015

I use MSSQL2Adapter for a legacy db where boolean types are defined as bit. I fixed it on my local copy. If MSSQL2Adapter is marked experimental, we can still update it?

@niphlod
Copy link
Member

niphlod commented Feb 17, 2015

Mssql2 is a proof of concept of using unicode-capable fields. It's legacy,old and should be reviewed. Of course changing boolean mapping will break compatibility with current models

@ilvalle
Copy link
Contributor Author

ilvalle commented Mar 25, 2015

@mdipierro I've just realized that MSSQL2Adapter is broken anyway.
The following return False.

db = DAL('mssql2://user:passwd@ip/db')
db.define_table('tt', Field('aa', 'boolean', default=True))
db.tt.insert(aa=True)
db().select(db.tt.aa)[0].aa == True

Given that I guess no one is really using this adapter. Despite the backward compatibility, I propose to update it by switching the boolean type from CHAR(1) to BIT

@mdipierro
Copy link
Contributor

We do not know that nobody is using it. Can we make another one and
deprecate this?
On Mar 25, 2015 12:37 AM, "Paolo Valleri" notifications@github.com wrote:

@mdipierro https://github.com/mdipierro I've just realized that
MSSQL2Adapter is broken anyway.
The following return False.

db = DAL('mssql2://user:passwd@ip/db')
db.define_table('tt', Field('aa', 'boolean', default=True))
db.tt.insert(aa=True)
db().select(db.tt.aa)[0].aa == True

Given that I guess no one is really using this adapter. Despite the
backward compatibility, I propose to update it by switching the boolean
type from CHAR(1) to BIT


Reply to this email directly or view it on GitHub
#62 (comment).

@ilvalle
Copy link
Contributor Author

ilvalle commented Mar 25, 2015

Yes we could. The fact is that, the adapter is broken in a very basic operation. Given that I think we are in the position of fixing and breaking the compatibility, isn't?

@mdipierro
Copy link
Contributor

Will not be able to look at the code until later. Meanwhile feel free to
make a pr
On Mar 25, 2015 9:28 AM, "Paolo Valleri" notifications@github.com wrote:

Yes we could. The fact is that, the adapter is broken in a very basic
operation. Given that I think we are in the position of fixing and breaking
the compatibility, isn't?


Reply to this email directly or view it on GitHub
#62 (comment).

mdipierro added a commit that referenced this issue Mar 29, 2015
fix broken boolean type in MSSQL2Adapter. closes #62
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants