Filter fix #163
Filter fix #163
Conversation
floriankramer
force-pushed
the
filer_fix
branch
from
21a5a20
to
9458ce9
Compare
| popd | ||
| fi | ||
|
|
||
| # Launch the Server using the freshly baked index. Can't simply use a subshell here because | ||
| # then we can't easily get the SERVER_PID out of that subshell | ||
| pushd "$BINARY_DIR" | ||
| echo "Launching server from path $(pwd)" | ||
| ./ServerMain -i "$INDEX" -p 9099 -t -a --patterns &> server_log.txt & | ||
| ./ServerMain -i "$INDEX" -p 9099 -t -a &> server_log.txt & | ||
| SERVER_PID=$! | ||
| popd | ||
|
|
There was a problem hiding this comment.
Can you also remove the -P here: https://github.com/ad-freiburg/QLever/blob/master/Dockerfile#L39 otherwise it will break docker run without additional arguments
|
|
||
| optind = 1; | ||
| // Process command line arguments. | ||
| while (true) { | ||
| int c = getopt_long(argc, argv, "i:p:j:tauhPml", options, NULL); | ||
| int c = getopt_long(argc, argv, "i:p:j:tauhml", options, NULL); |
There was a problem hiding this comment.
Either keep "P" for no patterns or remove "P" in line 33
There was a problem hiding this comment.
The way I understand getopt we need a val in line 33 which will then be the value that getopt returns. If yout hink it helps readability I could swap out 'P' for a val;ue outside the ascii range (e.g. 128).
| @@ -91,7 +92,7 @@ int main(int argc, char** argv) { | |||
| optind = 1; | |||
| // Process command line arguments. | |||
| while (true) { | |||
| int c = getopt_long(argc, argv, "q:Ii:tc:lahuP", options, NULL); | |||
| int c = getopt_long(argc, argv, "q:Ii:tc:lahu", options, NULL); | |||
src/engine/Filter.cpp
Outdated
| return ValueReader<T>::get(l[lhs]) < | ||
| ValueReader<T>::get(r[lhs]); | ||
| }); | ||
| res->insert(res->end(), lower, upper); | ||
| } | ||
| } else { | ||
| getEngine().filter( | ||
| *static_cast<vector<RT>*>(subRes->_fixedSizeData), | ||
| [lhs, rhs, &subRes](const RT& e) { return e[lhs] == rhs; }, res); | ||
| input, [lhs, rhs, &subRes](const RT& e) { return e[lhs] == rhs; }, |
There was a problem hiding this comment.
I think that lambda doesn't actually need the &subRes
| bool onlyAddTextIndex = false; | ||
| bool keepTemporaryFiles = false; | ||
| optind = 1; | ||
| // Process command line arguments. | ||
| while (true) { | ||
| int c = getopt_long(argc, argv, "t:n:i:w:d:alT:K:PhAks:N", options, NULL); | ||
| int c = getopt_long(argc, argv, "t:n:i:w:d:alT:K:hAks:N", options, NULL); |
src/parser/SparqlParser.cpp
Outdated
|
|
||
| // _____________________________________________________________________________ | ||
| string SparqlParser::parseLiteral(const string& literal, bool isEntireString, | ||
| size_t off) { |
There was a problem hiding this comment.
can you add size_t off /*defaults to 0*/) so this is clear without looking into the header?
src/parser/SparqlParser.h
Outdated
| * If isEntireString is true an exception is thrown if the entire string | ||
| * is not a literal (apart from any leading and trailing whitespace). | ||
| **/ | ||
| static string parseLiteral(const string& literal, bool isEntireString, |
There was a problem hiding this comment.
Can you add a test for this method to SparqlParserTest
|
Ok I can confirm that this fixes the behavior described in #162 and the example query mentioned there. |
This pr removes the --patterns / -P option and replaces them with --no-patterns, making patterns the default option.
Furthermore it adds an aditional literal parsing step, that ensures literals are properly formatted regarding the count, positioning and escaping of qutation marks (as requested in #160).
Finally it adds support for filters on result tables that have more than 5 columns, presumably fixing #162.