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

Sphinx module update, adding limit/max_results support #2737

Closed
monetdb-team opened this issue Nov 30, 2020 · 0 comments
Closed

Sphinx module update, adding limit/max_results support #2737

monetdb-team opened this issue Nov 30, 2020 · 0 comments

Comments

@monetdb-team
Copy link

@monetdb-team monetdb-team commented Nov 30, 2020

Date: 2010-12-03 02:53:32 +0100
From: @skinkie
To: Stefan de Konink <>
Version: 5.22.1 (Oct2010) [obsolete]
CC: bugs-monetdb5

Last updated: 2011-03-28 17:31:42 +0200

Comment 15249

Date: 2010-12-03 02:53:32 +0100
From: @skinkie

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.208 Safari/534.10
Build Identifier:

Sphinx by default results 20 results. I need this to be dynamic. The attached improvement allows this.

Reproducible: Always

Comment 15250

Date: 2010-12-03 02:54:32 +0100
From: @skinkie

Created attachment 46
Update to support limit

I haven't tested this in production yet, it was a quick implementation. If some other patterns should be used in the module let me know.

Attached file: sphinx-update.diff (text/plain, 3883 bytes)
Description: Update to support limit

Comment 15299

Date: 2010-12-07 14:23:12 +0100
From: @grobian

couple of things:

Your patch is not API backwards compatible. Could you change it such that you just have a function with an extra argument, and that the default is to call that function with say 20?

Your patch includes commented out code, could you clean that up please?

Comment 15300

Date: 2010-12-07 15:15:59 +0100
From: @skinkie

Uuhh? Not compatible?

The old function call still exist right? And the internal default of Sphinx is 20. Am I overlooking something?

SPHINXsearchIndex calls SPHINXsearchIndexLimit

Regarding to the comments, that will be part of a 0.3 feature. If you can elaborate on the above, I'm happy to make a new patch.

Comment 15301

Date: 2010-12-07 21:15:39 +0100
From: @grobian

-command searchIndex(q:str, i:str) :bat[:oid,:lng]
+command searchIndex(q:str, i:str, l:int) :bat[:oid,:lng]

doesn't this change the API?

Comment 15302

Date: 2010-12-07 23:14:03 +0100
From: @skinkie

(In reply to comment 4)

-command searchIndex(q:str, i:str) :bat[:oid,:lng]
+command searchIndex(q:str, i:str, l:int) :bat[:oid,:lng]

doesn't this change the API?

Thanks Fabian, I see your point now.

Comment 15402

Date: 2011-01-24 13:32:54 +0100
From: @grobian

Ping. Any progress on this?

Comment 15448

Date: 2011-02-05 21:25:24 +0100
From: @skinkie

Created attachment 53
Updated patch, same API

Fabian can you please advise:

  • Is it required to have multiple wrapper functions? Or is it possible to do something with getArgReference to check the number of arguments?

  • How to implement 'auto loading' so --dbinit="include sphinx;" isn't required anymore.

Attached file: sphinx2.patch (text/plain, 4451 bytes)
Description: Updated patch, same API

Comment 15450

Date: 2011-02-08 14:36:00 +0100
From: @grobian

(In reply to comment 7)

Fabian can you please advise:

  • Is it required to have multiple wrapper functions? Or is it possible to do
    something with getArgReference to check the number of arguments?

You can do that, but maybe it is much nicer just to write a MAL-function that calls the ..Limit version with the extra argument 0 or something.

  • How to implement 'auto loading' so --dbinit="include sphinx;" isn't required
    anymore.

Look at e.g. the rdf dir/Makefile.ag. Create a e.g. 70_sphinx.mal file, that is installed in the autoload dir. The number is the priority, so chose something you think is wise.

Comment 15516

Date: 2011-03-05 18:49:06 +0100
From: @skinkie

(In reply to comment 8)

Look at e.g. the rdf dir/Makefile.ag. Create a e.g. 70_sphinx.mal file, that
is installed in the autoload dir. The number is the priority, so chose
something you think is wise.

I guess there is no such thing anymore there. Do you want me to commit my code and do hg export here. So at least the general code is committed?

Comment 15517

Date: 2011-03-05 18:53:14 +0100
From: @grobian

if you have an update patch, please attach it

Comment 15518

