polygene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niclas Hedhman <nic...@hedhman.org>
Subject Re: Sorting out issues with Indexing SQL
Date Tue, 11 Apr 2017 10:45:21 GMT
Also, Stan, I really appreciate that you still care about this...

On Tue, Apr 11, 2017 at 6:42 PM, Niclas Hedhman <niclas@hedhman.org> 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 <
> stanislav.muhametsin@zest.mail.kapsi.fi> 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 <
>>> stanislav.muhametsin@zest.mail.kapsi.fi> 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 <niclas@hedhman.org>
>>>>>> 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 <
>>>>>>> stanislav.muhametsin@zest.mail.kapsi.fi> 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
<
>>>>>>>>> stanislav.muhametsin@zest.mail.kapsi.fi> 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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message