sql: support USING on implicitly coercible data types

[sploiselle]

Fixes MaterializeInc/database-issues#1629

---

This change is 

[justinj]

LGTM, do you have a source that the logic for coerce_homogenous_exprs is indeed what's needed here? Mostly just curious, seems right!

[justinj]

Is there a reason this needs to be tested in testdrive as opposed to the sqllogictests?

[benesch]

Agreed that an SLT here would be best!

[justinj]

nit: for consistency s/r/rhs/?

[benesch]

Code looks good, but I think a small reordering (of stuff that existed before) could improve clarity. 1s!

[benesch]

Agreed that an SLT here would be best!

[sploiselle]

@justinj really, really good q re: coerce_homogeneous_exprs––using this function lets us avoid going all the way through the function selection machinery (func::resolve_func, func::select_impl –– which is definitely an option) bc we know we can directly invoke the functions we want with homogeneously typed exprs. That we want to use these same homogeneously typed exprs twice then makes it really decisive that this is the better choice.

As for a source, the logic is that for the join to work, the two expressions must be of the same type, as evidenced by the fact that BinaryFunc::Eq and VariadicFunc::Coalesce both require homogeneously typed exprs (though I realize that this makes my argument circular ¯\(ツ)/¯), and this is kind of implicit cast is allowed bc PG does it.

Not sure if I under-, over-, or incorrect-dimensionally-shot your question, but please lmk!

[benesch]

Ok, what do you think about restructuring the body of the loop in plan_using_constraint so it looks like this:

        let (lhs, _) = left_scope.resolve_column(column_name)?;
        let (mut rhs, _) = right_scope.resolve_column(column_name)?;
        if lhs.level != 0 || rhs.level != 0 {
            bail!(
                "Internal error: name {} in USING resolved to outer column",
                column_name
            )
        }

        // The new column can be named using aliases from either the right or
        // left.
        let mut names = left_scope.items[lhs.column].names.clone();
        names.extend(right_scope.items[rhs.column].names.clone());

        // Adjust the RHS reference to its post-join location.
        rhs.column += left_scope.len();

        // Join keys must be resolved to same type.
        let mut exprs = coerce_homogeneous_exprs(
            &format!("NATURAL/USING join column {}", column_name),
            &ecx,
            vec![
                CoercibleScalarExpr::Coerced(ScalarExpr::Column(lhs)),
                CoercibleScalarExpr::Coerced(ScalarExpr::Column(rhs)),
            ],
            None,
        )?;
        let (expr1, expr2) = (exprs.remove(0), exprs.remove(0));

        join_exprs.push(ScalarExpr::CallBinary {
            func: BinaryFunc::Eq,
            expr1: Box::new(expr1.clone()),
            expr2: Box::new(expr2.clone()),
        });
        map_exprs.push(ScalarExpr::CallVariadic {
            func: VariadicFunc::Coalesce,
            exprs: vec![expr1, expr2],
        });
        new_items.push(ScopeItem {
            names,
            expr: None,
            nameable: true,
        });
        dropped_columns.insert(lhs.column);
        dropped_columns.insert(rhs.column);

The idea is to do the thing that requires the unadjusted RHS column, adjust the RHS column, and then do all the things that require the adjusted RHS column.

[sploiselle]

@benesch Nice 🙇🙇🙇

[benesch]

Looks swell to me!