feat(optimizer)!: Annotate parameterized numeric types for Snowflake functions

[fivetran-MichaelLee]

Add snowflake specific coercion logic when the arguments are parameterized numbers. This affects the following functions:

• NULLIF
• IFNULL
• IFF
• COALESCE
• GREATEST
• LEAST

The rules for coercing 2 parameterized numbers, type1 and type2, where the precision and scale of the first type is p1 and s2, and the precision and scale of the second type is p2 and s2 are:

• If p1 >= p2 AND s1 >= s2: return type1
• If p2 >= p1 AND s2 >= s1: return type2
• Otherwise: return DECIMAL(min(38, max(p1, p2) + |s2 - s1|), max(s1, s2))

When a function can take more than 2 arguments, the final type is calculated by a reduce function using the pair coercion rules stated above.

[georgesittas]

Let's check whether these rules apply to other operators as well, e.g., DECIMAL1 + DECIMAL2, etc. If that's the case, then it may be worth extending / overriding _maybe_coerce to incorporate them for Snowflake, so we don't have to do this for every expression separately.

[georgesittas]

Great work & research @fivetran-MichaelLee, this is super valuable.

After reviewing this PR, I feel hesitant merging it as-is, mainly because it's uncovered some existing inconsistencies / feature gaps and my hunch is that once we properly address those, then the current solution can be made much simpler.

I suggest pausing for now and thinking this through a bit more.

[georgesittas]

I'd be surprised if non-integers were syntactically/semantically valid. I think it's safe to assume that only integers can be used for the precision and scale parameters.

Additionally, numerics always have default precision and scale if missing, e.g., NUMBER is a synonym of NUMBER(38, 0) (and is parsed as such). So we could also do something like:

precision_value = precision.this.to_py() if isinstance(precision, exp.DataTypeParam) or 38
scale_value = scale.this.to_py() if isinstance(scale, exp.DataTypeParam) or 0

And so it should be possible to simplify this whole logic quite a bit.

[georgesittas]

We should override the Literal annotator for this:

# In typing/snowflake.py
def _annotate_literal(self, expression):
    expression = self._annotate_literal(expression)
    if expression.is_type(exp.DataType.Type.DECIMAL):
        precision, scale = _extract_literal_precision_scale(expression.this)
        expression.type.set("expressions", [exp.DataTypeParam(...), exp.DataTypeParam(...)])

    return expression

[georgesittas]

This is an interesting change. Indeed, according to the docs, integers are really NUMBER(38, 0).

I wonder if we should hook into _set_type for Snowflake and intercept integer type annotations in order to automatically change them to NUMBER(38, 0) and coercion works as expected... This feels like a much bigger change, though.

(sqlglot) ➜  sqlglot git:(main) snow sql -q "select system\$typeof(cast(1 as int) + cast(2 as number(5, 3)))"
select system$typeof(cast(1 as int) + cast(2 as number(5, 3)))
+---------------------------------------------------------+
| SYSTEM$TYPEOF(CAST(1 AS INT) + CAST(2 AS NUMBER(5, 3))) |
|---------------------------------------------------------|
| NUMBER(38,3)[SB2]                                       |
+---------------------------------------------------------+

[georgesittas]

This also exposes an inconsistency where at parse time we change NUMBER (or equivalent, e.g. DECIMAL) to NUMBER(38, 0), but we don't do it for these integer types...

[georgesittas]

We don't need to copy() here, this is taken care of in _set_type -> type.setter -> DataType.build (copies by default)

[georgesittas]

We have E in sqlglot._typing and can use that here instead.

[georgesittas]

This feels like something that should be handled by a COERCES_TO override for Snowflake, i.e. NUMBER always coerces to FLOAT:

(sqlglot) ➜  sqlglot git:(main) snow sql -q "select system\$typeof(coalesce(cast(1 as number(4, 2)), cast(1 as float)))"
select system$typeof(coalesce(cast(1 as number(4, 2)), cast(1 as float)))
+--------------------------------------------------------------------+
| SYSTEM$TYPEOF(COALESCE(CAST(1 AS NUMBER(4, 2)), CAST(1 AS FLOAT))) |
|--------------------------------------------------------------------|
| FLOAT[DOUBLE]                                                      |
+--------------------------------------------------------------------+

[georgesittas]

Discussed in Slack and concluded that we'll temporarily close this to avoid:

• Introducing new tech debt that may be unnecessary after we address some existing debt
• Destabilizing main with new coercions, since there's a non-zero risk associated with these and so needs to be done right