Allow qualified type names in cast_as_quoted

[Expurple]

As discussed here: #827 (comment)

PR Info

• Closes Incorrect Quoting of Schema-Qualified Enums in cast_as_quoted #827

• Dependencies:

  • Remove unnecessary 'static bounds #921
  • Refactor conversions around qualified identifiers #953

Changes

The PR has been updated. The up-to-date description is in the changelog in the diff.

---

Original, outdated note:

Merge after #921. Marking as draft until then. The first commit comes as a dependency from that PR. If you want to review the diff before merging that PR, review only the second commit.

[Expurple]

Hold up. Why did I rename the parameter to col? It's a type name! ColumnRef doesn't make sense here. Asterisk and TableAsterisk aren't valid type names! That's exactly what I was talking about here: #795 (reply in thread).

I need to implement and merge MaybeQualified, and then use that here instead of ColumnRef.

Don't merge this PR as is.

[tyt2y3]

I see your point. it works but is kind of wrong in a type system sense. also I think this parameter can appear in other places as well, e.g. ALTER TYPE, so may be worth to have a TypeRef or something similar

[Expurple]

Rebased on top of #953 as an experiment. The first commit comes from that branch. The second commit is the actual content of this PR.

The cast_as_quoted API finally seems right. But there are still many things that I don't like:

1. I'd like to get rid of Expr::TypeName altogether. AFAIK, a type name is not a valid expression. We should model the SQL grammar more faithfully.

2. I'm not sure what's the deal with Expr::AsEnum. Why is it specifically about enums? This needs to be documented.

3. While browsing AsEnum references, I found this code in the Postgres backend:

   fn prepare_simple_expr(&self, simple_expr: &Expr, sql: &mut dyn SqlWriter) {
       match simple_expr {
           Expr::AsEnum(type_name, expr) => {
               write!(sql, "CAST(").unwrap();
               self.prepare_simple_expr_common(expr, sql);
               let q = self.quote();
               let type_name = type_name.to_string();
               let (ty, sfx) = if type_name.ends_with("[]") {
                   (&type_name[..type_name.len() - 2], "[]")
               } else {
                   (type_name.as_str(), "")
               };
               write!(sql, " AS {}{}{}{})", q.left(), ty, q.right(), sfx).unwrap();
           }
           _ => QueryBuilder::prepare_simple_expr_common(self, simple_expr, sql),
       }
   }

   type_name here is a DynIden, but the code also handles [] as part of the type name. Valid identifiers don't include []! If we want to support that, we need to model TypeName as something richer than (Option<SchemaName>, DynIden) (which it is in this version of the PR).

   And after we do that, we may not be able to reuse MaybeQualifiedTwice from Refactor conversions around qualified identifiers #953. So I put that on hold until we figure this out

@tyt2y3 @Huliiiiii

[Expurple]

I've just googled type names in Posgtres. Boy, is it complicated even without []. They can also span several words and have (size) / (precision): https://www.postgresql.org/docs/current/datatype.html#DATATYPE-TABLE

I think, we need to agree what exactly a DynIden is and what it can and can not contain. Based on the naming, I'd prefer the clarity of it being just a single identifier, and type names being a separate, more complex type

[Huliiiiii]

I think, we need to agree what exactly a DynIden is and what it can and can not contain. Based on the naming, I'd prefer the clarity of it being just a single identifier, and type names being a separate, more complex type

I agree with this.

Expr::SubQuery, Expr::AsEnum, and Expr::Case were all added as workarounds to support certain kinds of expressions that were previously unsupported, without breaking compatibility.

As I mentioned here: #926
There are many SQL expressions with unique structures, we need a more general representation to support them properly.

[tyt2y3]

Yeah it's complicated I don't think a universal solution is easy. I would just stick with the low hanging fruit for now, which basically is the draft PR you have.

for context, AsEnum was added to support ActiveEnum in SeaORM, and the problem was:

1. we can't quote primitive type names "integer"
2. we have to quote custom types (including enum) "my_enum"
3. however if we want to cast to my_enum[] the correct syntax is "my_enum"[]

this explains the special handling above. so it's not just enums, but any custom types. SeaORM currently only supports enums. https://github.com/SeaQL/sea-query/blob/master/src/expr.rs#L189

[Expurple]

Ok, then I'll merge these drafts "as is", to close #827 (being able to qualify type names and fixing cast_as_quoted signature specifically).

Then I'll create a separate issue for this discussion about further redesign of TypeName and Expr.

I'll also document AsEnum when I have time.