Allow aliases to IdentifierExpressions. #8
Conversation
This allows me to write goqu.I("col").As(goqu.I("other")) as well as the
original goqu.I("col").As("other").
| // Output: | ||
| // SELECT "a" AS "as_a" FROM "test" | ||
| // SELECT COUNT(*) AS "count" FROM "test" | ||
| // SELECT sum(amount) AS "total_amount" FROM "test" | ||
| // SELECT "test"."a" AS "test"."b" FROM "test" |
There was a problem hiding this comment.
I don't think this is valid sql, I tried running the example on postgres and mysql and received syntax errors.
Can you provide an example that would work on all databases like.
db.From("test").Select(goqu.I("test.a").As(goqu.I("b")))There was a problem hiding this comment.
You are correct. Maybe I should change the implementation to pull the column name from IdentifierExpression to ensure there isn't a table or schema thrown in.
|
Thanks for the PR! Only other comment would be can you add a test case in here https://github.com/doug-martin/goqu/blob/master/dataset_test.go#L356 just to ensure that the serialization is happening properly. Also the panic I am kinda torn on right now, Im not sure it has to change but wondering if you had any thoughts... We could do that one of two ways the way you did it which would ensure that the issue was found when actually used in the code...or we could defer the error until the time that the sql is generated and verify the type there this would prevent a panic, but could lead to an error that is far from the original source. What are your thoughts. |
|
I kind of like the panic for now. It is very similar to how cols() works and immediately triggers an error. Given that so many of the functions take interface{}, it's hard to enforce type safety. And like you said, it defers the error far from the original source. |
Aliasing from table.column to anything with a period is not allowed (at least not in mysql). Provide a better example that works across databases. Add a test to TestLiteralAliasedExpression that uses an IdentifierExpression.
|
I updated the example and added a small test case to dataset_test. I left the implementation of aliased as is since the current behavior of I("column").As("other.column") produces the same behavior. |
Allow aliases to IdentifierExpressions.
This allows goqu.I("col").As(goqu.I("other")) as well as the original goqu.I("col").As("other").