oqgraph-dev team mailing list archive

[Arjen Lentz <arjen@xxxxxxxxxxxxx>]

Andrew, you got the right region of code. Latch is put into 'op' and then the bitflags are added.
See further down the switch to see how all combinations are covered and functions like Dijkstra are used.

I also see that the bit with only destid has #if 0 in v3 so that's why that doesn't return results. Antony, what's wrong with that v3 code? It worked in v2?

Keeping the internal latch enumeration is ok, I just reckon that the external exposure should be a string, with backwards compatibility for 0-2.
Clearer in queries and less messy when people extend it.

I think it might be good to split the cases differently in the switch, per latch rather than complete op. That way adding functions becomes cleaner. We can refactor that a bit later and our improved testsuite should keep us safe.

Arjen Lentz <arjen@xxxxxxxxxxxxx> wrote:

>Hi Andrew,
>
>The bitmap you found exists for latch=1 (Dijkstra) and possibly also breadth first (2 - check docs).
>So I think you tracked that bitmap rather than latch itself?
>See docs for the different operations the engine does,  not only based on latch but also dependent on which cols were specified. Do add comments on code for this in the code, that's useful.
>
>I'm fine with a stepwise refactor of latch. The varchar version also needs to accept the old numbers (as strings) but that's easy. MySQL autocasts so we don't need to deal with type foo outside of the engine.
>
>Of course v3 table structure has to change. 
>
>Andrew McDonnell <bugs@xxxxxxxxxxxxxxxxxxx> wrote:
>
>>Hi Arjen, Antony
>>
>>So I just had a quick look:
>>
>>The latch is currently an integer which eventually gets translated as an
>>operation in graphcore.cc oqgraph::search()
>>
>>Where the operation seems to be a bit set:
>>
>>NO_SEARCH | HAVE_ORIG | HAVE_DEST
>>NO_SEARCH | HAVE_ORIG
>>NO_SEARCH | HAVE_DEST
>>NO_SEARCH
>>
>>etc.
>>
>>And I guess you would like to translate to a string because that will be
>>easier to extend in the future?
>>
>>What syntax is required? (Apologies if you already discussed in passing
>>traffic and I missed it)
>>
>>
>>So, I have a back of the envelope plan to reduce the risk and try and meet the
>>merge:
>>
>>1. Implement the varchar interface but make no changes to oqgraph::search().
>>
>>A parser would be called in ha_oqgraph.cc just prior to calling
>>oqgraph::search(), translating to the bitset.  This way the interface upgrade
>>can integrated onto MariaDb quicker, to reduce the number of changes to be
>>tested, and allow the SQL-side interface to be bedded down and be merged.
>>
>>2. Then at a later date, refactor search() to take a string and do the
>>parsing.  This could be after the upcoming merge, as it would be entirely
>>internal to ha_oqgraph.cc, etc
>>
>>3. In the future, internally, search() could even be improved to have a kind
>>of lookup table and algorithm implementation class allowing easy addition of
>>new algorithms without changing oqgraph::search() itself when new algorithms
>>get added.  A kind of algorithm plugin for our storage plugin if you will!
>>
>>
>>Or I could be barking up the wrong tree ...
>>
>>
>>
>>--Andrew
>>
>>-- 
>>Mailing list: https://launchpad.net/~oqgraph-dev
>>Post to     : oqgraph-dev@xxxxxxxxxxxxxxxxxxx
>>Unsubscribe : https://launchpad.net/~oqgraph-dev
>>More help   : https://help.launchpad.net/ListHelp
>-- 
>Mailing list: https://launchpad.net/~oqgraph-dev
>Post to     : oqgraph-dev@xxxxxxxxxxxxxxxxxxx
>Unsubscribe : https://launchpad.net/~oqgraph-dev
>More help   : https://help.launchpad.net/ListHelp