Support for quoted identifiers

Author: matejcerny

modules/core/shared/src/main/scala-2/syntax/StringContextOps.scala

@@ -19,6 +19,9 @@ class StringContextOps private[skunk](sc: StringContext) {
   def id(): Identifier =
     macro StringContextOps.StringOpsMacros.identifier_impl
 
+  def qid(): Identifier =
+    macro StringContextOps.StringOpsMacros.quotedIdentifier_impl
+
   /** Construct a constant `Fragment` with no interpolated values. */
   def const()(implicit or: Origin): Fragment[Void] =
     Fragment(sc.parts.toList.map(Left(_)), Void.codec, or)
@@ -156,6 +159,14 @@ object StringContextOps {
       }
     }
 
+    def quotedIdentifier_impl(): Tree = {
+      val Apply(_, List(Apply(_, List(Literal(Constant(part: String)))))) = c.prefix.tree: @unchecked
+      Identifier.fromStringQuoted(part) match {
+        case Left(s) => c.abort(c.enclosingPosition, s)
+        case Right(Identifier(s)) => q"_root_.skunk.data.Identifier.fromStringQuoted($s).fold(sys.error, identity)"
+      }
+    }
+
   }
 
 }

modules/core/shared/src/main/scala-3/syntax/StringContextOps.scala

@@ -154,6 +154,21 @@ object StringContextOps {
         return '{???}
     }
 
