Also, Stan, I really appreciate that you still care about this... On Tue, Apr 11, 2017 at 6:42 PM, Niclas Hedhman <[email protected]> wrote:
> So, for now, I will require that ValueComposites that need to be > Queryable, they must extend ValueComposite. That works here locally, and I > think is a reasonable compromise for now. > > Cheers > > On Tue, Apr 11, 2017 at 5:28 PM, Stanislav Muhametsin < > [email protected]> wrote: > >> The check itself is rational, but maybe implementation is no longer >> adequate, since composite types are no longer required to extend >> XXXComposite interfaces provided by Polygene. >> Is there some way of asking of Core whether some arbitrary type is >> ValueComposite? >> It seems to be that by sorting this issue the SQL indexing might start >> working again, so IMO it is worth the hassle of investigating into it. >> >> >> On 11.4.2017 11:50, Niclas Hedhman wrote: >> >>> No, that is actually not the problem. It is that the CompositeDescriptor >>> of >>> types found in declared properties are determined by interfaces only, and >>> not looked up in the model. And it remains like that "forever", so it is >>> an >>> issue in the model/design, that was a consequence that wasn't realized >>> when >>> the dynamic addition of the type took place. >>> >>> It is apparent that indexing-sql is approaching this slightly different >>> than indexing-rdf and indexing-elasticsearch, but I am not convinced that >>> it does the wrong thing.. basically checking "these primitive types and >>> ValueComposites are allowed"... which seems rational enough to me. >>> >>> Cheers >>> >>> On Tue, Apr 11, 2017 at 4:36 PM, Stanislav Muhametsin < >>> [email protected]> wrote: >>> >>> Sorry about being inactive - weekend completely booked. >>>> But glad to hear that you have located the issue, Niclas! >>>> >>>> Could it be that indexing-sql tries to access type information about >>>> compoties "too soon"? >>>> If the core does the type adding in some place after extension-related >>>> code has already run... ? >>>> >>>> >>>> On 10.4.2017 15:34, Paul Merlin wrote: >>>> >>>> Awesome! >>>>> Sorry if I contributed to that mess:/ >>>>> >>>>> >>>>> Le 2017-04-10 13:49, Niclas Hedhman a écrit : >>>>> >>>>> I think I have located another "problem" in the indexing-sql >>>>>> implementation. And it has probably never worked. >>>>>> >>>>>> I am looking at >>>>>> org.apache.polygene.test.indexing.AbstractQueryTest#script29, which >>>>>> tries >>>>>> to search for a person that has "http" field in the URL of its >>>>>> personalWebsite. >>>>>> >>>>>> The startup code correctly traverse the types and sets up tables for >>>>>> all >>>>>> the qnames. >>>>>> >>>>>> But during Indexing, I am getting this; >>>>>> >>>>>> WARN o.a.p.i.s.s.s.SQLCompatEntityStateWrapper - Unsupported >>>>>> Property >>>>>> type: public abstract >>>>>> org.apache.polygene.api.property.Property<org.apache.polygen >>>>>> e.test.model.URL> >>>>>> >>>>>> org.apache.polygene.test.model.Person.personalWebsite() >>>>>> >>>>>> which is the "reason" why properties within that Value type are not >>>>>> indexed. >>>>>> >>>>>> Ok, so what is causing this? >>>>>> >>>>>> org/apache/polygene/index/sql/support/skeletons/SQLCompatEnt >>>>>> ityStateWrapper.java:64 >>>>>> >>>>>> retrieves the valueType() from propertyDescriptor and on >>>>>> org/apache/polygene/index/sql/support/skeletons/SQLCompatEnt >>>>>> ityStateWrapper.java:91 >>>>>> >>>>>> it checks if that is an instanceof ValueCompositeType, which it is >>>>>> not. >>>>>> Q.E.D >>>>>> >>>>>> But why is that? >>>>>> >>>>>> For instance, the propertyDescriptor for >>>>>> >>>>>> @Optional >>>>>> Property<Address> address(); >>>>>> >>>>>> is correct. But >>>>>> >>>>>> @Optional >>>>>> Property<URL> personalWebsite(); >>>>>> >>>>>> is invalid (URL is a test specific type and not java.net.URL). And the >>>>>> difference between Address and URL is that the former extends >>>>>> ValueComposite and latter doesn't. >>>>>> >>>>>> So, the reason seems to be exactly POLYGENE-137, i.e. fixed in RDF >>>>>> (elsewhere?) but wasn't fixed for indexing-sql. Unfortunately, no >>>>>> notes >>>>>> left behind on POLYGENE-137. And Paul's >>>>>> commit 7c2814ee145e91088ab6859147ef41c1d1ef8abe on 2 March 2017 >>>>>> mentions >>>>>> the POLYGENE-137 but a lot of other stuff in it at the same time. >>>>>> >>>>>> >>>>>> Ok, further... The ValueType that is provided, doesn't have >>>>>> ValueComposite >>>>>> in its types, as I would have expected. So why then, is ValueType not >>>>>> populated with ValueComposite, as I would have expected? Could it be >>>>>> that >>>>>> TypeLookup cuts some corners, or is it the whole model is doing >>>>>> something >>>>>> unexpected and doesn't build up all the descriptors correctly? Well, >>>>>> I am >>>>>> not sure, and I will try to figure that out tomorrow. >>>>>> >>>>>> At least, I have located what this particular problem (script29) is >>>>>> all >>>>>> about, and there is some hope that other test cases are failing for >>>>>> same >>>>>> reason. >>>>>> >>>>>> >>>>>> Cheers >>>>>> >>>>>> On Tue, Apr 4, 2017 at 6:17 PM, Niclas Hedhman <[email protected]> >>>>>> wrote: >>>>>> >>>>>> Thanks, I will try to dig into this. Worst case, we take indexing-sql >>>>>> off >>>>>> >>>>>>> the release specification. >>>>>>> >>>>>>> On Tue, Apr 4, 2017 at 5:26 PM, Stanislav Muhametsin < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>> On 4.4.2017 11:03, Niclas Hedhman wrote: >>>>>>> >>>>>>>> Indexing-SQL scans whole application structure on startup to detect >>>>>>>> all >>>>>>>> >>>>>>>>> the visible and indexable entity composite types, and if the things >>>>>>>>>> >>>>>>>>> changed >>>>>>>>> there, it might cause problems. >>>>>>>>> >>>>>>>>> Oy! Do you remember How/Where is that done? Because the >>>>>>>>> EntityComposite >>>>>>>>> type is added to the EntityDescriptor and must be done that way (or >>>>>>>>> the >>>>>>>>> Entity object with instanceof) and not be checking the Java >>>>>>>>> interfaces. >>>>>>>>> That could be the root of many problems. >>>>>>>>> >>>>>>>>> >>>>>>>>> It is "constructApplicationInfo" method in >>>>>>>> "extensions/indexing-sql/src/m >>>>>>>> ain/java/org/apache/polygene/index/sql/support/skeletons/Abs >>>>>>>> tractSQLStartup.java". >>>>>>>> However, I am looking at it (via GitHub - still no Java IDE here), >>>>>>>> and >>>>>>>> it >>>>>>>> doesn't _seem_ to use direct type check - it uses EntityDescriptors >>>>>>>> instead. >>>>>>>> You still might wanna make sure that it actually works as needed. >>>>>>>> >>>>>>>> Unfortunately, I couldn't find the code which emits "unsupported >>>>>>>> property >>>>>>>> type" messages (no text-content-search in Github :( ), but putting a >>>>>>>> breakpoint in there might reveal something crucial. >>>>>>>> I am very certain that I didn't get any of those when I got >>>>>>>> Indexing-SQL >>>>>>>> working back in the days. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> The other change is that identity is now a type and not String, and >>>>>>>> >>>>>>>>> somewhere that might cause problems. >>>>>>>>> >>>>>>>>> >>>>>>>>> Rewrite; Yeah, I have been considering this for a while. And >>>>>>>>> perhaps >>>>>>>>> build >>>>>>>>> something by leveraging QueryDSL (http://www.querydsl.com/), or at >>>>>>>>> least >>>>>>>>> borrow internals for it. But I concluded that it was more work than >>>>>>>>> fit >>>>>>>>> into 3.0 plan. But I would be really happy to get to a situation >>>>>>>>> where we >>>>>>>>> have "ES backed indexer", which would help a lot going forward. The >>>>>>>>> RDF >>>>>>>>> implementation is also getting "old"... >>>>>>>>> >>>>>>>>> Cheers >>>>>>>>> Niclas >>>>>>>>> >>>>>>>>> On Tue, Apr 4, 2017 at 3:15 PM, Stanislav Muhametsin < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>> Hi! >>>>>>>>> >>>>>>>>> Hmm yeah, it's been quite a while since I wrote that beast. :D I >>>>>>>>>> would >>>>>>>>>> do >>>>>>>>>> a lot of things differently now, but I guess we are stuck with >>>>>>>>>> what >>>>>>>>>> we >>>>>>>>>> have >>>>>>>>>> (until someone rewrites that). >>>>>>>>>> >>>>>>>>>> There was a discussion in October 2016, and I am not sure if the >>>>>>>>>> issue >>>>>>>>>> discussed then has been resolved, or maybe is (partially) the >>>>>>>>>> cause >>>>>>>>>> for >>>>>>>>>> errors. >>>>>>>>>> I will copy-paste my reply I wrote in October at the end of this >>>>>>>>>> mail. >>>>>>>>>> >>>>>>>>>> Looking at that issue link ( https://issues.apache.org/jira >>>>>>>>>> /browse/POLYGENE-222 ), there might be something in detection of >>>>>>>>>> entity >>>>>>>>>> composite types - looks like "the first bad commit" was something >>>>>>>>>> about >>>>>>>>>> entity composite types no longer needing to extend EntityComposite >>>>>>>>>> interface? >>>>>>>>>> Indexing-SQL scans whole application structure on startup to >>>>>>>>>> detect >>>>>>>>>> all >>>>>>>>>> the visible and indexable entity composite types, and if the >>>>>>>>>> things >>>>>>>>>> changed >>>>>>>>>> there, it might cause problems. >>>>>>>>>> >>>>>>>>>> The other commits are more unfamiliar territory for me - looking >>>>>>>>>> at >>>>>>>>>> attached test report, I think the most important information is >>>>>>>>>> the >>>>>>>>>> standard output. >>>>>>>>>> There is a bunch of "unsupported property type" messages - are >>>>>>>>>> associations and manyassociations in Polygene just Property<..> >>>>>>>>>> these >>>>>>>>>> days? >>>>>>>>>> That definetly will break things in Indexing-SQL. >>>>>>>>>> It is a bit hard to follow for me since so many things have >>>>>>>>>> changed >>>>>>>>>> in >>>>>>>>>> Polygene now (which is also a good thing - a sign of >>>>>>>>>> development!). >>>>>>>>>> >>>>>>>>>> Here is the copy-paste from October. >>>>>>>>>> It seems that the problem was Indexing-SQL not recognizing >>>>>>>>>> "Identity" >>>>>>>>>> type >>>>>>>>>> as primitive (something like 'String' or 'Integer'). >>>>>>>>>> >>>>>>>>>> -------- BEGIN QUOTED MAIL -------- >>>>>>>>>> In "extensions/indexing-sql/src/main/java/org/apache/zest/index >>>>>>>>>> /sql/support/skeletons/AbstractSQLStartup.java" file, there is >>>>>>>>>> "initTypes" method. >>>>>>>>>> You might want to add mapping for Identity.class in >>>>>>>>>> this._primitiveTypes >>>>>>>>>> and jdbcTypes, and also most likely this._customizableTypes. >>>>>>>>>> >>>>>>>>>> I say "might" want to, since I have really really vague memories >>>>>>>>>> on >>>>>>>>>> how >>>>>>>>>> that worked, and I realized I don't have any PC right now with >>>>>>>>>> Java >>>>>>>>>> coding >>>>>>>>>> environment set up. >>>>>>>>>> The "appendColumnDefinitionsForProperty" method in the same file >>>>>>>>>> uses >>>>>>>>>> the >>>>>>>>>> type mappings mentioned above to deduce what kind of column is to >>>>>>>>>> be >>>>>>>>>> created for property, which might be the issue here. >>>>>>>>>> >>>>>>>>>> Also, you probably want to modify the >>>>>>>>>> "/extensions/indexing-sql/src/ >>>>>>>>>> main/java/org/apache/zest/index/sql/support/common/QNameInfo >>>>>>>>>> .java" >>>>>>>>>> file, >>>>>>>>>> so that the detection whether some Java type is primitive is >>>>>>>>>> updated >>>>>>>>>> to >>>>>>>>>> include Identity. >>>>>>>>>> It is done just before setting this._isFinalTypePrimitive. >>>>>>>>>> >>>>>>>>>> Let us know if this is of any help to you. >>>>>>>>>> -------- END QUOTED MAIL -------- >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 03/04/2017 19:53, Paul Merlin wrote: >>>>>>>>>> >>>>>>>>>> Stan, Niclas, >>>>>>>>>> >>>>>>>>>> Indexing SQL has been broken for quite some time. >>>>>>>>>>> See https://issues.apache.org/jira/browse/POLYGENE-222 >>>>>>>>>>> >>>>>>>>>>> That issue contains the result of bisecting the history to >>>>>>>>>>> identify >>>>>>>>>>> when >>>>>>>>>>> it broke. With the Docker based testing infra it is now very >>>>>>>>>>> easy to >>>>>>>>>>> reproduce. >>>>>>>>>>> >>>>>>>>>>> Stan, you wrote that beast :) >>>>>>>>>>> Niclas, one commit of yours broke that beast :) >>>>>>>>>>> >>>>>>>>>>> Could you have a look? >>>>>>>>>>> >>>>>>>>>>> Thanks! >>>>>>>>>>> >>>>>>>>>>> /Paul >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>> Niclas Hedhman, Software Developer >>>>>>> http://polygene.apache.org - New Energy for Java >>>>>>> >>>>>>> >>>>>>> >>> >> > > > -- > Niclas Hedhman, Software Developer > http://polygene.apache.org - New Energy for Java > -- Niclas Hedhman, Software Developer http://polygene.apache.org - New Energy for Java
