New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(spanner/spansql): add support for CAST functions #5038
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this. The parsing looks good to me, but this does not seem to add any actual implementation of the cast function itself. It also seems to circumvent the 'normal' function parsing and evaluation. See https://github.com/googleapis/google-cloud-go/pull/4670/files for an example on how functions are normally supported.
I know that we have some constructs in spansql that only parses the construct, but does not do anything with them, but that is mostly related to DDL. Having a query construct that is parsed but not evaluated is less useful, especially in this case as the type of the returned value should be different.
Would you mind taking a look at that?
@@ -2796,6 +2832,14 @@ func (p *parser) parseLit() (Expr, *parseError) { | |||
return Paren{Expr: e}, nil | |||
} | |||
|
|||
// Handle type conversion functions. | |||
if tok.caseEqual("CAST") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we should handle CAST
functions differently from 'normal' functions (see below). I understand that the expression inside the CAST
function is a little different from many other expressions, so that might need some custom handling, but this implementation seems to circumvent the part where the function is evaluated. From what I can tell, that means that you can specify a CAST
in a query, but it cannot be evaluated. Or am I missing something here?
Thank you for reviewing. The example is following DDL: CREATE TABLE users (
user_id STRING(36) NOT NULL,
some_string STRING(16) NOT NULL,
number_key INT64 AS (SAFE_CAST(some_string AS INT64)) STORED, -- `some_string` may wrapped in SUBSTR or some string functions
) PRIMARY KEY(user_id); At this case, I don't need that the cast functions are actually evaluated, only I want to managing the DDL schema. |
@yokoyama10 I've added an alternative here that implements the CAST and SAFE_CAST functions as actual functions instead of a special kind of literal. Would you mind taking a look at that if that would also satisfy your requirement? |
Thank you for your work #5057 and it is very applicable to our requirements! |
The changes make the spansql library possible to parse type conversion functions (
CAST
andSAFE_CAST
).https://cloud.google.com/spanner/docs/conversion_functions
CastFunc
struct for handling these functions.noParam
toparseType
function, because parameterized typename (e.g.STRING(MAX)
) is not supported in cast function.