Bug fix/internal issue #316 #7911
Conversation
Signed-off-by: Andrey Abramov <andrey@arangodb.com>
…-fix/internal-issue-#316
…-fix/internal-issue-#316 # Conflicts: # arangod/IResearch/AqlHelper.cpp # arangod/IResearch/AqlHelper.h # arangod/IResearch/IResearchOrderFactory.cpp # arangod/IResearch/IResearchOrderFactory.h # arangod/IResearch/IResearchViewNode.cpp # arangod/IResearch/IResearchViewNode.h # arangod/IResearch/IResearchViewOptimizerRules.cpp
| return false; | ||
| } | ||
|
|
||
| size_t const n = lhs->numMembers(); |
There was a problem hiding this comment.
Does it make sense to compare a node's flags attribute here?
In most cases the flags should not make a difference IMHO, just wondering if there are any cases where the flags would matter.
There was a problem hiding this comment.
I didn't get, which flags attribute do you mean?
There was a problem hiding this comment.
An AstNode has a flags member, which contains some computed information.
To my knowledge, that information can always be recomputed, so it should not matter when comparing two AstNodes.
| case aql::NODE_TYPE_OPERATOR_BINARY_TIMES: | ||
| case aql::NODE_TYPE_OPERATOR_BINARY_DIV: | ||
| case aql::NODE_TYPE_OPERATOR_BINARY_MOD: | ||
| case aql::NODE_TYPE_OPERATOR_BINARY_EQ: |
There was a problem hiding this comment.
are the array variants of the operators, e.g. NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ intentionally not supported here?
There was a problem hiding this comment.
Could you give me an example of how NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ can be expressed in AQL?
There was a problem hiding this comment.
NODE_TYPE_OPERATOR_BINARY_EQ: a == b
NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ: [1,2,3] ALL IN b
arangod/IResearch/AqlHelper.cpp
Outdated
| } | ||
|
|
||
| case aql::NODE_TYPE_VALUE: | ||
| case aql::NODE_TYPE_OBJECT: { |
There was a problem hiding this comment.
does this only need to handle object literals with already-known values, or does this also need to handle objects with dynamic values inside, e.g.
{ "a" : 1, "b": 2 } // object literal, should work when using aql::CompareAstNodes
{ "a" : foo.bar } // object with runtime values, will probably not work right now
{ [name] : 2 } // object with dynamic attribute name, will probably not work right now
There was a problem hiding this comment.
good catch, I have to cover object case manually
| case aql::NODE_TYPE_OPERATOR_BINARY_TIMES: | ||
| case aql::NODE_TYPE_OPERATOR_BINARY_DIV: | ||
| case aql::NODE_TYPE_OPERATOR_BINARY_MOD: | ||
| case aql::NODE_TYPE_OPERATOR_BINARY_EQ: |
There was a problem hiding this comment.
are the array variants of the operators, e.g. NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ intentionally not supported here?
There was a problem hiding this comment.
Could you give me an example of how NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ can be expressed in AQL?
arangod/IResearch/AqlHelper.cpp
Outdated
| auto sub = node->getMemberUnchecked(i); | ||
|
|
||
| if (sub) { | ||
| hash = fasthash64(static_cast<const void*>(sub->getStringValue()), |
There was a problem hiding this comment.
this assumes each object attribute member is of type NODE_TYPE_OBJECT_ELEMENT.
At least in theory object attribute members can also have the type NODE_TYPE_CALCULATED_OBJECT_ELEMENT, but I am not sure if that is allowed here.
Example:
{ [ CONCAT("test" + 1) ] : "foo" }
There was a problem hiding this comment.
need to check it, thanks
<
9E88
a href="/arangodb/arangodb/pull/7911/files/4030503d1df5c0e9290716669f534f32ac8669ce#diff-de5fd5adf820db07583462831e91cf7197e96d6e906e9aa30fb00c6f90d44e38" class="text-mono text-small Link--primary wb-break-all mr-2">arangod/IResearch/IResearchViewNode.cpp
Outdated
Show resolved
Hide resolved
| var scorers = ''; | ||
| if (node.scorers && node.scorers.length > 0) { | ||
| scorers = keyword(' LET '); | ||
| for (var j = 0;;) { |
There was a problem hiding this comment.
could be simplified to
scorers = keyword(' LET ' ) + node.scorers.map(function(scorer) {
return variableName(scorer) + ' = ' + buildExpression(scorer.node);
}).join(', ');
|
Apart from the above, I am wondering if the change of the scorer serialization data is a compatibility problem, e.g. while performing a rolling upgrade, when some servers in a cluster may use version 3.4 and some already use 3.5/devel. |
|
This PR doesn't touch score serialization at all, the only thing it does is allow users to use scorer function outside of SORT statement, e.g. in arbitrary calculations. That means, while performing a rolling upgrade, those coordinators which are outdated will simply decline unsupported queries even if all db server were already updated. A long story short - these changes are backwards compatible |
… feature/aql-blocks-by-line-executor-calculation * origin/feature/aql-blocks-by-line: Doc - Extend generation of program option tables (#7888) Bug fix/internal issue #316 (#7911) fix openssl 1.3 usage upgrade to boost 1.69.0 (#7910) Doc - Add a paragraph that JS JS trxs are excluded from intermediate commits. (#7919)
Please note that for legal reasons we require you to sign the
Contributor Agreement
before we can accept your pull requests.