malloy icon indicating copy to clipboard operation
malloy copied to clipboard

DuckDB /Postgres Dialects should use TRY_CAST for the Malloy :: operator.

Open lloydtabb opened this issue 1 year ago • 11 comments

:: means safe_cast in BigQuery, It should mean TRY_CAST in the other dialects.

lloydtabb avatar Jun 18 '23 20:06 lloydtabb

Looks like there is no 'TRY_CAST' for Postgres. There are some ugly workarounds... I'm going to fix for duckdb and leave postgres for now...

https://stackoverflow.com/questions/10306830/postgres-define-a-default-value-for-cast-failures

lloydtabb avatar Jun 19 '23 15:06 lloydtabb

Love to have your thoughts on this.. This is one of those lowest common denominator problems...

lloydtabb avatar Jun 19 '23 15:06 lloydtabb

I actually really love :: for cast perhaps a separate safe case operater :::

project: 
  num is num::number  // generates CAST(num AS float64) as num
  safe_num is num:::number // generates SAFE_CAST(num AS float64) as num

and posgres doen't support the ::: operator

I also really like the :: syntax for native types and it should work for both syntaxes.

project: native_safe_int is foo:::'int64' project: native_int is foo::'int64'

lloydtabb avatar Jun 19 '23 15:06 lloydtabb

@mtoy-googly-moogly ^^ what do you think?

lloydtabb avatar Jun 19 '23 23:06 lloydtabb

BTW, generally safe_cast is used during the ETL process when cleaning up data. You really want to use normal cast when doing table to table stuff. SAFE_CAST is the exception, generally (so much so that Postgres refuses to implement it on principle).

lloydtabb avatar Jun 19 '23 23:06 lloydtabb

::: as "safe_cast" makes sense to me. I think it's a common enough pattern that it deserves its own operator.

carlineng avatar Jun 20 '23 16:06 carlineng

Currently cast(x as type) is unsafe cast and x::type is safe cast, so it would be a breaking change to make x::type unsafe cast.

That said, I so kinda like ::: as safe.

christopherswenson avatar Jun 20 '23 17:06 christopherswenson

Chris is correct, malloy has two cast gestures ... :: and cast and :: generates a safe cast on bigquery. I think the change which should go in immediately is to use try_cast on :: in duckdb. There is a field in the dialect fragment passed to sqlCast which you can query in the dialect to know if you should generate a safe or unsafe cast.

We should have a group chat about ::: vs. cast(x as

mtoy-googly-moogly avatar Jun 20 '23 18:06 mtoy-googly-moogly

I think the way it is now is that :: is cast and ::: is safe cast, for all dialects. I'll let @christopherswenson comment, but I think we should close this?

mtoy-googly-moogly avatar Sep 21 '23 20:09 mtoy-googly-moogly

I think this needs a @lloydtabb look to see if he thinks it's correct. I think it's correct now?

christopherswenson avatar Sep 21 '23 20:09 christopherswenson

I think the way it is now is that :: is cast and ::: is safe cast, for all dialects. I'll let @christopherswenson comment, but I think we should close this?

Yes, close. This is correct on dialects that support safe cast (postgres doesn't)

lloydtabb avatar Sep 21 '23 21:09 lloydtabb