Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 344695 - [DB/Core] Allow SQL Query handler to return Map instead of Object[]
Summary: [DB/Core] Allow SQL Query handler to return Map instead of Object[]
Status: CLOSED FIXED
Alias: None
Product: EMF
Classification: Modeling
Component: cdo.db (show other bugs)
Version: 4.0   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Eike Stepper CLA
QA Contact: Eike Stepper CLA
URL:
Whiteboard:
Keywords: noteworthy
Depends on: 345456
Blocks:
  Show dependency tree
 
Reported: 2011-05-04 08:15 EDT by Erdal Karaca CLA
Modified: 2011-06-23 03:38 EDT (History)
0 users

See Also:


Attachments
Added support for a Map being transferrable by CDO (7.31 KB, patch)
2011-05-04 08:16 EDT, Erdal Karaca CLA
no flags Details | Diff
Test case to handle sql result as map (2.44 KB, patch)
2011-05-05 07:24 EDT, Erdal Karaca CLA
stepper: iplog+
Details | Diff
This replaces CDOID.NULL by java null value when MAP is read by I/O on client side (7.34 KB, patch)
2011-05-11 09:03 EDT, Erdal Karaca CLA
stepper: iplog+
Details | Diff
Patch v3 (includes test) (20.68 KB, patch)
2011-05-11 12:38 EDT, Eike Stepper CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Erdal Karaca CLA 2011-05-04 08:15:24 EDT
I have a "simple" result set that returns a row consisting of 60+ columns.
Now, when doing a non object query an Object array will be returned and a I have to know where each value is located in that array.
A Map<String,Object> would add much more convenience as I would just have to reference the value by its sql column name.
Comment 1 Erdal Karaca CLA 2011-05-04 08:16:32 EDT
Created attachment 194704 [details]
Added support for a Map being transferrable by CDO
Comment 2 Eike Stepper CLA 2011-05-05 06:40:19 EDT
The idea is not bad. Some questions though:

1) Why do you call the new CDOType HASHMAP and not MAP?
2) Your I/O code implies String keys. Why aren't they handled more generally (like the values)? In case you want to keep the String keys, how could we rename the CDOType to make that clear?
3) Can you write a couple of test cases to protect us against regressions?

Thank you, Erdal ;-)
Comment 3 Eike Stepper CLA 2011-05-05 06:40:46 EDT
And please confirm that:

1) The number of lines that you changed is smaller than 250.
2) You are the only author of these changed lines.
3) You apply the EPL to these changed lines.
Comment 4 Erdal Karaca CLA 2011-05-05 07:17:39 EDT
1) I thought it should be HASHMAP as internally java.util.HashMap is used to create the MAP and there are other implementations available. Maybe, it is good to make the user aware of this by calling it "HASHMAP".
2) The jdbc column labels/names of a result set's meta data are always Strings. Maybe, we can call the type HASHMAP_OF_STRING_TO_OBJECT or if HASHMAP is not ok call it MAP_OF_STRING_TO_OBJECT?
3) I can provide some test cases in SQLQueryTest like testNonCDOObjectQueries_Complex and replace the indexes by String keys :-)

(In reply to comment #2)
> The idea is not bad. Some questions though:
> 
> 1) Why do you call the new CDOType HASHMAP and not MAP?
> 2) Your I/O code implies String keys. Why aren't they handled more generally
> (like the values)? In case you want to keep the String keys, how could we rename
> the CDOType to make that clear?
> 3) Can you write a couple of test cases to protect us against regressions?
> 
> Thank you, Erdal ;-)
Comment 5 Erdal Karaca CLA 2011-05-05 07:19:09 EDT
(In reply to comment #3)
> And please confirm that:
> 
> 1) The number of lines that you changed is smaller than 250.

I confirm.

> 2) You are the only author of these changed lines.

I am.
> 3) You apply the EPL to these changed lines.

I do.

(Sounds like a proposal of marriage :-)
Comment 6 Erdal Karaca CLA 2011-05-05 07:24:03 EDT
Created attachment 194815 [details]
Test case to handle sql result as map
Comment 7 Erdal Karaca CLA 2011-05-11 09:03:38 EDT
Created attachment 195340 [details]
This replaces CDOID.NULL by java null value when MAP is read by I/O on client side
Comment 8 Eike Stepper CLA 2011-05-11 12:01:19 EDT
Your test code reveals an issue with Map<String, Object>. To make the Java compiler happy one had to write:

      @SuppressWarnings("unchecked")
      List<Map<String, Object>> results = (List<Map<String, Object>>)(List<?>)query.getResult(Map.class);

I already see myself explaing that to users again and again. A horror :P

I've fixed that in bug 345456 so that we can now write:

	List<Map<String, Object>> results = query.getResult();
Comment 9 Eike Stepper CLA 2011-05-11 12:01:59 EDT
Regarding the name of the CDOType, how do you like "Record"?
Comment 10 Eike Stepper CLA 2011-05-11 12:26:25 EDT
Forget my RECORD naming proposal. I've rewritten your new CDOType so that it can work with arbitrary key types. Now it makes sense to call it MAP (HASHMAP would make it harder to exchange the impl class later).
Comment 11 Eike Stepper CLA 2011-05-11 12:38:42 EDT
Created attachment 195391 [details]
Patch v3 (includes test)
Comment 12 Eike Stepper CLA 2011-05-11 12:44:19 EDT
Committed revision 7678 (sorry this contains some changes that would belong to bug 345456)
Comment 13 Eike Stepper CLA 2011-05-11 12:44:44 EDT
Thank you, Erdal ;-)
Comment 14 Eike Stepper CLA 2011-06-23 03:38:58 EDT
Available in R20110608-1407