jurismarches / luqum

A lucene query parser generating ElasticSearch queries and more !
Other
188 stars 40 forks source link

Bug in base operation #61

Closed velykanov closed 3 years ago

velykanov commented 3 years ago

Hey guys!

I'd like to know is there any reason spaces were removed from string representation? https://github.com/jurismarches/luqum/blob/48869be3903408e029f1b1532d9a16385ac462c8/luqum/tree.py#L398

Here's the code, that I'm running and facing the issue:

from luqum.tree import SearchField, AndOperation, Word
a = AndOperation(SearchField('po_cancelled', Word('false')), SearchField('deleted', Word('false')))

Previously I've had

str(a) == 'po_cancelled:false AND deleted:false'

After version 0.10.0 release I have

str(a) == 'po_cancelled:falseANDdeleted:false'

which brakes further code execution since syntactic is wrong

Could you please explain me another approach to build a query so my code would run as previously or add spaces back?

velykanov commented 3 years ago

I've found approach by using head and tail params:

from luqum.tree import SearchField, AndOperation, Word
a = AndOperation(SearchField('po_cancelled', Word('false'), tail=' '), SearchField('deleted', Word('false'), head=' '))
assert str(a) == 'po_cancelled:false AND deleted:false'

I have kinda function that looks like this one:

from luqum.parser import parser
from luqum.utils import LuceneTreeTransformer

class LuceneParser:
    def __init__(self, transformer: LuceneTreeTransformer):
        self.transformer = transformer

    def parse(self, query: str) -> str:
        tree = parser.parse(query)
        return str(self.transformer.visit(tree))

and non-fixed luqum library in requirements (which is obviously my own issue) thus code failed to execute once it was deployed.

And since visit is deprecated it seems a lot of code must be rewritten..

alexgarel commented 3 years ago

Hi @velykanov,

I know this change is disturbing.

there is an auto_head_tail to help you with that. See here (sorry I just saw readthedocs is not up to date :-( )

Also the LuceneTreeTransformer is deprecated but it wont be removed any time soon. Just you're encouraged to use the new pattern. See here. This new pattern is more robust than the previous one, which was causing subtle bugs.

velykanov commented 3 years ago

Thank you @alexgarel auto_head_tail looks like a nice solution, I'll check that out

I already changed my visits to yield values and won't use deprecated methods anyway) Especially if new transformer is more robust!