expr,sql: isolate string casting functions by benesch · Pull Request #1105 · MaterializeInc/materialize

benesch · 2019-11-27T03:02:03Z

Isolate functions that cast to strings, rather than having a universal
datum -> string cast function. In general, expr should not be performing
any type switches, as the type switches can all be preselected by the
SQL planner.

This commit also introduces casting for bytes <-> string and decimal ->
string, which were left out of the initial implementation as their
implementation is somewhat challenging.

Isolate functions that cast to strings, rather than having a universal datum -> string cast function. In general, expr should not be performing any type switches, as the type switches can all be preselected by the SQL planner. This commit also introduces casting for bytes <-> string and decimal -> string, which were left out of the initial implementation as their implementation is somewhat challenging.

Checking for null in every single SQL function implementation gets old really fast. Hoist the null checking into the expression evaluator, and add a special case for the presently one function that needs to do something special with nulls (coalesce). (There will be a handful more such special functions in the future, but 99% of SQL functions will always just propagate nulls through.)

For consistency with other functions, unwrap to assert that the types are as expected. The SQL planner guarantees that they shall be so.

quodlibetor

lgtm! I didn't even think about what propagates_nulls() check for cast(NULL TO TEXT) should do, this seems clearly more correct.

benesch · 2019-11-27T18:57:46Z

I didn't even think about what propagates_nulls() check for cast(NULL TO TEXT) should do, this seems clearly more correct.

Hah, I made exactly the same mistake when I initially split things up, and introduced a CastNullToString function. It wasn't until I got an unreachable pattern error because of

materialize/src/sql/query.rs

Line 2513 in 09cc712

(Null, _) => {

that I noticed the mistake!

@kurtmckee

benesch requested a review from quodlibetor November 27, 2019 03:02

benesch force-pushed the func-no-branch branch from 2be8214 to 8e5a6cd Compare November 27, 2019 03:53

benesch force-pushed the func-no-branch branch from 8e5a6cd to cbff25f Compare November 27, 2019 15:37

benesch added 2 commits November 27, 2019 11:19

expr: unwrap in to_char implementation

43c2bc8

For consistency with other functions, unwrap to assert that the types are as expected. The SQL planner guarantees that they shall be so.

benesch force-pushed the func-no-branch branch from cbff25f to 43c2bc8 Compare November 27, 2019 16:19

quodlibetor approved these changes Nov 27, 2019

View reviewed changes

benesch merged commit ee4790d into MaterializeInc:master Nov 27, 2019

benesch deleted the func-no-branch branch November 27, 2019 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expr,sql: isolate string casting functions#1105

expr,sql: isolate string casting functions#1105
benesch merged 3 commits intoMaterializeInc:masterfrom
benesch:func-no-branch

benesch commented Nov 27, 2019

Uh oh!

quodlibetor left a comment

Uh oh!

benesch commented Nov 27, 2019 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

benesch commented Nov 27, 2019

Uh oh!

quodlibetor left a comment

Choose a reason for hiding this comment

Uh oh!

benesch commented Nov 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

benesch commented Nov 27, 2019 •

edited

Loading