Date: 2011-03-05 19:02:52 +0100
From: @skinkie

(In reply to comment 10)

if you have an update patch, please attach it

I'm still quite puzzled what you ment with:

"You can do that, but maybe it is much nicer just to write a MAL-function that
calls the ..Limit version with the extra argument 0 or something."

Since the MAL functions:
SPHINXsearch calls SPHINXsearchIndex calls SPHINXsearchIndexLimit each fills in one of the gaps. Do you want me to change that?

My actual point was in SPHINXsearchIndexWrap/SPHINXsearchIndexLimitWrap since those are called by SQL, and duplicate a lot of code. Is anything possible in the pattern/address statements?

Comment 15519

Date: 2011-03-05 19:06:29 +0100
From: @grobian

(In reply to comment 11)

Since the MAL functions:
SPHINXsearch calls SPHINXsearchIndex calls SPHINXsearchIndexLimit each fills in
one of the gaps. Do you want me to change that?

no, that's fine

My actual point was in SPHINXsearchIndexWrap/SPHINXsearchIndexLimitWrap since
those are called by SQL, and duplicate a lot of code. Is anything possible in
the pattern/address statements?

you asked about variable arguments, my comment just referred to using multiple signatures if possible, before resorting to the getArg approach.

Comment 15520

Date: 2011-03-05 19:32:35 +0100
From: @skinkie

(In reply to comment 12)

you asked about variable arguments, my comment just referred to using multiple
signatures if possible, before resorting to the getArg approach.

command searchIndex(q:str, i:str) :bat[:oid,:lng]
address SPHINXsearchIndex
comment "Search the query on the specified indices";

command searchIndexLimit(q:str, i:str, l:int) :bat[:oid,:lng]
address SPHINXsearchIndexLimit
comment "Search the query on the specified index, with limit";

pattern sphinx_searchIndex(q:str, i:str) :bat[:str,:bat]
address SPHINXsearchIndexWrap
comment "Search the query on the specified index";

pattern sphinx_searchIndexLimit(q:str, i:str, l:int) :bat[:str,:bat]
address SPHINXsearchIndexLimitWrap
comment "Search the query on the specified index, with limit";

Could you elaborate on how that would look?

pattern sphinx_searchIndex(q:str, i:str, 20) :bat[:str,:bat]
address SPHINXsearchIndexWrap
comment "Search the query on the specified index";

Would the above be valid?

Comment 15521

Date: 2011-03-05 19:46:01 +0100
From: @grobian

(In reply to comment 13)

Could you elaborate on how that would look?

pattern sphinx_searchIndex(q:str, i:str, 20) :bat[:str,:bat]
address SPHINXsearchIndexWrap
comment "Search the query on the specified index";

Would the above be valid?

nope

function sphinx_searchIndex(q:str, i:str) :bat[:str,:bat]
ret := sphinx_searchIndexLimit(q, i, 20);
return ret;
end sphinx_searchIndex;

minus typos

Comment 15523

Date: 2011-03-05 20:50:30 +0100
From: @skinkie

Fabian could you give a hand what is wrong here?

function search(q:str) :bat[:oid,:lng]
ret := sphinx_searchIndexLimit(q, "*", 20);
return ret;
end search;
comment "Searches the query on all indices";

function searchIndex(q:str, i:str) :bat[:oid,:lng]
ret := sphinx_searchIndexLimit(q, i, 20);
return ret;
end sphinx_searchIndex;
comment "Search the query on the specified indices";

command searchIndexLimit(q:str, i:str, l:int) :bat[:oid,:lng]
address SPHINXsearchIndexLimit
comment "Search the query on the specified index, with limit";

function sphinx_searchIndex(q:str, i:str) :bat[:str,:bat]
ret := sphinx_searchIndexLimit(q, i, 20);
return ret;
end sphinx_searchIndex;
comment "Search the query on the specified index";

pattern sphinx_searchIndexLimit(q:str, i:str, l:int) :bat[:str,:bat]
address SPHINXsearchIndexLimitWrap
comment "Search the query on the specified index, with limit";

MonetDB upon startup returns:

