Add support for (un)signed int, bigint, tinyint, double and real

[alexsnaps]

I've had a usecase where I needed support for unsigned integers. So I've added this. If there is something you'd want me to do differently or whatever, let me know.

[alexsnaps]

wdyt?

[ms705]

Thanks! This is very useful :+1:

I do wonder if we want to use a separate variant of the SqlType enum for the unsigned and signed types. This has two advantages:

1. it avoids ambiguous booleans in the code; and
2. it produces more helpful errors for dependent code that doesn't cover the new types yet (unhandled enum variant vs. changed type definition).

Does SQL have signed/unsigned reals and doubles? I thought doubles (in the IEEE 754 sense) are always signed.

[ms705]

Looking good! The only thing I would change (beyond the literal point raised above) is that Some(sign) if sign.len() == 8 seems like a brittle way to check if sign is the string "signed" or "unsigned". A better way might be something along the lines of:

match ... {
    Some(ref signedness) => {
         let signedness = String::from_utf8(signedness).unwrap();
         if signedness == "unsigned" {
             SqlType::UnsignedInt(len.map(|l|len_as_u16(l)).unwrap_or(32))
         } else if signedness == "signed" {
             SqlType::Int(len.map(|l|len_as_u16(l)).unwrap_or(32))
         } else {
             unreachable!()
         }
     }
}

[ms705]

@alexsnaps I'd love to get this merged. What do you think about the proposed change above?

[alexsnaps]

@alexsnaps I'd love to get this merged. What do you think about the proposed change above?

Yeah, sorry 🤦‍♂ This thing called life came in between and I… lost track of this. Let me recap:

• [x] Add Literal::UnsignedInteger(u64) and use the sign of literal to use unsigned or signed (I'll look what this the also means for operators et al - thinking out loud)
• [ ] I can add the explicit testing of the signedness… makes me somewhat sad, as I expect nom to keep the ref to the original string, so … I need to redo a case insensitive comparison again, while nom already took care of that for us… Not a huge horrendous duplicate. But that's why I went for length.

I'll do my best to wrap this up this week! Does that sound good?

[ms705]

Great, sounds good! Let me know if I can help.

[alexsnaps]

@ms705 First, sorry for the long delay again. But I think I implemented what we had discussed. Is that more inline with what you had in mind?

[ms705]

Yes, thank you! Looking great.