Regarding CI failures:

• diesel-tests fails due to some unrelated issue with IpNetwork.
• cargo fmt fails because the CI uses nightly rustc 1.88.0-nightly (25cdf1f67 2025-04-28), while I use the latest stable (1.86.0) that doesn't format doctests. I'll reformat on nightly

thank you. although I have to say the original intention of not accepting strs is to prevent mistyping, and to encourage you to use Derive(Iden) on structs / enums. I get that you may have a different style of codebase.
May be we should only accept &'static str?
I'd like to put this under a feature flag, wdyt?

the original intention of not accepting strs is to prevent mistyping, and to encourage you to use Derive(Iden) on structs / enums. I get that you may have a different style of codebase.

I understand your sentiment, and I try to use Iden types for table and column names in my queries. But my queries (as well as sea_query codebase, as seen in the diff) still have plenty of and-hoc table/column aliases, or stored procedure calls, or casts between standard SQL types (for which you don't provide a Rust Iden representation, as far as I know). And you already have Alias to support these use cases, anyway. I don't see the point in making it artificially verbose. If anything, verbosity results in inattentive reading and MORE typos / bugs.

May be we should only accept &'static str?

That's already the case in methods with IntoIden parameters, like cast_as / expr_as / join_as (these motivated me the most). IntoIden implies 'static because of the bound on the blanket impl<T> IntoIden for T where T: Iden + 'static. So, Iden is implemented for any &str (makes sense to me), but IntoIden is implemented only for &'static str.

If that's a hard requirement for merging, I could remove the impls for String and &String. But I don't see the point. Alias is the same thing, just more verbose. See my verbosity argument above.

I'd like to put this under a feature flag, wdyt?

If that's a hard requirement for merging, I could do that. That would still solve my personal issue in my codebase. However, I don't see the point in keeping sea_query more verbose by default*. It has many similar hurdles where the obvious code doesn't compile, and then you need to waste time and manually figure out the right wrappers / type conversions. I try to resolve these papercuts little by little and make the crate better for everyone.

*That is, if you mean a non-default feature flag, where users don't get the impls by default. A default feature flag (with an option to manually disable it and disallow string identifiers) would be better, but I don't see much benefit in that either. I doubt that many people would find it useful and manually disable the impls.

On a side note, I'd like to come to an understanding and document this area:

I'm kinda suspicious of Iden::unquoted that looks like Display::fmt, and Iden::to_string that obviously duplicates ToString::to_string (which is exactly what we would get by requiring Iden: Display!). But I'm not sure whether the unquoted string is semantically the same as Display and whether it could be replaced by it.

I'm kinda suspicious of Iden::unquoted that looks like Display::fmt

yes, but I don't think it should be replaced. one could have a struct that impl Display differently from Iden. In some of my old code bases, I'd impl Iden manually on a struct MyTable(i32)

but IntoIden is implemented only for &'static str

good to know

If that's a hard requirement for merging, I could remove the impls for String and &String. But I don't see the point. Alias is the same thing, just more verbose. See my verbosity argument above.

how about pub-ing the internal field, i.e. struct Alias(pub String), so we can do Alias(my_string)?

my thinking: we remove String and &String for now, then feature flag not needed

my thinking: we remove String and &String for now, then feature flag not needed

Ok, removed these. My most important use case is static string literals, so that's ok for me. The PR should now be mergeable for you too.

how about pub-ing the internal field, i.e. struct Alias(pub String), so we can do Alias(my_string)?

I've decided not to do this for now, because this doesn't seem very beneficial. This constructor accepts a concrete String, so it's only beneficial for Strings. But most of the time (confirmed by the prior usages of Alias::new in sea_query), the calling code passes static string literals. Unlike Alias::new("str"), Alias("str") doesn't compile and still requires something like Alias("str".into()).

Updated changelog

• Implemented Iden for &str. Now you don't need to wrap string literals into Alias::new where T: IntoIden is needed.

yes, but I don't think it should be replaced. one could have a struct that impl Display differently from Iden. In some of my old code bases, I'd impl Iden manually on a struct MyTable(i32)

Thanks for the info, I added docs about this decision to the code.

🎉 Released In 0.32.5 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

I've removed some Alias::new from sea-orm as well

Cool. I too had a note to myself to do that after this change is released

we remove String and &String for now, then feature flag not needed

Implement Iden for String can be still worthwide because the current impl for &str must capture a reference so that it can't bring the ownership.

Alias can work as a workaround but the name indicates a different semantic so perhaps a direct impl Iden for String is still valuable.

Days before, I made:

#[derive(Debug, Clone)]
pub struct IdenStr<'a>(pub &'a str);

impl Iden for IdenStr<'_> {
    fn unquoted(&self, s: &mut dyn Write) {
        write!(s, "{}", self.0).unwrap();
    }
}

#[derive(Debug, Clone)]
pub struct IdenString(pub String);

impl Iden for IdenString {
    fn unquoted(&self, s: &mut dyn Write) {
        write!(s, "{}", self.0).unwrap();
    }
}

Now I can try to gradually remove IdenStr, but IdenString is still required for the reason above.

cc @tyt2y3 @Expurple

@tisonkun I agree with you and impl Iden for String makes sense to me too. I simply decided to not spend time defending it, and instead quickly merge the MVP that solves most of my issues. I'll happily open another PR for String if the maintainers change their mind

Related: Why doesn't Iden impl From<String>, but it does From<&str>?