feat(duckdb): Add transpilation support for BOOLEAN and TEXT cases of TRY_CAST function#7681
feat(duckdb): Add transpilation support for BOOLEAN and TEXT cases of TRY_CAST function#7681fivetran-amrutabhimsenayachit wants to merge 1 commit into
Conversation
66c32b6 to
42ac8f4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
|
||
| class TryCast(Cast): | ||
| arg_types = {**Cast.arg_types, "requires_string": False} | ||
| arg_types = {**Cast.arg_types, "requires_string": False, "dialect_cast": False} |
There was a problem hiding this comment.
This isn't very descriptive. We should choose a name that reflects the semantics well. In this case, the behavior we're trying to represent with the new arg is: "do TRY_CASTs to text types (CHAR, VARCHAR, TEXT, etc) with a max length specifier result in NULL, when their argument exceeds said length?"
| if to_type == exp.DType.BOOLEAN: | ||
| return _to_boolean_sql(self, exp.ToBoolean(this=src, safe=True)) |
There was a problem hiding this comment.
This should not be handled in DuckDB's trycast_sql. We should instead convert (try-)casts to boolean in Snowflake into ToBoolean nodes, at parse time. From Snowflake's docs:
The semantics of
CASTare the same as the semantics of the correspondingTO_datatype conversion functions.
| src = expression.this | ||
| to = expression.to | ||
| to_type = to.this | ||
|
|
There was a problem hiding this comment.
Let's get rid of the first branch altogether, and assign the constituent args to variables only within the transpilation branch. We should fall back to the base trycast_sql only at the end of the method.
| if self.dialect.TRY_CAST_REQUIRES_STRING and (to := kwargs.get("to")): | ||
| kwargs["dialect_cast"] = to.this == exp.DataType.Type.BOOLEAN or ( | ||
| to.this in exp.DataType.TEXT_TYPES and bool(to.expressions) | ||
| ) |
There was a problem hiding this comment.
This isn't right, we should not conflate the two concepts. These properties are orthogonal to each other:
TRY_CASTin Snowflake only operates on string valuesTRY_CASTto text types with a length specifier result inNULLwhen the value exceeds said length
I would honestly refactor this whole section so that Snowflake overrides build_cast and sets these two kwargs after the fact, when the instance is a TryCast. The TRY_CAST_REQUIRES_STRING seems redundant right now, only Snowflake sets it.
42ac8f4 to
3d63802
Compare
… TRY_CAST function [CLAUDE] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3d63802 to
6bf87a6
Compare
PR#1
Refer #7672 for comments.
BOOLEAN: Snowflake's
TRY_CAST(x AS BOOLEAN)accepts'on'/'off'and returnsTRUE/FALSE, but DuckDB's nativeTRY_CAST(x AS BOOLEAN)returnsNULLfor those two values silently.TEXT(n): Snowflake's
TRY_CAST(x AS VARCHAR(n))returnsNULLif the string exceeds length n, but DuckDB's nativeTRY_CAST(x AS VARCHAR(n))silently ignores the length constraint and returns the full string.After fixing the above,
Boolean:
Text: