Skip to content

fix: and-or simple query that contains string literals #10632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

ksqsf
Copy link
Contributor

@ksqsf ksqsf commented Dec 4, 2023

fixes #8354
fixes #8472
fixes #9143
fixes #10542
fixes https://discuss.logseq.com/t/simple-query-errors-on-combination-of-or-and-and-free-text-search-advanced-query-version-works/16897

Result

image

How and Why

In the case of (and e1.. (or e2..)), if a free string is found in e1 or e2, the form q will be wrapped one more time in add-bindings!:

or?
(cond
(->> (flatten form)
(remove (every-pred string? page-ref/page-ref?))
(some string?)) ; block full-text search
(concat [['?b :block/content '?content]] [q])
:else
q)

(Note [q])

This creates a malformed query:

image

(Note the double (()

You can more easily check it by (parse "(and \"foo\" (or \"bar\" \"baz\"))").

It turns out that there's no need to add yet another :block/content, as it is already handled by the (block-content) rule.

@ksqsf ksqsf changed the title fix: and-or query fix: and-or simple query that contains string literals Dec 4, 2023
Copy link
Collaborator

@logseq-cldwalker logseq-cldwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksqsf Thanks for fixing this long-standing issue with a test! 👍 ❤️ Confirmed this address failures in all the mentioned issues

@@ -329,6 +338,12 @@ prop-d:: nada"}])
(keep :block/content)
set)))

(is (= #{"DONE b2 [[page 1]]" "LATER b4 [[page 2]]"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@logseq-cldwalker logseq-cldwalker merged commit bfe7f5e into logseq:master Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants