Bugzilla – Full Text Bug Listing
| Summary: | [DB] Add new type mapping: Boolean from VARCHAR | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Erdal Karaca <erdal.karaca.de> | ||||
| Component: | cdo.net4j.db | Assignee: | Eike Stepper <stepper> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | Eike Stepper <stepper> | ||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | sraul.martins | ||||
| Version: | 4.2 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Erdal Karaca
Created attachment 194804 [details]
Added support for VARCHAR to Boolean
Moving all open enhancement requests to 4.1 Stefan, can you review this small contribution first? Erdal, thank you for your contribution. The management/initialization code seems ok so far, did you test it? (in the best case, please provide a TestCase for the DB test suite) I am currently unsure whether mapping/unmapping of the types work (do the JDBC drivers support get and set for Java type Boolean and DB type VARCHAR out of the box? If so, the code should work. If not, then you should implement a separate type mapping instead of reusing the existing one and do the conversion Boolean <-> String explicitly yourself. Moving all open issues to 4.2. Open bugs can be ported to 4.1 maintenance after they've been fixed in master. The JavaDoc for ResultSet.getBoolean() says: If the designated column has a datatype of CHAR or VARCHAR and contains a "0" [...] a value of false is returned. If the designated column has a datatype of CHAR or VARCHAR and contains a "1" [...] a value of true is returned. commit 2d90f84ddaaa0ec6dec264a5fa80ce516d96800c I still think that Boolean should be mapped to a some number. I was about to write something up but I couldn't test it on Oracle. Does someone want to submit a respective (tested) patch? I suggest to submit a separate bugzilla for this undertaking. From the tests I did, this is not enough to make this work. It would also be needed to map the EBoolean to VARCHAR (not only the EBooleanObject):
CoreTypeMappings:
public static final Factory FACTORY_VARCHAR = new Factory(TypeMappingUtil.createDescriptor(ID_PREFIX
+ ".Boolean_VARCHAR", EcorePackage.eINSTANCE.getEBoolean(), DBType.VARCHAR));
TypeMappingRegistry:
container.registerFactory(CoreTypeMappings.TMBoolean.FACTORY_VARCHAR);
However, we still have a problem with this approach: when CDO is creating the columns for the ecore types, it will create a column with type VARCHAR2(4000) for a boolean attribute, because the OracleAdapter is "adapting" the dbType BOOLEAN to VARCHAR.
And after all, CDO still stores 0 and 1 for the boolean (not 'false' or 'true').
So, I would definitely use the SMALLINT rather than VARCHAR.
Should I create a new bugzilla for this so I can attach a patch?
> So, I would definitely use the SMALLINT rather than VARCHAR.
>
> Should I create a new bugzilla for this so I can attach a patch?
Yes, please! ;-)
Maybe you can add the "EBoolean to VARCHAR" mapping at the same time? (In reply to comment #10) > Maybe you can add the "EBoolean to VARCHAR" mapping at the same time? Created bugzilla for the Boolean to NUMBER mapping: https://bugs.eclipse.org/bugs/show_bug.cgi?id=393917 Regarding the missing mapping for VARCHAR, do we really need it? If we decide to change the mapping from Boolean to NUMBER, then we could simply revert the changes from this commit. @Erdal, @Eike, what do you think? (In reply to comment #11) > (In reply to comment #10) > > Maybe you can add the "EBoolean to VARCHAR" mapping at the same time? > > Created bugzilla for the Boolean to NUMBER mapping: > https://bugs.eclipse.org/bugs/show_bug.cgi?id=393917 > > Regarding the missing mapping for VARCHAR, do we really need it? If we > decide to change the mapping from Boolean to NUMBER, then we could simply > revert the changes from this commit. > > @Erdal, @Eike, what do you think? Sure, I dont see any problems. Existing apps would have to somehow migrate their column definitions to use the new mapping (NUMBER(1)/SMALLINT instead of VARCHAR(5)). (In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > Maybe you can add the "EBoolean to VARCHAR" mapping at the same time? > > > > Created bugzilla for the Boolean to NUMBER mapping: > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=393917 > > > > Regarding the missing mapping for VARCHAR, do we really need it? If we > > decide to change the mapping from Boolean to NUMBER, then we could simply > > revert the changes from this commit. > > > > @Erdal, @Eike, what do you think? > > Sure, I dont see any problems. > Existing apps would have to somehow migrate their column definitions to use > the new mapping (NUMBER(1)/SMALLINT instead of VARCHAR(5)). The only way that existing apps managed to work in Oracle was by using the patches and local changes. So, in such cases, people either migrate the columns or continue to use local changes on the OracleAdapter. Agreed. I've reverted the commit: commit 776874bc9b9431ed0972c66da1094ef19ab023bb Closing |