[
https://issues.apache.org/jira/browse/OAK-828?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13744832#comment-13744832
]
Thomas Mueller commented on OAK-828:
------------------------------------
Some minor remarks:
{code}
+ // pass-through impl
+ if (baseIndex.getNodeAggregator() == null) {
+ return baseIndex.getCost(filter, rootState);
+ }
+ // TODO dummy implementation
return baseIndex.getCost(filter, rootState);
{code}
I guess we could you always use pass-though, no matter if there is an
aggregator or not; I don't think aggregation has a significant I/O cost (we
don't care too much about CPU cost really, unless it's a really heavy
operations such as PDF text extraction).
{code}
public Cursor query(Filter filter, NodeState rootState) {
+ if (baseIndex == null) {
+ return Cursors.newPathCursor(new ArrayList<String>());
+ }
{code}
I would probably fail here if baseIndex is null, unless you need it for
testing. The problem is that it would hide a bug somewhere else (for example in
the configuration) if the index is used like that. If you need this for
testing, what about logging a warning "index is used inappropriately", or using
a special switch so that it works for testing but fails otherwise.
{code}
if (filter instanceof FilterImpl)
{code}
I would avoid "instanceof" as changing the FilterImpl might break things. What
about creating a copy constructor "FilterImpl(Filter copy)", or a "getCopy()"
method in the Filter interface + FilterImpl class?
{code}
public String getPlan(Filter filter, NodeState rootState) { ... }
public String getIndexName() { ... }
{code}
I wouldn't do the pass-through branch. I think it's slightly better if it's
visible in the logs (and while debugging) that the AggregateIndex is in use.
{code}
+ private Iterator<String> aggregates = null;
+
+ private String item = null;
{code}
Initializing fields with null is not needed.
AggregationCursor: the fields "aggregates" and "init" are sometimes set to
false/true/null. I think this could be simplified. But I guess the most
important thing is that it works and is well tested :-)
... to be continued ...
> Full-text support for index aggregates
> --------------------------------------
>
> Key: OAK-828
> URL: https://issues.apache.org/jira/browse/OAK-828
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: oak-lucene, query
> Reporter: Alex Parvulescu
> Assignee: Alex Parvulescu
> Attachments: aggregates.patch
>
>
> There's no support for indexing aggregates in Oak [0]
> I'd like to start slowly rolling some basic support in, so that at least
> assumptions like fulltext search on files work properly.
> [0] http://wiki.apache.org/jackrabbit/IndexingConfiguration
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