!SyntaxException:parseError:ret := sphinx_searchIndexLimit(q, "*", 20);
!SyntaxException:parseError:^';' expected
!TypeException:sphinx.search[1]:'ret' may not be used before being initialized
!SyntaxException:parseError:comment "Searches the query on all indices";
!SyntaxException:parseError: ^';' expected
!SyntaxException:parseError:function searchIndex(q:str, i:str) :bat[:oid,:lng]
!SyntaxException:parseError:^';' expected
!SyntaxException:parseError:end sphinx_searchIndex;
!SyntaxException:parseError: ^non matching end label, overruled
!TypeException:user.main[15]:'user.sphinx_searchIndexLimit' undefined in: ret:any := user.sphinx_searchIndexLimit(q:any, i:any, _18:int)
!TypeException:user.main[15]:'q' may not be used before being initialized
!TypeException:user.main[15]:'i' may not be used before being initialized
!SyntaxException:parseError:comment "Search the query on the specified indices";
!SyntaxException:parseError: ^';' expected
!SyntaxException:parseError:command searchIndexLimit(q:str, i:str, l:int) :bat[:oid,:lng]
!SyntaxException:parseError:^';' expected
!SyntaxException:parseError:address SPHINXsearchIndexLimit
!SyntaxException:parseError: ^';' expected
!SyntaxException:parseError:comment "Search the query on the specified index, with limit";
!SyntaxException:parseError:^';' expected
!SyntaxException:parseError:ret := sphinx_searchIndexLimit(q, i, 20);
!SyntaxException:parseError:^';' expected
!TypeException:sphinx.sphinx_searchIndex[1]:'ret' may not be used before being initialized
!SyntaxException:parseError:comment "Search the query on the specified index";
!SyntaxException:parseError: ^';' expected
!SyntaxException:parseError:pattern sphinx_searchIndexLimit(q:str, i:str, l:int) :bat[:str,:bat]
!SyntaxException:parseError:^';' expected
!SyntaxException:parseError:address SPHINXsearchIndexLimitWrap
!SyntaxException:parseError: ^';' expected
!SyntaxException:parseError:comment "Search the query on the specified index, with limit";
!SyntaxException:parseError:^';' expected

Comment 15524

Date: 2011-03-05 20:57:46 +0100
From: @skinkie

Looks this 'works', now need to test my database without 'segfaults'...

command searchIndexLimit(q:str, i:str, l:int) :bat[:oid,:lng]
address SPHINXsearchIndexLimit
comment "Search the query on the specified index, with limit";

function search(q:str) :bat[:oid,:lng];
ret := searchIndexLimit(q, "*", 20);
return ret;
end search;

function searchIndex(q:str, i:str) :bat[:oid,:lng];
ret := searchIndexLimit(q, i, 20);
return ret;
end searchIndex;

pattern sphinx_searchIndexLimit(q:str, i:str, l:int) :bat[:str,:bat]
address SPHINXsearchIndexLimitWrap
comment "Search the query on the specified index, with limit";

function sphinx_searchIndex(q:str, i:str) :bat[:str,:bat];
ret := sphinx_searchIndexLimit(q, i, 20);
return ret;
end sphinx_searchIndex;

Comment 15525

Date: 2011-03-05 20:58:23 +0100
From: @grobian

indeed, missing semicolon in the example I gave you :)

Comment 15529

Date: 2011-03-05 21:40:15 +0100
From: @skinkie

Created attachment 56
Updated patch.

(In reply to comment 17)

indeed, missing semicolon in the example I gave you :)

Can you do a proper review? I have tested it, seems to work. Hope you are happy about it too.

Attached file: sphinx02.diff (text/plain, 4459 bytes)
Description: Updated patch.

Comment 15530

Date: 2011-03-05 21:47:10 +0100
From: @grobian

Patch looks good to me. Will you generate an hg export (with you as author)?

Comment 15531

Date: 2011-03-05 21:57:52 +0100
From: @skinkie

Created attachment 57
hg export

Exported.

Attached file: sphinx02-hg.diff (text/plain, 5376 bytes)
Description: hg export

Comment 15532

Date: 2011-03-05 22:04:13 +0100
From: @grobian

forgot bug message in commit, sorry, here it is
http://dev.monetdb.org/hg/MonetDB/rev/a1b9d3c35460

thanks for the update!

Comment 15654

Date: 2011-03-28 17:31:42 +0200
From: @sjoerdmullender

The Mar2011 version has been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant