You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "Alex Parvulescu (JIRA)" <ji...@apache.org> on 2012/10/10 12:15:02 UTC
[jira] [Created] (OAK-371) Query package refactoring
Alex Parvulescu created OAK-371:
-----------------------------------
Summary: Query package refactoring
Key: OAK-371
URL: https://issues.apache.org/jira/browse/OAK-371
Project: Jackrabbit Oak
Issue Type: Sub-task
Components: core, jcr
Reporter: Alex Parvulescu
The task for the query related refactoring
--
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
[jira] [Updated] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu updated OAK-371:
--------------------------------
Attachment: query-refactoring-v4.patch
v4 with the missing files
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13474252#comment-13474252 ]
Michael Dürig commented on OAK-371:
-----------------------------------
One thing that occurred to me is that {{PropertyValue}} should either
# not implement {{PropertyState}} or,
# return an empty string for {{getName()}}
I'd prefer 1. since a {{PropertyValue}} is not a {{PropertyState}}
Will attach a patch
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Updated] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu updated OAK-371:
--------------------------------
Attachment: query-refactoring-v3.patch
last version of the patch before the switch to a dedicated value class.
fixed the last of the remaining CoreValue references: the property index and the query sort bits.
all the test pass, looking good.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473187#comment-13473187 ]
Michael Dürig commented on OAK-371:
-----------------------------------
Agreed. As I mentioned off line already, we might need to introduce such a representation for query/index. From going through the code this is the only place where it is really needed. I was thinking of an adapter which for {{PropertyState}}s which just forgets about the name:
{code}
public class PropertyValue {
public static PropertyValue create(PropertyState property) { /* ... */ }
boolean isArray() { /* ... */ }
Type<?> getType() { /* ... */ }
<T> T getValue(Type<T> type) { /* ... */ }
<T> T getValue(Type<T> type, int index) { /* ... */ }
long size() { /* ... */ }
long size(int index) { /* ... */ }
int count() { /* ... */ }
}
{code}
All these methods would just delegate to the underlying property state. The {{equals}}, {{hashCode}} and {{toString}} methods would have to be re-implemented "forgetting" the name.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Updated] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu updated OAK-371:
--------------------------------
Attachment: query-refactoring-v4.patch
v4 with a dedicated value class (org.apache.jackrabbit.oak.spi.query.PropertyValue).
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473228#comment-13473228 ]
Michael Dürig commented on OAK-371:
-----------------------------------
The two binary implementations where concerned with in memory binaries and binaries from the Microkernel. Both cases are covered through the various {{Blob}} implementations.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13474181#comment-13474181 ]
Alex Parvulescu commented on OAK-371:
-------------------------------------
> Regarding getString() for the binaries
See my previous comment about the getString behaviour of the alternative binary impl. If we take it out completely there's going to be one test failing [0]
The proposed (temporary) fix in v4 was to delegate the getString() call to the Binary#toString method and have that implemented only where it is reasonable to.
[0] testBinaryLiteral(org.apache.jackrabbit.test.api.query.qom.NodeLocalNameTest)
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Comment Edited] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473177#comment-13473177 ]
Alex Parvulescu edited comment on OAK-371 at 10/10/12 12:45 PM:
----------------------------------------------------------------
One of the things I've found missing and it's likely to break some code is the lack of a value container that has multiple values of different types.
Before, when using CoreValues you could have had a bunch of values that don't have the same type and live happily together.
Examples are:
- PropertyValueImpl: the case where we need all the properties of a node (* select) this cannot be expressed as one multiple-valued PropertyState, but would rather be an array of single valued PropertyState(s) but that conflicts with the entire AstElement#currentProperty definition
The refactoring that came with OAK-350 introduced the CoreValues.getValues helper but it casts all the included values to the property's type, which in the aforementioned cases may not be ideal.
[edit: I removed the PropertyIndexUpdate from the examples, it turns out it only contains string values (paths), so it doesn't apply]
was (Author: alex.parvulescu):
One of the things I've found missing and it's likely to break some code is the lack of a value container that has multiple values of different types.
Before, when using CoreValues you could have had a bunch of values that don't have the same type and live happily together.
Examples are:
- PropertyIndexUpdate an inverted index that has the property value as the index key and the CoreValues of all the matching nodes as the index value. This works only if we can add values of different types under one PropertyState. Otherwise we have to ensure type check on the property index, which means that a property index will be specific to a node type only.
- PropertyValueImpl: the case where we need all the properties of a node (* select) this cannot be expressed as one multiple-valued PropertyState, but would rather be an array of single valued PropertyState(s) but that conflicts with the entire AstElement#currentProperty definition
The refactoring that came with OAK-350 introduced the CoreValues.getValues helper but it casts all the included values to the property's type, which in the aforementioned cases may not be ideal.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Resolved] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu resolved OAK-371.
---------------------------------
Resolution: Fixed
Fix Version/s: 0.6
fixed with revision 1397098.
thanks Michael for the support & code review!
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13474416#comment-13474416 ]
Alex Parvulescu commented on OAK-371:
-------------------------------------
+1 good stuff
applied the patch with rev 1397221
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
> Attachments: OAK-371_Query_package_refactoring.patch, query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Updated] (OAK-371) Query package refactoring
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Dürig updated OAK-371:
------------------------------
Attachment: OAK-371_Query_package_refactoring.patch
Making {{PropertyValue}} not implement {{PorpertyState}}.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
> Attachments: OAK-371_Query_package_refactoring.patch, query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch, query-refactoring-v4.patch, query-refactoring-v4.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Updated] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu updated OAK-371:
--------------------------------
Attachment: query-refactoring.patch
proposed patch.
Some tests fail[1], but the 90% of the code is there.
feedback welcome!
[1] mostly node name stuff:
org.apache.jackrabbit.test.api.query.qom.NodeNameTest
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473214#comment-13473214 ]
Alex Parvulescu commented on OAK-371:
-------------------------------------
Another issue I've came across was the BINARY property creation code.
It turns out that the CoreValueFactoryImpl which is the default CoreValueFactory impl introduces an alternative BINARY impl (org.apache.jackrabbit.oak.kernel.BinaryValue) which has a different #getString signature.
Interestingly enough this custom #getString impl allowed the oak-core to pass a JCR test that called getString on a binary value. Replacing this call with a PropertyStates#binaryProperty made the test start failing. It took a while until I came across the alternative binary value implementation.
I'm wondering what the purpose of this alternative implementation is, and if there still is need for it on the oak-core.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473210#comment-13473210 ]
Alex Parvulescu commented on OAK-371:
-------------------------------------
+1 for PropertyValue
we can also make it Comparable and have a custom equals, this way it can absorb the corresponding methods from the PropertyStates class.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Updated] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu updated OAK-371:
--------------------------------
Attachment: query-refactoring-v2.patch
patch v2, all the tests pass
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473307#comment-13473307 ]
Michael Dürig commented on OAK-371:
-----------------------------------
Regarding {{getString()}} for the binaries, I don't think this is the right thing to do in general since it won't work for large binaries. Let's add a TODO there and figure out later how to fix it.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch, query-refactoring-v3.patch
>
>
> The task for the query related refactoring
--
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
[jira] [Commented] (OAK-371) Query package refactoring
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473177#comment-13473177 ]
Alex Parvulescu commented on OAK-371:
-------------------------------------
One of the things I've found missing and it's likely to break some code is the lack of a value container that has multiple values of different types.
Before, when using CoreValues you could have had a bunch of values that don't have the same type and live happily together.
Examples are:
- PropertyIndexUpdate an inverted index that has the property value as the index key and the CoreValues of all the matching nodes as the index value. This works only if we can add values of different types under one PropertyState. Otherwise we have to ensure type check on the property index, which means that a property index will be specific to a node type only.
- PropertyValueImpl: the case where we need all the properties of a node (* select) this cannot be expressed as one multiple-valued PropertyState, but would rather be an array of single valued PropertyState(s) but that conflicts with the entire AstElement#currentProperty definition
The refactoring that came with OAK-350 introduced the CoreValues.getValues helper but it casts all the included values to the property's type, which in the aforementioned cases may not be ideal.
> Query package refactoring
> -------------------------
>
> Key: OAK-371
> URL: https://issues.apache.org/jira/browse/OAK-371
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: core, jcr
> Reporter: Alex Parvulescu
> Attachments: query-refactoring.patch, query-refactoring-v2.patch
>
>
> The task for the query related refactoring
--
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