Choose better types for the ci_sessions table

[pstef]

Description
Choose better types for the ci_sessions table:

• for Postgres, use type BYTEA for binary data. Compared to storing base64-encoded text, this takes less space
• update the user guide to suggest using type INET for storing IP addresses in Postgres. This type offers input error checking and specialized operators and functions
• for both MySQL and Postgres, use type timestamp instead of integer. This type is easier to work with when analyzing session activity.

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[lonnieezell]

Unfortunately this solution is not scalable. While I applaud the desire for better performing tables, in this case all of the checks are either Postgre or not Postrge, which is semi-workable now since we only have 3 drivers, but there will be more drivers down the line that get ported over.

[pstef]

A single DatabaseHandler is not "scalable", but is currently the accepted solution anyway.

Usually you go from "semi-workable" to "scalable" by refactoring; this could be redone when there are too many drivers to keep the checks. Worrying about it now seems premature.

[lonnieezell]

Looking back at this one and my rejection looks like it was premature. We already had postgre-specific checks in the code so I'll re-open this now.

My main concern now is about the BC-breaking of changing to timestamp column type. If you can roll that part back to integers than I'm happy to include the other changes now.

[pstef]

4b1774a requires one to change the type of column ci_sessions.data whenever its creation predates whatever CI version that would include this change first. I admit I haven't thought about it until now, but it feels like that would have been OK for a 4.0.0 release (or any other release expected to break compatibility).

Currently I can't see 529b5ad breaking anything except applications that already exist and expect integers coming from that particular column. Again, I assumed that this change would be included in a release allowed to make such changes.

dd8a920 has the potential of breaking backwards compatibility if an application has to deal with nonconforming addresses - the new behavior of Postgres rejecting those could be surprising, but perhaps healthier in the long run. An attempt to convert an existing column would result either in a failure or success - I mean, it's not something that would silently cause damage until you decide to look at it.

Please feel free to make adjustments or cherry-pick any of these changes.

[lonnieezell]

@michalsn @MGatner @paulbalandan Which one of you knows Postgres enough to validate these changes?

[MGatner]

Sadly I know no Postgres.

[paulbalandan]

Postgres is Greek to me. 😂 Maybe @samsonasik ?

[samsonasik]

I just remember \x prefix is valid for hex in postgres 👍