sqlparser: support expression type hint #482
Conversation
Yaiba
force-pushed
the
feature/sql-expr-type-hint
branch
from
336aaf6 to
df96076
Compare
parse/sql/tree/sql-writer/writer.go
Outdated
| @@ -38,6 +44,11 @@ func (s *SqlWriter) String() string { | |||
| s.stmt.WriteString(")") | |||
| } | |||
|
|
|||
| if s.typeHint != "" { | |||
| s.stmt.WriteString("::") | |||
| s.stmt.WriteString(s.typeHint) | |||
There was a problem hiding this comment.
This won't work, because our types aren't named the same as Postgres's. Also, I don't think the writer package is the right place to do this. It seems like it should individually be written in each expression's ToSQL().
There was a problem hiding this comment.
I don't think the writer package is the right place to do this
Yeah you're right, since only expression has typeHint.
There was a problem hiding this comment.
because our types aren't named the same as Postgres's
For now I have no checks on typeHint, so you can pass Postgres's type
There was a problem hiding this comment.
Hmm, shouldn't we expect Kwil schema types and translate them to PG types when writing the statement out from the AST (or when generating the AST from the user code)?
There was a problem hiding this comment.
Oh, but we only have int and string, which is pretty constraining...
There was a problem hiding this comment.
yeah we need a supported type list, then translate it when ToSQL.
I think it's time to extend our supported column types?
There was a problem hiding this comment.
I'm not sure. Safest may be to use just the two Kwil types, and then we map those to either TEXT or INT8 in the type cast.
Seems to me that the main purpose of this (at present anyway) is to allow the author to assert type to disambiguate between integer or string, also to reinterpret type as needed for downstream instructions within an action. Maybe we add the different integer types later, with boolean and possibly our own 32-byte integer type when we are ready and have thought through the implications.
Yaiba
force-pushed
the
feature/sql-expr-type-hint
branch
from
df96076 to
39af5b0
Compare
Yaiba
force-pushed
the
feature/sql-expr-type-hint
branch
from
39af5b0 to
5c3b092
Compare
Yaiba
changed the title
| @@ -93,7 +93,7 @@ require ( | |||
| github.com/jmhodges/levigo v1.0.0 // indirect | |||
| github.com/klauspost/compress v1.17.0 // indirect | |||
| github.com/kwilteam/action-grammar-go v0.0.1-0.20230926160920-472768e1186c // indirect | |||
| github.com/kwilteam/sql-grammar-go v0.0.3-0.20230925230724-00685e1bac32 // indirect | |||
| github.com/kwilteam/sql-grammar-go v0.0.3-0.20240202213606-3f118696cec5 // indirect | |||
There was a problem hiding this comment.
Since 3f118696cec5 isn't on main of that repo, we should get kwilteam/sql-grammar#3 and then kwilteam/sql-grammar-go#3 before this
There was a problem hiding this comment.
Yeah, theses refs will break when those PRs merge and the feature branches are deleted
* sqlparser: support expression type hint * handle typeHint only in Expression * rename to TypeCast, with predifiend types
* sqlparser: support expression type hint * handle typeHint only in Expression * rename to TypeCast, with predifiend types
* sqlparser: support expression type hint * handle typeHint only in Expression * rename to TypeCast, with predifiend types
* sqlparser: support expression type hint * handle typeHint only in Expression * rename to TypeCast, with predifiend types
* sqlparser: support expression type hint * handle typeHint only in Expression * rename to TypeCast, with predifiend types
* sqlparser: support expression type hint * handle typeHint only in Expression * rename to TypeCast, with predifiend types
Support type hint in sql expression, so that could be used to cast/infer type of an expression.
Description
Related Issue
Following prs should be merged first, then update go.mod accordingly.
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist:
Checklist Explanation:
How to Review this PR:
sql syntax changes are in kwilteam/sql-grammar#3
changes made to sqlparser are mostly just set
TypeHintto certaintree.ExpressionXXAdditional Information:
'type hint' works as below:
This logic is also enforced in the sqlparser.
TODO:
TypeHinttype?TypeHintvalue