Why these conditions can’t be compatible?
What is the difference between SQL_FUNC_NEEDCOLL
and new flag? Does NEEDCOLL refer to returning value?
Please, comment this part.

These conditions can’t be compatible because
SQL_FUNC_NEEDCOLL and SQL_FUNC_ARG_COLL ask for
different actions. SQL_FUNC_NEEDCOLL asks
to find first argument with collation in
the arguments expression list (from left
to right) and use it inside function
as a collation of the whole expression.
SQL_FUNC_ARG_COLL asks to check if
arguments collations are compatible
and use the right one if they are.
I would add comment to the code.

If we want to establish proper collation
handling in all functions, we should
probably use only one flag to do check
and set the right collation.

I would merge these flags into one, OK?

bool unused;
uint32_t id;
if (sql_expr_coll(pParse,
@@ -4064,6 +4065,37 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
return 0;
}
}
+ if (pDef->funcFlags & SQL_FUNC_ARG_COLL) {
+ assert(nFarg == 2);

Add comment to this assertion or better to the whole branch.
It is not obvious at first sight. Why only functions with two args…
For instance, there’s MAX function, which may take any number
of arguments. Collations are also vital for this function, since
they are used to compare strings.

What is more, I suggest to move this part in a separate patch,
since position is not the only function that must apply collations
on its operands. I don’t insist on dozens of tests on each built-in
function, just a few.

So do you propose to make two patches: first with adding proper
collation checking/setting in sqlExprCodeTarget() and another
with adding proper collation usage in functions in func.c?
Is it OK if I put them in one patch set?

+ struct coll *colls[2] = {NULL};

I’d rather use explicit naming:
struct coll *lhs_coll = NULL;
struct coll *rhs_coll = NULL;

It makes code a bit longer, but on the other hand IMHO
makes it less prone to typos.

But this function sqlExprCodeTarget() is called for expression
list (of arguments). If we would use it for functions like MAX
(as we want to) then I think an array should be used. Or just
two loops to check every pair of arguments collations for
compatibility.

+ uint32_t colls_ids[2] = {0};
+ bool is_forced[2] = {false};
+ if (sql_expr_coll(pParse, pFarg->a[0].pExpr,
+   &is_forced[0], &colls_ids[0],
+   &colls[0]) != 0)
+ return 0;
+ if (sql_expr_coll(pParse, pFarg->a[1].pExpr,
+   &is_forced[1], &colls_ids[1],
+   &colls[1]) != 0)
+ return 0;
+
+ uint32_t coll_id;
+ if (collations_check_compatibility(colls_ids[0],
+    is_forced[0],
+    colls_ids[1],
+    is_forced[1],
+    &coll_id)
+    != 0) {
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
+ return 0;
+ }
+
+ coll = (coll_id == colls_ids[0]) ? colls[0] :
+    colls[1];
+ if (coll == NULL)
+ coll = coll_by_id(COLL_NONE)->coll;

Why do we need this?

Because if there is no collation at all among
arguments, then the context would not have any
collation as well. And sqlGetFuncCollSeq()
would fail with assertion in this case, because
it always expects some collations being set in context.