Community
Participate
Working Groups
I have moved most of the Query related classes into org.eclipse.equinox.p2.metadata. However there seems to be a lot of things and we should review what we really want to make API and try to consolidate this.
I think we should consider bug 266971 as part of this.
And perhaps bug 289764 too.
We should also review the queries that are available across the rest of the p2 code and see if we can move some down in the core. For example in the UI I think we can move down: - AnyRequiredCapabilityQuery
Pascal, I'm just wondering why you removed the dependency on bug 266971? I wrote the query mechanism and I get confused with compound vs. composite. I think pipedQuery helps differentiate. The patch on bug 266971 is stale (since the query stuff was moved), but it's pretty easy to fix. I'm going to do this now so we can "hopefully" finish the query API stuff for M3.
I added bug 266971 back (I think maybe you removed it by accident). If you removed it on purpose, feel free to re-remove it and close the bug as WONTFIX.
I accidentally removed the depends on 266917 when I added 289764. Sorry about the confusion.
Created attachment 154580 [details] org.eclipse.equinox.p2.metadata.query.patch This patch moves the query stuff out of the provisional package. I'm putting it here so I don't loose it.
I have created the final packages for the p2 query and moved the classes. I have left the following classes as internal (for now) - IUPropertyQuery - LastestIUVersionQuery - ObjectMatchQuery (I'm not sure what this class is for) - UpdateQuery We still need to 1. Ensure all API classes have the proper @since tags 2. Ensure all API classes have class comments 3. Ensure all API methods have method comments 4. Ensure any classes / interfaces that shouldn't be extended / implemented have the proper API Tags
Not sure this is the right bugzilla, but I have one more request for the query API. I would like us to do the following: 1. Add the method iterator() to IQueryable. 2. Change the IQuery.perform(Iterator iterator) to perform(IQueryable queryable). The reason for this is to enable future indexing capabilities on IQueryable that the IQuery then can take advantage of. In many cases it will never need to ask the IQueryable for an iterator. Consider the common case where we resolve an IRequirement. The requirement itself is opaque but inside it, there's an expression that could contain logic to access an index where IU's can be obtained based on provided capability names. So rather then using the IQueryable.iterator() each time, it could use that index directly. Adding a generic index capability on IQueryable would make it possible to remove special handling for requirements in QueryableArray and instead let all potentially large queryables such as the SimpleMetadataRepository expose the appropriate index.
(In reply to comment #9) > Not sure this is the right bugzilla, but I have one more request for the query > API. I would like us to do the following: > > 1. Add the method iterator() to IQueryable. > 2. Change the IQuery.perform(Iterator iterator) to perform(IQueryable > queryable). > If we do this then the Queryable takes a Query and the Query takes a Queryable. Something doesn't feel right here.
I agree. It would be cleaner if the IQuery didn't exist and everything was expressions. Then the Queryable would evaluate the expression and return a result. An alternative would be that the queryable is responsible for creating the query so that the actual query instance is connected to the queryable. IQuery query = queryable.parseQuery(String expression); IQueryResult result = query.evaluate(<parameters>); or, in case we already have an expression: IQuery query = queryable.createQuery(IExpression expression); IQueryResult result = query.evaluate(<parameters>); This is fairly similar to JDBC where a connection is used to prepare statements which then can be evaluated multiple times with different set of parameters. This is a fairly large step though, since it effectively invalidates all queries that cannot be written as expressions. I would welcome that of course, but I don't know how others see it.
I am using 3.6M5 and had a few comments on the IQueryResult API. First off I would like to say I like the direction this is going especially compared to the old Collector way. Second, I would like to see IQueryResult extend java.lang.Iterable. This interface has a single method iterator() which IQueryResult already implements. If IQueryResult implemented this interface it could be used directly in a for loop: for (IInstallableUnit installableUnit : installableUnitQueryResult) { ... } instead of having to use an iterator pattern or calling toSet() or toArray() from within the for loop.
(In reply to comment #12) > I am using 3.6M5 and had a few comments on the IQueryResult API. > > First off I would like to say I like the direction this is going especially > compared to the old Collector way. > Great. > Second, I would like to see IQueryResult extend java.lang.Iterable. > Believe me, so would I (and many others). It is however not possible to do that without also abandon the Java 1.4 platform. That will not happen in 3.6.
(In reply to comment #12) > instead of having to use an iterator pattern or calling toSet() or toArray() > from within the for loop. I'd like to give you an advice here. both toSet() and toArray() are expensive since they always will copy the whole result. Unless you really need a copy, you should not use them. For iterating, the most optimal is: for(Iterator<SomeType> iter = result.iterator(); iter.hasNext();) { SomeType elem = iter.next(); } and if you think that's too ugly, you should use: for(SomeType elem : result.unmodifiableSet()) { } If you want to accumulate results. Do not use: resultCollector.addAll(queryResult.toSet()); // BAD creates an unnecessary copy instead use: resultCollector.addAll(queryResult.unmodifiableSet()); // GOOD. No unnecessary copy
I"m going to close this bug as the Query API is now public and all the dependent bugs are fixed. We can open specific issues as things arise.
