malloydata / malloy

Malloy is an experimental language for describing data relationships and transformations.
http://www.malloydata.dev
MIT License
1.99k stars 76 forks source link

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

Open lloydtabb opened 1 year ago

lloydtabb commented 1 year ago

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

lloydtabb commented 1 year ago

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 commented 1 year ago

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

lloydtabb commented 1 year ago

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 commented 1 year ago

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

lloydtabb commented 1 year ago

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).

carlineng commented 1 year ago

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

christopherswenson commented 1 year ago

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.

mtoy-googly-moogly commented 1 year ago

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 commented 1 year ago

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?

christopherswenson commented 1 year ago

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

lloydtabb commented 1 year ago

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)