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.

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

Fix the issue #106 (https://github.com/pardom/ActiveAndroid/issues/106) #230

Merged
merged 1 commit into from
May 28, 2014
Merged

Fix the issue #106 (https://github.com/pardom/ActiveAndroid/issues/106) #230

merged 1 commit into from
May 28, 2014

Conversation

sabadow
Copy link

@sabadow sabadow commented May 28, 2014

When the library uses the cursor method getColumnIndex(String columnName) and has multiple columns with the same name, the android implementation doesn't return the first index, so the model entity is filled with wrong data.
Create a custom search of index and replace the usages.
Also add a junit test to check the issue and the fix.

@sabadow
Fix the issue #106 (#106)
c97998d 
When use the cursor method getColumnIndex(String columnName) and have multiple columns with same name, the android implementation not return the first index, so the model entity is filled with wrong data.
Create a custom search of index and replace the usages.
Also add a junit test to check the issue and the fix.
@sabadow sabadow mentioned this pull request May 28, 2014
Open
SeanPONeil added a commit that referenced this pull request May 28, 2014
@SeanPONeil
Merge pull request #230 from sabadow/master
834914c 
Fix the issue #106 (#106)
@SeanPONeil SeanPONeil merged commit 834914c into pardom-zz:master May 28, 2014
SSchlicht added a commit to rockabyte/ActiveAndroid that referenced this pull request Jun 13, 2014
@SSchlicht
chore (merge): pardom-zz#230
2caa3f8
@ghost
Copy link

ghost commented Nov 22, 2014

@sabadow How is your patch any different from

http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/2.3.1_r1/android/database/AbstractCursor.java/#295

Android returns the first index found.

Also new ArrayList<String> is redundant and overkill -- .asList already returns an array list.

Actually .asList is also too heavyweight, should just loop through the String[] array to get the index.

@sabadow
Copy link
Author

sabadow commented Nov 24, 2014

@lucastan i think the affected code is on SQLiteCursor, that overrides the AbstractCursor implementation: http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/2.3.1_r1/android/database/sqlite/SQLiteCursor.java#SQLiteCursor.getColumnIndex%28java.lang.String%29

On this code the column index operations are performed/stored on Map objects with the column as key. When the cursor has duplicate columns names, the key is the same and the value is override returning wrong column index.

Maybe you are right and my patch can be more efficient using an approach like the code you points on AbstractCursor

@ghost
Copy link

ghost commented Nov 24, 2014

I have created a pull request: #303 to mimic the AbstractCursor behavior.
But the owner of the project is not responding to any of the pull requests since May.

Anyway, how do we ensure the column names ordering (getColumnNames()) is the same as that you specified in the SQL statement? It seems like your patch is dependent on this "hidden" assumption for it to work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sabadow @SeanPONeil