Skip to content

1: Field/parameter names are not case insensitive #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benedeki wants to merge 9 commits into
base: master
Choose a base branch
from
Open

1: Field/parameter names are not case insensitive #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d6aee9d
1: Field/parameter names are not case insensitive
benedeki Dec 10, 2025
b73ff73
* Fixed issues found by AI review
benedeki Dec 10, 2025
5692a5a
Merge branch 'master' into bugfix/1-field-parameter-names-are-not-cas…
benedeki Dec 10, 2025
fd6e147
* mixed case names are now lower cased unless they are quoted or cont…
benedeki Dec 19, 2025
7d81fb3
* removed unneeded `unapply` method
benedeki Dec 19, 2025
2e09d2f
Merge branch 'master' into bugfix/1-field-parameter-names-are-not-cas…
benedeki Dec 23, 2025
ea9ed2f
* addressed PR comments
benedeki Dec 31, 2025
140c150
* Further fixes
benedeki Dec 31, 2025
4d0d379
Merge branch 'master' into bugfix/1-field-parameter-names-are-not-cas…
benedeki Dec 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ jobs:
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/07_testing_pg_types_data.sql
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/08_testing.simple_function.sql
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/09_testing.table_lifecycle.ddl
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/10_testing.strange_columns.sql
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/11_testing.strange_columns_data.sql

- name: Build and run integration tests
run: sbt ++${{matrix.scala}} testIT
2 changes: 2 additions & 0 deletions .github/workflows/jacoco_report.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ jobs:
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/07_testing_pg_types_data.sql
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/08_testing.simple_function.sql
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/09_testing.table_lifecycle.ddl
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/10_testing.strange_columns.sql
psql postgresql://postgres:postgres@localhost:5432/mag_db -f balta/src/test/resources/db/postgres/11_testing.strange_columns_data.sql

- name: Build and run tests
continue-on-error: true
Expand Down
59 changes: 32 additions & 27 deletions balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@

package za.co.absa.db.balta.classes

import DBFunction.{DBFunctionWithNamedParamsToo, DBFunctionWithPositionedParamsOnly, ParamsMap}

import scala.collection.immutable.ListMap
import za.co.absa.db.balta.typeclasses.{QueryParamValue, QueryParamType}
import za.co.absa.db.balta.classes.DBFunction.{DBFunctionWithNamedParamsToo, DBFunctionWithPositionedParamsOnly}
import za.co.absa.db.balta.typeclasses.QueryParamType
import za.co.absa.db.balta.classes.inner.Params.{NamedParams, OrderedParams}