+  def qidImpl(sc: Expr[StringContext])(using qc: Quotes): Expr[Identifier] =
+    import qc.reflect.report
+    sc match {
+      case '{ StringContext(${Varargs(Exprs(Seq(part)))}: _*) } =>
+        Identifier.fromStringQuoted(part) match {
+          case Right(Identifier(s)) => '{ Identifier.fromStringQuoted(${Expr(s)}).fold(sys.error, identity) }
+          case Left(s) =>
+            report.error(s)
+            return '{???}
+        }
+      case _ =>
+        report.error(s"Identifiers cannot have interpolated arguments")
+        return '{???}
+    }
+
 }
 
 trait ToStringContextOps {
@@ -164,6 +179,9 @@ trait ToStringContextOps {
   extension (inline sc: StringContext) inline def id(): Identifier =
     ${ StringContextOps.idImpl('sc) }
 
+  extension (inline sc: StringContext) inline def qid(): Identifier =
+    ${ StringContextOps.qidImpl('sc) }
+
   implicit def toStringOps(sc: StringContext): StringContextOps =
     new StringContextOps(sc)
 }

modules/core/shared/src/main/scala/Channel.scala

@@ -120,29 +120,29 @@ object Channel {
     new Channel[F, String, String] {
 
     val listen: F[Unit] =
-      proto.execute(Command(s"LISTEN ${name.value}", Origin.unknown, Void.codec)).void
+      proto.execute(Command(s"LISTEN ${name.asSql}", Origin.unknown, Void.codec)).void
 
     val unlisten: F[Unit] =
-      proto.execute(Command(s"UNLISTEN ${name.value}", Origin.unknown, Void.codec)).void
+      proto.execute(Command(s"UNLISTEN ${name.asSql}", Origin.unknown, Void.codec)).void
 
     def listen(maxQueued: Int): Stream[F, Notification[String]] =
       for {
         _ <- Stream.resource(Resource.make(listen)(_ => unlisten))
         s <- Stream.resource(proto.notifications(maxQueued))
-        n <- s.filter(_.channel === name)
+        n <- s.filter(_.channel.value === name.value)
       } yield n
 
 
     def listenR(maxQueued: Int): Resource[F, Stream[F, Notification[String]]] =
       for {
         _      <- Resource.make(listen)(_ => unlisten)
         stream <- proto.notifications(maxQueued)
-      } yield stream.filter(_.channel === name)
+      } yield stream.filter(_.channel.value === name.value)
 
 
     def notify(message: String): F[Unit] =
       // TODO: escape the message
-      proto.execute(Command(s"NOTIFY ${name.value}, '$message'", Origin.unknown, Void.codec)).void
+      proto.execute(Command(s"NOTIFY ${name.asSql}, '$message'", Origin.unknown, Void.codec)).void
 
   }
 

modules/core/shared/src/main/scala/data/Identifier.scala

@@ -9,7 +9,15 @@ import cats.Eq
 import scala.util.matching.Regex
 
 sealed abstract case class Identifier(value: String) {
-  override def toString: String = value // ok?
+  def quoted: Boolean = false
+  def asSql: String = if (quoted) s"\"${value.replace("\"", "\"\"")}\"" else value
+  override def toString: String = asSql
+
+  override def equals(other: Any): Boolean = other match {
+    case that: Identifier => this.value == that.value && this.quoted == that.quoted
+    case _                => false
+  }
+  override def hashCode: Int = (value, quoted).##
 }
 
 object Identifier {
@@ -20,7 +28,7 @@ object Identifier {
   val pat: Regex = "([A-Za-z_][A-Za-z_0-9$]*)".r
 
   implicit val EqIdentifier: Eq[Identifier] =
-    Eq.by(_.value)
+    Eq.instance((a, b) => a.value == b.value && a.quoted == b.quoted)
 
   def fromString(s: String): Either[String, Identifier] =
     s match {
@@ -34,6 +42,20 @@ object Identifier {
       case _ => Left(s"Malformed identifier: does not match ${pat.regex}")
     }
 
+  def fromStringQuoted(s: String): Either[String, Identifier] = {
+    if (s.isEmpty)
+      Left("Illegal identifier: zero-length delimited identifier.")
+    else if (s.contains('\u0000'))
+      Left("Illegal identifier: cannot contain the null byte (\\u0000).")
+    else {
+      val byteLen = s.getBytes(java.nio.charset.StandardCharsets.UTF_8).length
+      if (byteLen > maxLen)
+        Left(s"Identifier too long: $byteLen bytes (max allowed is $maxLen)")
+      else
+        Right(new Identifier(s) { override val quoted: Boolean = true })
+    }
+  }
+
   val keywords: Set[String] =
     Set(
       "A",                               "ABORT",                   "ABS",                              "ABSENT",                   "ABSOLUTE",
@@ -190,4 +212,4 @@ object Identifier {
       "YES",                             "ZONE"
     )
 
-}
+}

modules/core/shared/src/main/scala/net/message/package.scala

@@ -58,7 +58,7 @@ package object message { module =>
 
   val identifier: SCodec[Identifier] =
     utf8z.exmap(
-      s  => Attempt.fromEither(Identifier.fromString(s).leftMap(Err(_))),
+      s  => Attempt.fromEither(Identifier.fromString(s).orElse(Identifier.fromStringQuoted(s)).leftMap(Err(_))),
       id => Attempt.successful(id.value)
     )
 

modules/docs/src/main/laika/reference/Identifiers.md

@@ -1,2 +1,52 @@
+```scala mdoc:invisible
+import skunk.data.Identifier
+import skunk.implicits._
+```
 # Identifiers
 
+`skunk.data.Identifier` represents a Postgres SQL identifier — the name of a table, column, schema, channel, etc. Skunk validates identifiers up front so they can be safely spliced into SQL without risking injection.
+
+Postgres recognises two flavours of identifier, and `Identifier` supports both.
+
+## Unquoted identifiers
+
+An *unquoted* identifier matches `[A-Za-z_][A-Za-z_0-9$]*`, is at most 63 characters, and is not a reserved keyword. Postgres folds unquoted identifiers to lower case, so `FOO`, `Foo`, and `foo` all refer to the same object.
+
+Construct one with `Identifier.fromString` or the `id"…"` interpolator:
+
+```scala mdoc:compile-only
+val a: Either[String, Identifier] = Identifier.fromString("my_table")
+val b: Identifier                 = id"my_table"
+```
+
+The `id"…"` form validates at compile time and fails the build for malformed input.
+
+## Quoted (delimited) identifiers
+
+A *quoted* (delimited) identifier is any non-empty character sequence that does not contain the NUL byte. Quoting preserves case and lets you use characters or reserved words that an unquoted identifier cannot.
+
+Construct one with `Identifier.fromStringQuoted` or the `qid"…"` interpolator:
+
+```scala mdoc:compile-only
+val a: Either[String, Identifier] = Identifier.fromStringQuoted("MyTable")  // case preserved
+val b: Identifier                 = qid"q_my_queue.INSERT"                  // keywords allowed
+```
+
+Like `id"…"`, the `qid"…"` form validates at compile time and fails the build for malformed input (empty string, embedded space, or > 63 bytes).
+
+Length is checked in **bytes** (Postgres' `NAMEDATALEN-1` is byte-counted), so multibyte characters are accounted for correctly.
+
+## Rendering as SQL
+
+`Identifier#asSql` returns the SQL-ready form: the bare value for unquoted identifiers, or the value wrapped in double quotes (with any embedded `"` doubled) for quoted ones. `toString` returns `asSql`, so logged identifiers show their SQL-correct form. `value` always returns the bare, unescaped name.
+
+```scala mdoc:compile-only
+val unq = id"my_table"
+val unqRendered = unq.asSql       // "my_table"
+
+val q = qid"My.Channel"
+val qBare = q.value               // "My.Channel"
+val qRendered = q.asSql           // "\"My.Channel\""
+```
+
+`Channel` uses `asSql` internally when issuing `LISTEN`/`UNLISTEN`/`NOTIFY`, so quoted channel names round-trip correctly.

modules/docs/src/main/laika/tutorial/Channels.md

@@ -27,6 +27,15 @@ Observe the following:
 - `ch` is a `Channel` which consumes `String`s and emits `Notification[String]`s. A notification is a structure that includes the process ID and channel identifier as well as the payload.
 - `Channel` is a profunctor and thus can be contramapped to change the input type, and mapped to change the output type.
 
+If the channel name contains characters that are not valid in an unquoted identifier, use the `qid"…"` interpolator (or `Identifier.fromStringQuoted`) to build a quoted identifier:
+
+```scala mdoc:compile-only
+// assume s: Session[IO]
+val ch = s.channel(qid"q_my_queue.INSERT")
+```
+
+The resulting `LISTEN`/`UNLISTEN`/`NOTIFY` statements wrap the name in double quotes so Postgres parses it correctly.
+
 ## Listening to a Channel
 
 To listen on a channel, construct a stream via `.listen`.

modules/tests/shared/src/test/scala/ChannelTest.scala

@@ -56,5 +56,16 @@ class ChannelTest extends SkunkTest {
     }
   }
 
+  sessionTest("channel with quoted identifier round-trips through LISTEN/NOTIFY/UNLISTEN") { s =>
+    val data = List("foo", "bar", "baz")
+    val ch = s.channel(qid"pgmq.q_my_queue.INSERT")
+    ch.listenR(42).use { r =>
+      for {
+        _ <- data.traverse_(ch.notify)
+        d <- r.map(_.value).takeThrough(_ != data.last).compile.toList
+        _ <- assert(s"channel data $d $data", data == d)
+      } yield "ok"
+    }
+  }
 
 }

modules/tests/shared/src/test/scala/data/IdentifierTest.scala

@@ -51,5 +51,70 @@ class IdentifierTest extends ffstest.FTest {
     assertEqual("value", id"foo".toString, "foo")
   }
 
+  test("quoted - valid with dots") {
+    Identifier.fromStringQuoted("pgmq.q_my_queue.INSERT") match {
+      case Left(err) => fail(err)
+      case Right(id) =>
+        for {
+          _ <- assertEqual("value", id.value, "pgmq.q_my_queue.INSERT")
+          _ <- assertEqual("asSql", id.asSql, "\"pgmq.q_my_queue.INSERT\"")
+          _ <- assertEqual("quoted", id.quoted, true)
+        } yield ()
+    }
+  }
+
+  test("quoted - escapes embedded double quotes") {
+    Identifier.fromStringQuoted("with\"quote") match {
+      case Left(err) => fail(err)
+      case Right(id) => assertEqual("asSql", id.asSql, "\"with\"\"quote\"")
+    }
+  }
+
+  test("quoted - empty rejected") {
+    Identifier.fromStringQuoted("") match {
+      case Left(err) => err.pure[IO]
+      case Right(value) => fail(s"expected error, got $value")
+    }
+  }
+
+  test("quoted - null byte rejected") {
+    val nullByte = '\u0000'
+    Identifier.fromStringQuoted(s"a${nullByte}b") match {
+      case Left(err) => err.pure[IO]
+      case Right(value) => fail(s"expected error, got $value")
+    }
+  }
+
+  test("quoted - too long in bytes") {
+    Identifier.fromStringQuoted("é" * 32) match {
+      case Left(err) => err.pure[IO]
+      case Right(value) => fail(s"expected error, got $value")
+    }
+  }
+
+  test("quoted - keywords allowed") {
+    Identifier.fromStringQuoted("SELECT") match {
+      case Left(err) => fail(err)
+      case Right(id) =>
+        for {
+          _ <- assertEqual("value", id.value, "SELECT")
+          _ <- assertEqual("asSql", id.asSql, "\"SELECT\"")
+        } yield ()
+    }
+  }
+
+  test("unquoted - asSql equals value") {
+    assertEqual("asSql", id"foo".asSql, "foo")
+  }
+
+  test("Eq distinguishes quoted from unquoted with same value") {
+    val unquoted = Identifier.fromString("foo").toOption.get
+    val quoted = Identifier.fromStringQuoted("foo").toOption.get
+    for {
+      _ <- IO(assert(unquoted =!= quoted))
+      _ <- IO(assert(unquoted != quoted))
+    } yield ()
+  }
+
 }