`nextval` generates incorrect SQL for sequences with an explicit schema in rel8 1.4.1.0

[elldritch]

I believe this is a regression introduced by #217.

In rel8 1.4.0.0, calling nextval "foo.bar" to get the next value of a sequence bar in a schema foo used to generate the SQL nextval(CAST(E'foo.bar' AS text)), which is nextval('foo.bar').

However, in 1.4.1.0, it now generates nextval(CAST(quote_ident(CAST(E'foo.bar' AS text)) AS text)), which is nextval(quote_ident('foo.bar')) which is nextval('"foo.bar"'). This SQL is incorrect, and causes an error relation "foo.bar" does not exist because it's trying to access the sequence named "foo.bar" in the current schema, when in reality I want the sequence bar in schema foo.

[ocharles]

Thanks, I think we're going to need to change the type of nextval to something like Either QualifiedName Name in order to fix this (though I think we'd introduce a specific sum-type here, rather than using Either). @shane-circuithub any thoughts?

[elldritch]

Yeah this might be a big breaking change but I'd love to have arguments typed something like

data RelationName = RelationName {
  schema :: Maybe Text,
  name :: Text
}

instance IsString RelationName

wherever we have functions where getting a name argument wrong can cause an ERROR: relation _ does not exist.

[shane-circuithub]

Sorry, missed this. Yeah I agree with @elldritch, some sort of type like that would be best. TableSchema could use it too, so that TableSchema becomes just RelationName and the columns.

[chris-martin]

This caused an outage for me :grimacing:

[ocharles]

@chris-martin Ack, as in upgrading Rel8 and getting #217 caused the outage?

[chris-martin]

Yeah, I upgraded 1.4.0 to 1.4.1, and I had one insert involving nextVal that stopped working.

In retrospect I guess I should have had golden tests on the statement ByteString that would have forced me to look over the changes to the statement when upgrading. Not sure I would have noticed the problem even if I'd seen it, though, because I didn't know that the SQL quoting worked this way.

[chris-martin]

In case anyone else needs it, my quick workaround was to change

Rel8.unsafeCastExpr (Rel8.nextval "foo.bar")

to

Rel8.unsafeLiteral "nextval('foo.bar')"