/**
* A class that represents a database function call. It can be used to execute a function and verify the result.
Expand All @@ -29,18 +28,20 @@ import za.co.absa.db.balta.typeclasses.{QueryParamValue, QueryParamType}
* name; note that the position defined parameters can be added only at the beginning of the parameter list
*
* @param functionName - the name of the function
* @param params - the list of parameters
* @param orderedParams - the list of parameters identified by their position (preceding the named parameters)
* @param namedParams - the list of parameters identified by their name (following the positioned parameters)
*
*/
sealed abstract class DBFunction private(functionName: String,
params: ParamsMap) extends DBQuerySupport {
orderedParams: OrderedParams,
namedParams: NamedParams) extends DBQuerySupport {

private def sql(orderBy: String): String = {
val paramEntries = params.map{case(key, setterFnc) =>
key match {
case Left(_) => setterFnc.sqlEntry
case Right(name) => s"$name := ${setterFnc.sqlEntry}" // TODO https://github.com/AbsaOSS/balta/issues/2
}
val positionedParamEntries = orderedParams.values.map(_.sqlEntry)
val namedParamEntries = namedParams.items.map{ case (columnName, queryParamValue) =>
columnName.sqlEntry + " := " + queryParamValue.sqlEntry
}
val paramEntries = positionedParamEntries ++ namedParamEntries
val paramsLine = paramEntries.mkString(",")
s"SELECT * FROM $functionName($paramsLine) $orderBy"
}
Expand Down Expand Up @@ -92,7 +93,7 @@ sealed abstract class DBFunction private(functionName: String,
*/
def execute[R](orderBy: String)(verify: QueryResult => R /* Assertion */)(implicit connection: DBConnection): R = {
val orderByPart = if (orderBy.nonEmpty) {s"ORDER BY $orderBy"} else ""
runQuery(sql(orderByPart), params.values.toList)(verify)
runQuery(sql(orderByPart), orderedParams.values ++ namedParams.values)(verify)
}

/**
Expand All @@ -104,9 +105,7 @@ sealed abstract class DBFunction private(functionName: String,
* @return - a new instance of the DBFunction class with the new parameter
*/
def setParam[T: QueryParamType](paramName: String, value: T): DBFunctionWithNamedParamsToo = {
val key = Right(paramName) // TODO normalization TODO https://github.com/AbsaOSS/balta/issues/1
val queryValue = implicitly[QueryParamType[T]].toQueryParamValue(value)
DBFunctionWithNamedParamsToo(functionName, params + (key -> queryValue))
DBFunctionWithNamedParamsToo(functionName, orderedParams, namedParams.add(paramName, value))
}

/**
Expand All @@ -127,15 +126,13 @@ sealed abstract class DBFunction private(functionName: String,
* @return - a new instance of the DBFunction class without any parameters set
*/
def clear(): DBFunctionWithPositionedParamsOnly = {
DBFunctionWithPositionedParamsOnly(functionName)
DBFunctionWithPositionedParamsOnly(functionName, OrderedParams())
Copy link
Collaborator

@lsulak lsulak Jan 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this method to be really confusing and cannot really see a nice clear usage. I'm actually not a big fan of some of this design at all, it seems rather complicated.

I like DBFunction - inheritance and API wise.

I think I like that it's private and you have 3 apply method in the companion object. But I don't like these two DBFunctionWithPositionedParamsOnly and DBFunctionWithNamedParamsToo at all, that's where the problems are arising.

I don't like name of DBFunctionWithNamedParamsToo - what is 'too'? (rhetorical question, ofc I know the answer) The class itself is not self-explanatory, some outside context is required, which builds mental dependencies. Unnecessary complexity.

Also, the 'direction' of code flow is just not simple. This is the core of the problem for me. You have DBFunction as a basis, then DBFunctionWithNamedParamsToo is its child, but the parent DBFunction is returning DBFunctionWithNamedParamsToo in one of the method. :(

Copy link
Collaborator

@lsulak lsulak Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the name clear is just too wide in my opinion

Copy link
Collaborator

@lsulak lsulak Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if DBFunction is just a trait? Containing very little method implementation, probably not even the sql method. Then the companion object would contain 1-3 apply methods (I prefer just 1 - why to make everything so rich and exhaustive?), something like:

def apply(functionName: String, orderedParams: OrderedParams = OrderedParams(), namedParams: NamedParams = NamedParams()): DBFunction =
    DBFunctionImpl(functionName, orderedParams, namedParams)

see the use of default parameters - you already have apply methods for Named and Ordered Params for this.

And then you would have something like this, the implementation class within the companion object itself

 private case class DBFunctionImpl(functionName: String, orderedParams: OrderedParams, namedParams: NamedParams) extends DBFunction { ... <method SQL would be implemented here> ... }

Copy link
Collaborator

@lsulak lsulak Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my prompt, the AI generated something like this, take it as an idea of some pseudo-code but interesting in design

/**
 * A class that represents a database function call. It can be used to execute a function and verify the result.
 * There are two implementations of this class:
 * - withPositionedParams: parameters are defined by their position only
 * - withNamedParams: parameters are defined by their name only
 *
 * @param functionName  - the name of the function
 * @param orderedParams - the list of parameters identified by their position
 * @param namedParams   - the list of parameters identified by their name
 */
sealed trait DBFunction extends DBQuerySupport {
  def perform()(implicit connection: DBConnection): Unit = execute("")(_ => ())
  def getResult(orderBy: String = "")(implicit connection: DBConnection): List[QueryResultRow] = execute(orderBy)(_.toList)
  def execute[R](verify: QueryResult => R)(implicit connection: DBConnection): R = execute("")(verify)
  def execute[R](orderBy: String)(verify: QueryResult => R)(implicit connection: DBConnection): R
  def setOrderedParam[T: QueryParamType](value: T): DBFunction
  def setNamedParam[T: QueryParamType](name: String, value: T): DBFunction
  def clear(): DBFunction
}

object DBFunction {
  def apply(functionName: String, orderedParams: OrderedParams = OrderedParams(), namedParams: NamedParams = NamedParams()): DBFunction =
    DBFunctionImpl(functionName, orderedParams, namedParams)

  private case class DBFunctionImpl(functionName: String, orderedParams: OrderedParams, namedParams: NamedParams) extends DBFunction {
    private def sql(orderBy: String): String = {
      val orderedEntries = orderedParams.values.map(_.sqlEntry)
      val namedEntries = namedParams.items.map { case (columnName, queryParamValue) =>
        columnName.sqlEntry + " := " + queryParamValue.sqlEntry
      }
      val paramsLine = (orderedEntries ++ namedEntries).mkString(",")
      s"SELECT * FROM $functionName($paramsLine) $orderBy"
    }
    override def setOrderedParam[T: QueryParamType](value: T): DBFunction =
      copy(orderedParams = orderedParams.add(value))
    override def setNamedParam[T: QueryParamType](name: String, value: T): DBFunction =
      copy(namedParams = namedParams.add(name, value))
    override def execute[R](orderBy: String)(verify: QueryResult => R)(implicit connection: DBConnection): R = {
      val orderByPart = if (orderBy.nonEmpty) s"ORDER BY $orderBy" else ""
      runQuery(sql(orderByPart), orderedParams.values ++ namedParams.values)(verify)
    }
    override def clear(): DBFunction = copy(orderedParams = OrderedParams(), namedParams = NamedParams())
  }
}

Copy link
Contributor Author

@benedeki benedeki Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will think about the rest, but the AI suggestion is fundamentally wrong.
It allows to add positioned (ordered) parameters after you have added a named one - that is incorrect. Once you add a named parameter only named ones can follow. That's why DBFunction is just a shell, and have two (hidden) children.

The other comments might have their point - but they are somewhat getting into the territory of really refactoring DBFunction - which is not the goal of this PR, and interacts with my plans for further improvements here (I know, I haven't voice them, and even less put them into a ticket). Which of course doesn't diminish their validity (definitely the naming could be better, I accept that)

Copy link
Collaborator

@lsulak lsulak Jan 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows to add positioned (ordered) parameters after you have added a named one - that is incorrect

Ok, then it's probably necessary to create some sub-entities of DBFunction and treat it separately. Maybe. I will think about it

The other comments might have their point - but they are somewhat getting into the territory of really refactoring DBFunction - which is not the goal of this PR, and interacts with my plans for further improvements here (I know, I haven't voice them, and even less put them into a ticket)

Hmm I partially understand/agree, but it wasn't possible to strongly voice this earlier - you introduced orderedParams and namedParams only now, instead of ParamsMap in the DBFunction. But ofc DBFunctionWithNamedParamsToo was present earlier and I haven't noticed / thought about it this way

Copy link
Contributor Author

@benedeki benedeki Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ordered and named parameters were there before too, just handled by a Map hidden behind a custom type and pretending posiyioned parameters have a name, with dedicated methods and functions. Now it's much cleaner actually, I believe, which perhaps made stand out the remaining imperfections.

Copy link
Contributor Author

@benedeki benedeki Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ordered and namef parameters were there before too, just handled by a Map hidden behind a custom type and pretending posiioned parameters have a name, with dedicated methods and functions. Now it's much cleaner actually, I believe, which perhaps made stand out the remaining imperfections.

}
}


object DBFunction {

type ParamsMap = ListMap[Either[Int, String], QueryParamValue]

/**
* Creates a new instance of the DBFunction class with the given function name without any parameters set.
*
Expand All @@ -146,16 +143,24 @@ object DBFunction {
DBFunctionWithPositionedParamsOnly(functionName)
}

def apply(functionName: String, params: NamedParams): DBFunctionWithNamedParamsToo = {
Copy link
Collaborator

@lsulak lsulak Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like these two apply methods.

DBFunctionWithNamedParamsToo(functionName, OrderedParams(), params)
}

def apply(functionName: String, params: OrderedParams): DBFunctionWithPositionedParamsOnly = {
DBFunctionWithPositionedParamsOnly(functionName, params)
}

/**
* Class that represents a database function call with parameters defined by their position only. It's the default
* class when creating a new instance of the DBFunction class without any parameters set.
*
* @param functionName - the name of the function
* @param params - the list of parameters
* @param orderedParams - the list of parameters identified by their position (preceding the named parameters)
*/
sealed case class DBFunctionWithPositionedParamsOnly private(functionName: String,
params: ParamsMap = ListMap.empty
) extends DBFunction(functionName, params) {
orderedParams: OrderedParams = OrderedParams()
) extends DBFunction(functionName, orderedParams, namedParams = NamedParams()) {
/**
* Sets a parameter for the function call. It actually creates a new instance of the DBFunction class with the new
* parameter. The new parameter is the last in the parameter list.
Expand All @@ -164,9 +169,7 @@ object DBFunction {
* @return - a new instance of the DBFunction class with the new parameter
*/
def setParam[T: QueryParamType](value: T): DBFunctionWithPositionedParamsOnly = {
val key = Left(params.size + 1)
val queryValue = implicitly[QueryParamType[T]].toQueryParamValue(value)
DBFunctionWithPositionedParamsOnly(functionName, params + (key -> queryValue))
DBFunctionWithPositionedParamsOnly(functionName, orderedParams.add(value))
}

/**
Expand All @@ -187,9 +190,11 @@ object DBFunction {
* position (at the beginning of the list) and by their name (for the rest of the list).
*
* @param functionName - the name of the function
* @param params - the list of parameters
* @param orderedParams - the list of parameters identified by their position (preceding the named parameters)
* @param namedParams - the list of parameters identified by their name (following the positioned parameters)
*/
sealed case class DBFunctionWithNamedParamsToo private(functionName: String,
params: ParamsMap = ListMap.empty
) extends DBFunction(functionName, params)
orderedParams: OrderedParams = OrderedParams(),
namedParams: NamedParams = NamedParams()
) extends DBFunction(functionName, orderedParams, namedParams)
}
2 changes: 1 addition & 1 deletion balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import za.co.absa.db.balta.typeclasses.QueryParamValue
*/
trait DBQuerySupport {

protected def runQuery[R](sql: String, queryValues: List[QueryParamValue])
protected def runQuery[R](sql: String, queryValues: Vector[QueryParamValue])
(verify: QueryResult => R /* Assertion */)
(implicit connection: DBConnection): R = {
val preparedStatement = connection.connection.prepareStatement(sql)
Expand Down
27 changes: 14 additions & 13 deletions balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package za.co.absa.db.balta.classes
import za.co.absa.db.balta.classes.inner.Params
import za.co.absa.db.balta.classes.inner.Params.NamedParams
import za.co.absa.db.balta.typeclasses.{QueryParamValue, QueryParamType}
import za.co.absa.db.balta.classes.inner.Params.OrderedParams

/**
* This class represents a database table. It allows to perform INSERT, SELECT and COUNT operations on the table easily.
Expand All @@ -35,10 +36,11 @@ case class DBTable(tableName: String) extends DBQuerySupport{
* @return - the inserted row.
*/
def insert(values: Params)(implicit connection: DBConnection): QueryResultRow = {
val columns = values.keys.map {keys =>
val keysString = keys.mkString(",") // TODO https://github.com/AbsaOSS/balta/issues/2
s"($keysString)"
}.getOrElse("")
val columns = values match {
case namedParams: NamedParams => namedParams.paramNames.map(_.sqlEntry).mkString("(", ",", ")")
case _: OrderedParams => ""
}

val paramStr = values.values.map(_.sqlEntry).mkString(",")
val sql = s"INSERT INTO $tableName $columns VALUES($paramStr) RETURNING *;"
runQuery(sql, values.values){_.next()}
Expand All @@ -49,18 +51,18 @@ case class DBTable(tableName: String) extends DBQuerySupport{
*
* @param keyName - the name of the key column
* @param keyValue - the value of the key column
* @param fieldName - the name of the field to be returned
* @param columnName - the name of the field to be returned
* @param connection - a database connection used for the SELECT operation.
* @tparam K - the type of the key value
* @tparam T - the type of the returned field value
* @return - the value of the field, if the value is NULL, then `Some(None)` is returned; if no row is found,
* then `None` is returned.
*/
def fieldValue[K: QueryParamType, T](keyName: String, keyValue: K, fieldName: String)
def fieldValue[K: QueryParamType, T](keyName: String, keyValue: K, columnName: String)
Copy link
Collaborator

@lsulak lsulak Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def fieldValue[K: QueryParamType, T](keyName: String, keyValue: K, columnName: String)
def columnValue[K: QueryParamType, T](keyName: String, keyValue: K, columnName: String)

also the method description/doc can be changed slighlty

(implicit connection: DBConnection): Option[Option[T]] = {
where(Params.add(keyName, keyValue)){resultSet =>
if (resultSet.hasNext) {
Some(resultSet.next().getAs[T](fieldName))
Some(resultSet.next().getAs[T](columnName))
} else {
None
}
Expand Down Expand Up @@ -212,7 +214,7 @@ case class DBTable(tableName: String) extends DBQuerySupport{
composeCountAndRun(strToOption(condition))
}

private def composeSelectAndRun[R](whereCondition: Option[String], orderByExpr: Option[String], values: List[QueryParamValue] = List.empty)
private def composeSelectAndRun[R](whereCondition: Option[String], orderByExpr: Option[String], values: Vector[QueryParamValue] = Vector.empty)
(verify: QueryResult => R)
(implicit connection: DBConnection): R = {
val where = whereCondition.map("WHERE " + _).getOrElse("")
Expand All @@ -221,15 +223,15 @@ case class DBTable(tableName: String) extends DBQuerySupport{
runQuery(sql, values)(verify)
}

private def composeDeleteAndRun[R](whereCondition: Option[String], values: List[QueryParamValue] = List.empty)
private def composeDeleteAndRun[R](whereCondition: Option[String], values: Vector[QueryParamValue] = Vector.empty)
(verify: QueryResult => R)
(implicit connection: DBConnection): R = {
val where = whereCondition.map("WHERE " + _).getOrElse("")
val sql = s"DELETE FROM $tableName $where RETURNING *;"
runQuery(sql, values)(verify)
}

private def composeCountAndRun[R](whereCondition: Option[String], values: List[QueryParamValue] = List.empty)
private def composeCountAndRun(whereCondition: Option[String], values: Vector[QueryParamValue] = Vector.empty)
(implicit connection: DBConnection): Long = {
val where = whereCondition.map("WHERE " + _).getOrElse("")
val sql = s"SELECT count(1) AS cnt FROM $tableName $where;"
Expand All @@ -247,9 +249,8 @@ case class DBTable(tableName: String) extends DBQuerySupport{
}

private def paramsToWhereCondition(params: NamedParams): String = {
params.pairs.foldRight(List.empty[String]) {case ((fieldName, value), acc) =>
// TODO https://github.com/AbsaOSS/balta/issues/2
val condition = s"$fieldName ${value.equalityOperator} ${value.sqlEntry}"
params.items.foldRight(List.empty[String]) {case ((columnName, value), acc) =>
val condition = s"${columnName.sqlEntry} ${value.equalityOperator} ${value.sqlEntry}"
condition :: acc
}.mkString(" AND ")
}
Expand Down
4 changes: 2 additions & 2 deletions balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package za.co.absa.db.balta.classes

import za.co.absa.db.balta.classes.QueryResultRow.{Extractors, FieldNames}
import za.co.absa.db.balta.classes.QueryResultRow.{Extractors, ColumnNames}

import java.sql.{ResultSet, ResultSetMetaData, SQLException}

Expand All @@ -31,7 +31,7 @@ class QueryResult(resultSet: ResultSet) extends Iterator[QueryResultRow] {

private [this] var nextRow: Option[QueryResultRow] = None

private [this] implicit val fieldNames: FieldNames = QueryResultRow.fieldNamesFromMetadata(resultSetMetaData)
private [this] implicit val columnNames: ColumnNames = QueryResultRow.columnNamesFromMetadata(resultSetMetaData)
private [this] implicit val extractors: Extractors = QueryResultRow.createExtractors(resultSetMetaData)

override def hasNext: Boolean = {
Expand Down
Loading