Skip to content
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

DRAFT: Add example to reproduce #143 #144

Closed
wants to merge 1 commit into from
Closed

Conversation

ximion
Copy link
Collaborator

@ximion ximion commented Aug 14, 2023

Hi!

This just adds some test code to reproduce the issue described in #143.
It uses an unconventional, manual way to construct the query, which leads to the node containing the token not being found.

@ximion ximion changed the title Add example to reproduce #143 DRAFT: Add example to reproduce #143 Aug 14, 2023
@hughsie
Copy link
Owner

hughsie commented Aug 15, 2023

So, the issue seems to be:

        /* search for token (fails) */
+       xb_silo_set_profile_flags(silo, ~0);
        query = xb_query_new(silo, "summary[text()~=?]", &error);

gives:

# XbMachine-DEBUG: opcodes_sig=FUNC:text,?INT,FUNC:search
# XbMachine-DEBUG: before optimizing: text(),?0,search()
# XbMachine-DEBUG: ignoring optimized call to search(text(),?0): ?INT:FUNC types not supported
# XbMachine-DEBUG: ignoring optimized call to text(): cannot optimize: no silo to query
# XbMachine-DEBUG: after optimizing: text(),?0,search()
# XbSilo-DEBUG: 0.01ms      query on component with `summary[text(),?0,search()]` [?0 → strategi] limit=0 -> 0 results

i.e. it's not matching FUNC:text,TEXT,FUNC:search as it's not a literal text value, it's a bound-at-query-time string. This means we can't optimize the query when it's created and we need to do it after calling xb_value_bindings_bind_str().

Does wip/hughsie/query-bound-tokenize make sense and work for you? If it does, then I'll rebase that and squash it down to make sense and do a PR.

@ximion
Copy link
Collaborator Author

ximion commented Aug 15, 2023

i.e. it's not matching FUNC:text,TEXT,FUNC:search as it's not a literal text value, it's a bound-at-query-time string. This means we can't optimize the query when it's created and we need to do it after calling xb_value_bindings_bind_str().

Ah, this suddenly makes so much sense! I really confused myself when investigating the issue ^^

Your proposed patch makes a lot of sense to me, I will test it later today (when I am at the right computer...) and report back. I do wonder whether there is any way I can work around this issue in AppStream... I could not bind the string at query time, but then I'd need to carefully sanitize the value beforehand to ensure it doesn't cause issues... It would also be a lot less flexible.
If this works as expected though, the search improvement for AppStream should be so massive that depending on a more recent libxmlb is probably the correct solution.

Thanks a lot for looking into this so quickly! :-D

@ximion
Copy link
Collaborator Author

ximion commented Aug 15, 2023

I have now tested wip/hughsie/query-bound-tokenize and it fixes the issue completely for me, AppStream's testsuite works now :-)
The changes also make logical sense to me, so, no objections!

(also, completely forgot I have commit access here - this pseudo-PR could have been a wip branch too ^^)

@hughsie
Copy link
Owner

hughsie commented Aug 15, 2023

Replaced by #145

@hughsie hughsie closed this Aug 15, 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
Development

Successfully merging this pull request may close these issues.

None yet

2 participants