Skip to content

Commit

Permalink
Replace appendPlaceholders with simpler placeholder
Browse files Browse the repository at this point in the history
This allows us to use mkString for placeholder implementations, instead
of doing complicated iteration things with conditional comma inclusion
  • Loading branch information
coreywoodfield committed Nov 18, 2024
1 parent 4064383 commit fcfd615
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class InterpolatedQuery(protected val parsedQuery: String, protected val params:

protected def applyParams(stmt: PreparedStatement) = parameterize(stmt, 1)

def appendPlaceholders(stringBuilder: StringBuilder) = stringBuilder ++= parsedQuery
def placeholder = parsedQuery

def withTimeout(seconds: Int): InterpolatedQuery = new InterpolatedQuery(parsedQuery, params) {
override protected def normalStatement(implicit conn: Connection) = new BaseStatement(conn)
Expand All @@ -37,13 +37,8 @@ class InterpolatedQuery(protected val parsedQuery: String, protected val params:
object InterpolatedQuery {

def fromParts(parts: Seq[String], params: Seq[Parameter]) = {
val stringBuilder = new StringBuilder()
parts.zip(params).foreach { case (part, param) =>
stringBuilder ++= part
param.appendPlaceholders(stringBuilder)
}
stringBuilder ++= parts.last
new InterpolatedQuery(stringBuilder.toString(), params)
val query = StringContext.standardInterpolator(identity, params.map(_.placeholder), parts)
new InterpolatedQuery(query, params)
}

}
25 changes: 4 additions & 21 deletions relate/src/main/scala/com/lucidchart/relate/Parameters.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import scala.language.implicitConversions
*/

trait Parameter {
def appendPlaceholders(stringBuilder: StringBuilder)
def placeholder: String
def parameterize(statement: PreparedStatement, i: Int): Int
}

Expand Down Expand Up @@ -616,7 +616,7 @@ object Parameter {
trait SingleParameter extends Parameter {
protected[this] def set(statement: PreparedStatement, i: Int)

def appendPlaceholders(stringBuilder: StringBuilder) = stringBuilder.append("?")
def placeholder = "?"
def parameterize(statement: PreparedStatement, i: Int) = {
set(statement, i)
i + 1
Expand All @@ -633,30 +633,13 @@ trait MultipleParameter extends Parameter {
}

class TupleParameter(val params: Iterable[SingleParameter]) extends MultipleParameter {
def appendPlaceholders(stringBuilder: StringBuilder) =
// if we don't use the iterator, we won't necessarily get a consistent iteration order: the element with index 0
// according to zipWithIndex might not be the first element handled by the foreach
params.iterator.zipWithIndex.foreach { case (param, index) =>
if (0 < index) {
stringBuilder.append(",")
}
param.appendPlaceholders(stringBuilder)
}
def placeholder = params.iterator.map(_.placeholder).mkString(",")
}

object TupleParameter {
def apply(params: SingleParameter*) = new TupleParameter(params)
}

class TuplesParameter(val params: Iterable[TupleParameter]) extends MultipleParameter {
def appendPlaceholders(stringBuilder: StringBuilder) = {
if (params.nonEmpty) {
params.foreach { param =>
stringBuilder.append("(")
param.appendPlaceholders(stringBuilder)
stringBuilder.append("),")
}
stringBuilder.setLength(stringBuilder.length - 1)
}
}
def placeholder = if (params.isEmpty) "" else params.map(_.placeholder).mkString("(", "),(", ")")
}
6 changes: 1 addition & 5 deletions relate/src/test/scala/ParameterizationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ class ParameterizationTest extends Specification {
}

"interpolate HashSets properly" in {
// note that HashSets don't iterate consistently: zipWithIndex and head get different "first" elements
// (also that zipWithIndex on a HashSet returns another HashSet)
val hashSet: Set[Int] = scala.collection.immutable.HashSet(1, 2, 3)
hashSet.zipWithIndex.head._2 mustNotEqual 0
// even so, we should interpolate it correctly
val querySql = sql"SELECT * FROM myTable WHERE id IN ($hashSet)"
querySql.toString mustEqual "SELECT * FROM myTable WHERE id IN (?,?,?)"
}
Expand All @@ -35,7 +31,7 @@ class ParameterizationTest extends Specification {
class CustomParameter(value: Int) extends SingleParameter {
protected[this] def set(statement: PreparedStatement, i: Int) =
implicitly[Parameterizable[Int]].set(statement, i, value)
override def appendPlaceholders(stringBuilder: StringBuilder) = stringBuilder.append("?::smallint")
override def placeholder = "?::smallint"
}
val querySql = sql"INSERT INTO myTable (foo, bar) VALUES (${(1, new CustomParameter(1))})"
querySql.toString mustEqual "INSERT INTO myTable (foo, bar) VALUES (?,?::smallint)"
Expand Down

0 comments on commit fcfd615

Please sign in to comment.