#62: Make composition of SQL expression easier and safer #63
Conversation
* Introduced a new class `SqlEntry` which represents a part of and SQL expression * `SqlEntry` brings a number of methods and helper classes * `SqlEntryComposition` offers a way to compose a full SQL expression as `SqlEntry` in SQL format (almost) * `SqlEntry` is now leveraged in `SqlItem`, `DBFunction` and `DBTable`
benedeki
requested review from
lsulak,
miroslavpojer,
salamonpavel and
tmikula-dev
as code owners
WalkthroughAdds a SqlEntry-based SQL composition DSL and ColumnReference types; replaces ParamsMap with typed NamedParams/OrderedParams; migrates DBFunction, DBTable, DBQuerySupport, QueryResultRow/QueryResult to use SqlEntry and typed params; adds tests and CI steps to load quoted/strange-column test data. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite / Client
participant DSL as SqlEntryComposition (DSL)
participant DBLib as DBTable / DBFunction
participant Runner as DBQuerySupport
participant PG as Postgres DB
Note over Test,DSL: Build query using DSL + ColumnReference
Test->>DSL: compose SELECT ... FROM ... WHERE ... => SqlEntry
DSL-->>DBLib: provide SqlEntry (query fragment)
Test->>DBLib: attach NamedParams / OrderedParams
DBLib->>Runner: runQuery(sql: SqlEntry, params: Vector[QueryParamValue])
Runner->>Runner: prepareStatement(sql.entry) and bind params
Runner->>PG: execute prepared statement
PG-->>Runner: result set
Runner->>DBLib: QueryResult (uses ColumnNames/QueryResultRow)
DBLib-->>Test: invoke verification callback with QueryResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
benedeki
added
the
dependent
|
This item depends on: |
…ion-easier-and-safer
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (1)
118-121: Incorrect deprecation message.The deprecation message says
"Use add(NULL)"but this method isaddNull(paramName: String)which requires a parameter name. The replacement should beadd(paramName, NULL)to maintain the same semantics.- @deprecated("Use add(NULL)", "balta 0.3.0") + @deprecated("Use add(paramName, NULL)", "balta 0.3.0") def addNull(paramName: String): NamedParams = { add(paramName, QueryParamType.NULL) }
🧹 Nitpick comments (11)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala (1)
19-19: LGTM!The rename from
FieldNamestoColumnNamesimproves terminology consistency across the codebase. The change correctly updates both the import and the implicit field type.Optional: Consider renaming the initialization method for consistency.
While not critical, the method
fieldNamesFromMetadatacould be renamed tocolumnNamesFromMetadatato align with the new terminology throughout the codebase.Also applies to: 34-34
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (1)
56-56: Non-standardunapplysignature.The
unapplymethod returnsStringdirectly instead of the conventionalOption[String]. While this works as an irrefutable extractor, the standard Scala convention is to returnOption[T]for single-value extraction. This affects usability in pattern guards and partial matching scenarios.- def unapply(columnName: ColumnName): String = columnName.enteredName + def unapply(columnName: ColumnName): Option[String] = Some(columnName.enteredName)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (3)
72-72: Unusual access modifier combination.The
protectedmodifier on asealedclass has limited utility since subclasses are constrained to this file anyway. Consider using justsealedfor clarity.- sealed protected class Query(val sqlEntry: SqlEntry) + sealed class Query(val sqlEntry: SqlEntry)
80-80: Remove or address commented-out code.The commented-out method creates technical debt. If this functionality is planned, consider adding a TODO with a tracking issue; otherwise, remove it.
116-116: Minor formatting inconsistency.Missing space before
=compared to other method definitions.- def BY(columns: ColumnReference*): OrderByFragment= new OrderByFragment(columnsToSqlEntry(columns)) + def BY(columns: ColumnReference*): OrderByFragment = new OrderByFragment(columnsToSqlEntry(columns))balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (3)
33-33: Custom==operator shadowsAny.==.Defining
def ==(other: String)can cause confusion since it shadows the inherited equality method. Callers might accidentally usesqlEntry == anotherSqlEntryexpecting value comparison but getting reference comparison for non-String types.Consider renaming to a more explicit method like
equalsStringormatches, or ensure documentation clarifies this behavior.
51-54: Implicit reliance ontoStringfor SQL building.The
mkStringcall relies onSqlEntry.toStringreturning the underlying entry. While this works, using.map(_.sqlEntry.entry)would be more explicit and less fragile iftoStringwere ever changed for debugging purposes.def apply(firstColumn: ColumnReference, otherColumns: ColumnReference*): SqlEntry = { - val allColumns = (firstColumn +: otherColumns).map(_.sqlEntry).mkString(", ") + val allColumns = (firstColumn +: otherColumns).map(_.sqlEntry.entry).mkString(", ") SqlEntry(allColumns) }
94-94: Typo in comment."so space" should be "no space".
- // It looks better to have so space before characters like , ( ) + // It looks better to have no space before characters like , ( )balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (3)
62-65: Verify NULL import guidance in deprecation messages.The deprecation messages suggest
add(paramName, NULL)andadd(NULL), but users will need to referenceQueryParamType.NULLunless they import it. Consider clarifying in the messages (e.g.,"Use add(paramName, QueryParamType.NULL)") or providing a convenience re-export ofNULL.Also applies to: 86-89
125-130: Consider caching computed properties.
values,items, andparamNamesare recomputed on each access via.toVector.map(...). For typical small parameter lists this is fine, but if these are accessed repeatedly, consider usinglazy valor caching to avoid redundant allocations.- override def apply(position: Int): QueryParamValue = values(position) - override def size: Int = namedParams.size - override def values: Vector[QueryParamValue] = namedParams.toVector.map(_._2) - override def items: ParamsList = namedParams.toVector - - def paramNames: Vector[ColumnName] = namedParams.toVector.map(_._1) + override lazy val items: ParamsList = namedParams.toVector + override lazy val values: Vector[QueryParamValue] = items.map(_._2) + lazy val paramNames: Vector[ColumnName] = items.map(_._1) + override def size: Int = namedParams.size + override def apply(position: Int): QueryParamValue = values(position)
149-153: Document the indexing convention.
ColumnReferenceis created with 1-based indices (line 150:size + 1), aligning with SQL conventions. However,apply(position: Int)uses 0-based indexing (line 167:items(position)). This is internally consistent but could confuse users. Consider adding a ScalaDoc clarifying thatapply(position)uses 0-based indexing while the storedColumnReferenceuses 1-based SQL-style indices.Also applies to: 167-167
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/build.yml(1 hunks).github/workflows/jacoco_report.yml(1 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala(7 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala(1 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala(11 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala(2 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala(2 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala(7 hunks)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala(2 hunks)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala(1 hunks)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala(1 hunks)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala(1 hunks)balta/src/main/scala/za/co/absa/db/mag/core/SqlItem.scala(1 hunks)balta/src/test/resources/db/postgres/10_testing.strange_columns.sql(1 hunks)balta/src/test/resources/db/postgres/11_testing.strange_columns_data.sql(1 hunks)balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala(2 hunks)balta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scala(1 hunks)balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala(1 hunks)balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryUnitTests.scala(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (2)
SqlEntry(23-47)SqlEntry(49-103)
balta/src/main/scala/za/co/absa/db/mag/core/SqlItem.scala (2)
balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (1)
sqlEntry(59-59)
balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryUnitTests.scala (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (3)
SqlEntry(23-47)SqlEntry(49-103)toString(45-45)
balta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scala (3)
balta/src/main/scala/za/co/absa/db/balta/DBTestSuite.scala (1)
test(65-81)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (6)
ColumnReference(34-67)sqlEntry(59-59)unapply(56-56)unapply(71-71)ColumnName(29-32)ColumnName(69-72)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)
balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala (5)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (18)
SqlEntryComposition(5-120)SELECT(111-111)FROM(41-41)FROM(49-49)WHERE(76-76)WHERE(77-79)WHERE(92-92)WHERE(93-95)ORDER(58-58)BY(114-114)BY(115-115)BY(116-116)INSERT(112-112)INTO(28-30)VALUES(34-37)RETURNING(63-65)RETURNING(66-69)DELETE(113-113)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (3)
ColumnName(29-32)ColumnName(69-72)sqlEntry(59-59)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (2)
SqlEntry(23-47)SqlEntry(49-103)balta/src/main/scala/za/co/absa/db/balta/DBTestSuite.scala (1)
test(65-81)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (4)
balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (4)
sqlEntry(59-59)apply(42-50)apply(52-54)apply(70-70)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (8)
SqlEntry(23-47)SqlEntry(49-103)apply(35-39)apply(50-50)apply(51-54)apply(56-56)prefix(59-59)toOption(41-43)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (4)
apply(144-146)apply(148-150)apply(152-154)sql(41-49)
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (1)
balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (1)
NULL(86-86)
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (7)
apply(35-39)apply(50-50)apply(51-54)apply(56-56)SqlEntry(23-47)SqlEntry(49-103)toString(45-45)
balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (3)
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (6)
ColumnName(29-32)ColumnName(69-72)ColumnReference(34-67)apply(42-50)apply(52-54)apply(70-70)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (3)
apply(59-59)apply(65-65)apply(157-160)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (16)
QueryParamType(32-92)NULL(86-86)toQueryParamValue(35-35)toQueryParamValue(39-39)toQueryParamValue(43-43)toQueryParamValue(47-47)toQueryParamValue(51-51)toQueryParamValue(55-55)toQueryParamValue(59-59)toQueryParamValue(63-63)toQueryParamValue(67-67)toQueryParamValue(71-71)toQueryParamValue(75-75)toQueryParamValue(79-79)toQueryParamValue(83-83)toQueryParamValue(89-89)
balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala (6)
balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (1)
QueryParamType(32-92)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (3)
QueryParamValue(30-53)sqlEntry(26-26)equalityOperator(27-27)balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (8)
Params(29-36)Params(37-175)OrderedParams(141-170)OrderedParams(172-174)values(127-127)values(169-169)paramNames(130-130)items(128-128)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (5)
SqlEntry(23-47)SqlEntry(49-103)toOption(41-43)toSqlEntry(69-71)toSqlEntry(75-78)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (18)
SqlEntryComposition(5-120)INSERT(112-112)INTO(28-30)VALUES(34-37)RETURNING(63-65)RETURNING(66-69)SELECT(111-111)FROM(41-41)FROM(49-49)WHERE(76-76)WHERE(77-79)WHERE(92-92)WHERE(93-95)ORDER(58-58)BY(114-114)BY(115-115)BY(116-116)DELETE(113-113)balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala (1)
runQuery(27-48)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (1)
balta/src/main/scala/za/co/absa/db/balta/implicits/MapImplicits.scala (1)
getOrThrow(28-30)
🪛 GitHub Actions: Build
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala
[error] 42-42: Postgres: syntax error at or near "id_field". Position: 38. (Cause: DBQuerySupport.runQuery)
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala
[error] 1-1: sbt.TestsFailedException: Tests unsuccessful. 3 tests failed (DBTableIntegrationTests).
balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala
[error] 38-38: PostgreSQL syntax error at or near "id_field" during DBTable lifecycle tests. Position: 38.
🔇 Additional comments (24)
.github/workflows/build.yml (1)
74-75: LGTM!The additional database setup commands follow the established pattern and correctly load the new test resources for strange column identifiers.
balta/src/test/resources/db/postgres/10_testing.strange_columns.sql (1)
17-33: LGTM!The DDL effectively covers edge cases for column identifier handling: case sensitivity, spaces, special characters, Unicode, and numeric prefixes. This provides comprehensive test coverage for the new SQL composition framework.
.github/workflows/jacoco_report.yml (1)
51-52: LGTM!The database setup correctly mirrors the changes in build.yml, ensuring the Jacoco coverage workflow has access to the same test resources.
balta/src/test/resources/db/postgres/11_testing.strange_columns_data.sql (1)
17-19: LGTM!The INSERT statement correctly uses quoted identifiers matching the DDL and provides clean baseline test data.
balta/src/main/scala/za/co/absa/db/mag/core/SqlItem.scala (1)
19-21: LGTM!Clean, minimal trait definition following the interface segregation principle. This abstraction enables polymorphic handling of SQL fragments across the new composition framework.
balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryUnitTests.scala (1)
23-34: LGTM!The tests correctly verify SqlEntry's concatenation behavior (with space insertion) and toString implementation. The test expectations align with the SqlEntry.concat logic for handling spacing between SQL fragments.
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (2)
95-95: LGTM!The API change from
addNulltoadd("text_field", NULL)provides a more uniform parameter handling interface, consistent with the new typed parameter framework.
176-238: The test code is correct and does not require changes. TheParams.add()method automatically handles column name quoting throughColumnReference.apply(), which intelligently quotes identifiers when necessary:
- Unquoted input like
"Col1","col 1","col-1","colá","1col"are automatically quoted to match the table schema.- Quoted input like
"\"Col1\""is preserved as-is but produces identical SQL entries.- The
ColumnReferenceunit tests confirm that both approaches yield equivalent behavior (equal sqlEntry values).All test expectations are valid: initial count is 1 (from the data script), the two inserts add rows 2 and 3, the three where queries correctly filter by different column references, and the final cleanup verifies count returns to 0.
Likely an incorrect or invalid review comment.
balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala (2)
24-30: LGTM! Well-structured test fixtures.The test fixtures are clearly defined and reusable across test cases. The setup covers common SQL composition elements (columns, table, condition, function, parameter).
32-96: Good test coverage for SQL composition DSL.The tests comprehensively cover the main SQL composition scenarios including SELECT, INSERT, and DELETE with various clauses. Consider adding tests for edge cases such as empty conditions, special characters in column/table names, or NULL handling in future iterations.
balta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scala (1)
21-119: Comprehensive test coverage for ColumnReference API.The tests thoroughly validate the ColumnReference factory behavior including quoting rules, equality semantics, and pattern matching. Good coverage of edge cases like special characters, numeric prefixes, and cross-type equality.
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (2)
41-51: Enhanced column label resolution for quoted identifiers.The logic correctly handles quoted column labels by first attempting an exact match, then falling back to the inner (unquoted) label. This provides flexibility when working with quoted SQL identifiers.
152-152: Breaking change: Type alias renamed fromFieldNamestoColumnNames.This rename may break external code that references
QueryResultRow.FieldNames. Ensure this is documented in the release notes and any dependent code is updated.balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (2)
24-28: Clean migration toSqlEntryfor type-safe SQL fragment handling.The trait now returns
SqlEntryinstead ofStringforsqlEntryandequalityOperator, enabling type-safe composition with the new SQL DSL. The default implementations using cachedSqlEntryinstances (sqlQuestionMark,sqlEquals) are efficient.
47-51: CorrectNullParamValueimplementation.The
NullParamValuecorrectly providesNULLforsqlEntryandISforequalityOperator, enabling proper SQL composition for null comparisons (e.g.,column IS NULL).balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala (1)
27-30: Signature change is correct; pipeline failure requires investigation of actual SQL output.The signature change from
StringtoSqlEntryandListtoVectoris correctly implemented. TheSqlEntry.concat()logic properly handles spacing for SQL syntax (adds space except before,,(,),.or after(). The WHERE clause composition atDBTable.scala:243should produce correctly formatted SQL likeid_field = ?.Without access to the actual pipeline failure logs or generated SQL, the root cause cannot be confirmed. The issue may originate elsewhere or require deeper investigation into specific test execution.
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (2)
19-27: Custom equality implementation looks correct.The equality based on
sqlEntrycomparison and correspondinghashCodeimplementation is appropriate for this domain type. However, note thatSqlEntryis anAnyVal(value class), sosqlEntry.hashCodewill delegate toString.hashCodeon the underlyingentryfield, which is the expected behavior.
42-50: Factory method pattern matching is well-structured.The cascading pattern match correctly handles:
- Standard SQL identifiers (lowercase with underscores)
- Quoted but regular names (removes unnecessary quotes)
- Already-quoted special names (preserves as-is)
- Unquoted special names (adds quoting with escaping)
This provides a safe abstraction for column name handling.
balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala (1)
241-247: Well-structured WHERE condition builder.The
paramsToWhereConditionmethod correctly builds AND-joined conditions using the typedSqlEntryoperators. The use offoldRightwith accumulator andtoSqlEntry(" AND ")is idiomatic.balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (2)
41-49: SQL construction using the DSL is correct.The function call SQL generation properly builds parameter entries from both ordered and named params, with named params using the
:=operator for PostgreSQL named parameter syntax. ThefunctionName(paramsLine)correctly producesfunction_name(param1, param2, name := value).
144-154: Factory methods provide convenient construction.The overloaded
applymethods allow creatingDBFunctioninstances with different parameter configurations cleanly. The use ofSqlEntry(functionName)wraps the string appropriately.balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (3)
19-36: LGTM!The sealed abstract class design with a private constructor and abstract accessors provides a clean foundation for the typed parameter model. The immutable collection types are appropriate.
133-135: LGTM!Clean factory method for creating empty
NamedParamsinstances.
172-174: LGTM!Clean factory method for creating empty
OrderedParamsinstances.
balta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scala
Show resolved
Hide resolved
* code improvements
JaCoCo 'balta' module code coverage report - scala 2.12.18
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (1)
176-238: Good test coverage for edge cases with special column names.The test exercises important edge cases including:
- Mixed-case column names (
col1vsCol1)- Columns with spaces, hyphens, and unicode characters
- Quoted vs unquoted column name handling
Minor: there's an indentation inconsistency on lines 200-234 (extra leading space).
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (3)
33-33: Custom==method shadows inherited equals and only compares to String.The custom
==(other: String)method only handles String comparison and shadowsAny.==. This can lead to confusing behavior:
sqlEntry == "foo"uses your methodsqlEntry == anotherSqlEntryuses reference equality from AnyValConsider renaming to something like
matchesorequalsStringto avoid confusion, or implement proper equality for SqlEntry comparisons as well.- def ==(other: String): Boolean = this.entry == other + def matches(other: String): Boolean = this.entry == other
96-108: Variable shadowing reduces readability.Lines 97-98 define
val firstandval secondwhich shadow the method parameters of the same names. This makes the code harder to follow.case (Some(e1), Some(e2)) => - val first = e1.entry - val second = e2.entry + val firstStr = e1.entry + val secondStr = e2.entry val e2Start = e2.entry(0) // first character of second.entry, can never be empty here because of the previous cases val e1End = e1.entry.last // last character of the first.entry, can never be empty here because of the previous cases val noSpaceStartChars = Set(',', '(', ')', '.') val noSpaceEndChars = Set('(') if (noSpaceStartChars.contains(e2Start) || noSpaceEndChars.contains(e1End)) { // It looks better to have so space before characters like , ( ) - Some(SqlEntry(s"$first$second")) + Some(SqlEntry(s"$firstStr$secondStr")) } else { - Some(SqlEntry(s"$first $second")) + Some(SqlEntry(s"$firstStr $secondStr")) }
101-102: Consider extracting spacing character sets as companion object constants.The
noSpaceStartCharsandnoSpaceEndCharssets are created on everyconcatcall. For a minor performance improvement and better maintainability, these could be extracted as private constants in the companion object.object SqlEntry { + private val noSpaceStartChars = Set(',', '(', ')', '.') + private val noSpaceEndChars = Set('(') + def apply(entry: String): SqlEntry = new SqlEntry(entry)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (3)
37-39: Minor style inconsistency in object instantiation.
SelectFragmentandInsertFragmentuse()whileDeleteFragmentomits them. While both work, consistency is preferred.private object SelectFragment extends SelectFragment() private object InsertFragment extends InsertFragment() - private object DeleteFragment extends DeleteFragment + private object DeleteFragment extends DeleteFragment()
108-108: Fix formatting: missing space before=.- def BY(columns: ColumnReference*): OrderByFragment= new OrderByFragment(columnsToSqlEntry(columns)) + def BY(columns: ColumnReference*): OrderByFragment = new OrderByFragment(columnsToSqlEntry(columns))
121-128: Consider unifying column-to-entry conversion logic.This logic is similar to lines 10-11 in
SelectFragment.apply:val allFields = firstField +: fields new SelectWithFieldsFragment(select + allFields.map(_.sqlEntry).mkSqlEntry(", "))Consider extracting a shared helper or using the existing
SqlEntry.apply(ColumnReference*)factory from the companion object for consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala(7 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala(1 hunks)balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala(11 hunks)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala(1 hunks)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala(1 hunks)balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala(3 hunks)balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (5)
balta/src/main/scala/za/co/absa/db/balta/DBTestSuite.scala (1)
this(41-43)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (3)
apply(143-145)apply(147-149)apply(151-153)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (2)
apply(9-12)apply(14-14)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (4)
apply(42-50)apply(52-54)apply(70-70)sqlEntry(59-59)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)
balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala (4)
balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (1)
QueryParamType(32-92)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (3)
QueryParamValue(30-53)sqlEntry(26-26)equalityOperator(27-27)balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (8)
Params(29-36)Params(37-175)OrderedParams(141-170)OrderedParams(172-174)values(127-127)values(169-169)paramNames(130-130)items(128-128)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (5)
SqlEntry(23-58)SqlEntry(60-113)toOption(52-54)mkSqlEntry(82-84)mkSqlEntry(85-88)
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (4)
balta/src/main/scala/za/co/absa/db/balta/DBTestSuite.scala (3)
table(88-90)add(122-124)add(144-146)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (1)
NULL(86-86)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala (2)
next(50-58)noMore(63-63)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (4)
getLong(95-95)getLong(96-96)getString(89-89)getString(90-90)
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala (2)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (2)
SqlEntry(23-58)SqlEntry(60-113)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (1)
sql(41-48)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (2)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (11)
SqlEntry(23-58)SqlEntry(60-113)apply(35-38)apply(47-50)apply(61-61)apply(62-65)apply(67-67)mkSqlEntry(82-84)mkSqlEntry(85-88)prefix(70-70)toOption(52-54)balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (4)
apply(42-50)apply(52-54)apply(70-70)sqlEntry(59-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Scala 2.11.12
🔇 Additional comments (15)
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (2)
68-70: LGTM!Test assertion string updated consistently across both the return value and assertion.
95-95: LGTM!Correctly uses the new typed parameter API with
Params.add("text_field", NULL)for deleting rows with NULL values.balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala (1)
24-98: Comprehensive test coverage for the SQL DSL.The test suite validates the key SQL composition patterns including SELECT, INSERT, DELETE with various combinations of WHERE, ORDER BY, VALUES, and RETURNING clauses. The tests are well-organized and readable.
balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala (3)
221-241: LGTM!The private helper methods (
composeSelectAndRun,composeDeleteAndRun,composeCountAndRun) are cleanly refactored to use the SqlEntry DSL. The composition pattern with optional WHERE and ORDER BY clauses is handled correctly.
243-249: LGTM!The
paramsToWhereConditionmethod correctly builds parameterized WHERE clauses by combining column names with their equality operators and values usingAND. UsingfoldRightwith prepending maintains parameter order.
41-50: The INSERT SQL generation correctly produces single parentheses around column and value lists.The implementation is correct:
columnsis aVector[SqlEntry]of column namestable(columns)invokesapply(params: Seq[SqlEntry], foo: String = "")params.mkSqlEntry("(", ",", ")")produces(col1,col2)as a singleSqlEntry- The
+operator concatenates table name with the parenthesized columns—theconcatfunction avoids extra spacing before(, producingtableName(col1,col2)- Final result:
INSERT INTO tableName(col1, col2) VALUES(val1, val2) RETURNING *Unit tests confirm the expected single-parentheses format. No double parentheses issue exists.
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (1)
40-50: Dummy parameter is a workaround for type erasure - consider alternative approaches.Using
foo: String = ""as a dummy disambiguator works but is a code smell. The Scaladoc correctly explains the rationale, which is good.Alternative approaches (for future consideration):
- Use a different method name like
applySeq- Use a marker type instead of String
Current implementation is acceptable given the documentation.
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (3)
1-5: LGTM!Package declaration and imports are appropriate for the DSL functionality being introduced.
41-63: LGTM!The fragment classes and mix-in traits provide a clean, composable API for SQL clause construction. The use of traits for
OrderByMixInandReturningMixInenables flexible query composition.
65-97: LGTM!The sealed query hierarchy correctly models SQL query state transitions. The design prevents invalid compositions (e.g., multiple WHERE clauses) through the type system.
balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (5)
19-24: LGTM!The imports are appropriate for the new SqlEntry-based composition and typed parameter containers.
95-98: LGTM!The conversion of the orderBy string to
Option[SqlEntry]via.toOptioncorrectly handles empty strings, and the parameter values are properly combined for query execution.
108-131: LGTM!The immutable pattern is correctly maintained -
setParamandclearreturn new instances rather than mutating state.
163-166: LGTM!The
sealed case classpattern is valid here - it provides value equality and copy methods while preventing external subclassing. The factory methods in the companion object correctly wrap string function names inSqlEntry.Also applies to: 199-202
41-48: No action needed—ORDER BY implementation correctly handles None parameters.The logic is sound. When
orderByisNone,BY(None)creates anOrderByFragmentwithsqlEntry = None, andSqlEntry + Option[SqlEntry]correctly returns the original SQL unchanged without appending an ORDER BY clause.
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala
Outdated
Show resolved
Hide resolved
benedeki
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (1)
121-128: Consider using mkSqlEntry for consistency.The manual string construction at lines 125-126 could use the
mkSqlEntryhelper method for consistency with the rest of the DSL.Apply this diff to use
mkSqlEntry:private def columnsToSqlEntry(fields: Seq[ColumnReference]): Option[SqlEntry] = { if (fields.isEmpty) { None } else { - val fieldEntries = fields.map(_.sqlEntry.entry) - Some(SqlEntry(fieldEntries.mkString(", "))) + Some(fields.map(_.sqlEntry).mkSqlEntry(", ")) } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala(1 hunks)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala (2)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (2)
SqlEntry(23-58)SqlEntry(60-113)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (1)
sql(41-48)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (11)
SqlEntry(23-58)SqlEntry(60-113)apply(35-38)apply(47-50)apply(61-61)apply(62-65)apply(67-67)mkSqlEntry(82-84)mkSqlEntry(85-88)prefix(70-70)toOption(52-54)
🔇 Additional comments (16)
balta/src/main/scala/za/co/absa/db/balta/classes/DBQuerySupport.scala (3)
20-20: LGTM!The import is necessary to support the SqlEntry-based API migration.
30-30: LGTM!Correctly extracts the underlying SQL string from the
SqlEntrywrapper using.entry.
27-27: Signature change fromStringtoSqlEntryandListtoVectoris appropriate and properly implemented.Vector provides random access and update in effectively constant time, which is more suitable than List for indexed access patterns in query execution. All callers (DBFunction and DBTable) have been correctly updated to pass
Vector[QueryParamValue]instances, confirming the API change is complete across the codebase.balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (13)
1-5: LGTM!Package declaration and imports are appropriate for the SQL composition DSL.
8-15: LGTM!The
SelectFragmentdesign correctly enforces at least one field in the SELECT clause through thefirstFieldparameter, preventing empty SELECT statements.
17-31: LGTM!The
InsertFragmentandQueryInsertIntoFragmentcorrectly enforce at least one value in the VALUES clause and provide convenient overloads.
33-35: LGTM!The
DeleteFragmentprovides a straightforward DELETE FROM builder.
37-39: LGTM!The private singleton pattern is appropriate for the fragment builders.
41-43: LGTM!The
SelectWithFieldsFragmentcorrectly chains FROM after SELECT fields.
45-47: LGTM!The
OrderByFragmentcorrectly handles optional ORDER BY clauses using theprefixhelper.
49-52: LGTM!The
OrderByMixIntrait provides reusable ORDER BY functionality through composition.
54-63: LGTM!The
ReturningMixIntrait correctly provides RETURNING clause support with type-safe field handling.
65-96: LGTM!The sealed class hierarchy effectively models SQL query states and transitions, enforcing correct SQL structure through types. The WHERE clause handling correctly supports both mandatory and optional conditions.
98-101: LGTM!The
SqlEntryConstantdesign correctly encapsulates SQL constants with controlled instantiation.
103-108: LGTM!The builder accessor methods provide a clean API for SQL composition with convenient overloads for ORDER BY.
113-119: LGTM!Caching SQL keyword fragments is a good optimization and improves consistency.
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala
Outdated
Show resolved
Hide resolved
…-names-are-not-case-insensitive' into feature/62-make-composition-of-sql-expression-easier-and-safer # Conflicts: # balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala # balta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scala
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (1)
112-112: Good security practice: implicit String conversion is disabled.The commented-out
StringToSqlEntryimplicit conversion with a TODO is the correct approach. This prevents developers from accidentally passing unsanitized user input as string literals, which would bypass the type-safe parameter handling.Note: A similar implicit conversion exists in
SqlEntry.scala(line 91). Consider aligning both files by either:
- Disabling it in both places, or
- Adding strong documentation warnings in
SqlEntry.scalaif keeping it thereBased on the past review comment on this file, the security implications of implicit String conversions have already been raised. The current state (commented out) reflects good judgment.
🧹 Nitpick comments (5)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (3)
33-33: Consider usingequalsinstead of overriding==.Overriding the
==operator in Scala can lead to confusion because it doesn't align withequals()andhashCode(). The standard pattern is to overrideequalsandhashCodein the companion object for value classes.However, since this is a value class extending
AnyVal, custom equality is limited. The current implementation compares string content, which may be acceptable for this use case.📋 Consider this alternative if referential equality is needed
If you need both string-based and referential equality:
def contentEquals(other: String): Boolean = this.entry == otherThen users can call
sqlEntry.contentEquals(str)explicitly.
47-50: Document the workaround for type erasure.The
fooparameter is a clever workaround for JVM type erasure, but the current comment could be clearer. Consider renaming to something more descriptive likedisambiguatoror adding a link to the erasure issue.📋 Suggested improvement
/** Translates a sequence of SqlEntry entries into a single SqlEntry formatted as a parameter list * * @param params - A sequence of SqlEntry to be included as parameters. - * @param foo - A bogus parameter do differentiate this method from the `apply(params: String*)` method, as their - * signatures would be same otherwise after type erasure. + * @param disambiguator - Unused parameter to differentiate this method from `apply(params: String*)` + * after JVM type erasure (both would have signature `apply(Seq)`). * @return A new SqlEntry that combines the input as a list of parameters/columns for a function or table. */ - def apply(params: Seq[SqlEntry], foo: String = ""): SqlEntry = { + def apply(params: Seq[SqlEntry], disambiguator: String = ""): SqlEntry = {
103-110: Consider edge cases in the spacing heuristic.The spacing logic handles common cases well, but may have edge cases:
- What about operators like
=,<,>, etc.?- What about string literals or quoted identifiers that contain these characters?
Since
SqlEntryis a simple string wrapper, it can't distinguish between SQL keywords and string literals. This is acceptable ifSqlEntryis only used for structural SQL parts (not user data), but worth documenting.Consider adding a comment explaining that
SqlEntryshould only contain SQL structure (keywords, operators, identifiers), not user data values (which should go through parameters).balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (1)
40-43: Suggest improving the error message for better user experience.The error message currently shows the normalized form (
columnNameQuoteless) rather than the original input. This could confuse users who provided a different casing or quoted name.🔎 Proposed improvement
def columnNumber(columnName: String): Int = { val columnNameQuoteless = ColumnName(columnName).quoteLess - columnNames.getOrThrow(columnNameQuoteless, new NoSuchElementException(s"Column '$columnNameQuoteless' not found")) + columnNames.getOrThrow(columnNameQuoteless, new NoSuchElementException(s"Column '$columnName' not found (normalized to '$columnNameQuoteless')")) }balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (1)
62-62: Consider clarifying the deprecation message.The deprecation message references
NULL, but users need to importQueryParamType.NULLto use it. Consider making this explicit in the message.🔎 Suggested improvement
-@deprecated("Use add(paramName, NULL)", "balta 0.3.0") +@deprecated("Use add(paramName, QueryParamType.NULL) or import QueryParamType.NULL", "balta 0.3.0")-@deprecated("Use add(NULL)", "balta 0.3.0") +@deprecated("Use add(QueryParamType.NULL) or import QueryParamType.NULL", "balta 0.3.0")Also applies to: 86-86
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scalabalta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scalabalta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scalabalta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scalabalta/src/main/scala/za/co/absa/db/balta/implicits/QueryResultRowImplicits.scalabalta/src/main/scala/za/co/absa/db/balta/postgres/implicits/Postgres.scalabalta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scalabalta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scalabalta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scalabalta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scalabalta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scalabalta/src/test/scala/za/co/absa/db/balta/implicits/PostgresRowIntegrationTests.scalabalta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scalabalta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala
🚧 Files skipped from review as they are similar to previous changes (4)
- balta/src/test/scala/za/co/absa/db/mag/core/SqlEntryCompositionUnitTests.scala
- balta/src/test/scala/za/co/absa/db/mag/core/ColumnReferenceUnitTests.scala
- balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala
- balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala
🧰 Additional context used
🧬 Code graph analysis (8)
balta/src/main/scala/za/co/absa/db/balta/postgres/implicits/Postgres.scala (2)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (4)
columnNumber(40-43)getArray(118-123)getArray(125-125)getArray(127-138)balta/src/main/scala/za/co/absa/db/balta/postgres/classes/SimpleJsonString.scala (2)
SimpleJsonString(29-29)SimpleJsonString(31-37)
balta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scala (1)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (5)
QueryResultRow(35-139)QueryResultRow(141-185)columnNamesFromMetadata(153-155)getOffsetDateTime(109-109)getOffsetDateTime(110-110)
balta/src/test/scala/za/co/absa/db/balta/implicits/PostgresRowIntegrationTests.scala (1)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (3)
QueryResultRow(35-139)QueryResultRow(141-185)columnNamesFromMetadata(153-155)
balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (6)
balta/src/main/scala/za/co/absa/db/balta/DBTestSuite.scala (3)
table(88-90)add(122-124)add(144-146)balta/src/main/scala/za/co/absa/db/balta/classes/DBTable.scala (7)
delete(163-165)delete(167-169)DBTable(31-250)where(84-86)where(97-99)where(109-111)where(122-124)balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (6)
Params(29-36)Params(37-175)add(51-53)add(75-77)add(105-109)add(149-153)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (1)
NULL(86-86)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala (2)
next(50-58)noMore(63-63)balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (4)
getLong(86-86)getLong(87-87)getString(80-80)getString(81-81)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala (1)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (3)
QueryResultRow(35-139)QueryResultRow(141-185)columnNamesFromMetadata(153-155)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (4)
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (6)
core(40-40)core(41-41)core(42-45)apply(47-66)apply(68-70)apply(78-78)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (2)
apply(9-12)apply(14-14)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (3)
apply(143-145)apply(147-149)apply(151-153)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)
balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (2)
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (9)
core(40-40)core(41-41)core(42-45)ColumnName(21-32)ColumnName(77-79)ColumnReference(34-75)apply(47-66)apply(68-70)apply(78-78)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (16)
QueryParamType(32-92)NULL(86-86)toQueryParamValue(35-35)toQueryParamValue(39-39)toQueryParamValue(43-43)toQueryParamValue(47-47)toQueryParamValue(51-51)toQueryParamValue(55-55)toQueryParamValue(59-59)toQueryParamValue(63-63)toQueryParamValue(67-67)toQueryParamValue(71-71)toQueryParamValue(75-75)toQueryParamValue(79-79)toQueryParamValue(83-83)toQueryParamValue(89-89)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (2)
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (5)
core(40-40)core(41-41)core(42-45)ColumnName(21-32)ColumnName(77-79)balta/src/main/scala/za/co/absa/db/balta/implicits/MapImplicits.scala (1)
getOrThrow(28-30)
🔇 Additional comments (20)
balta/src/main/scala/za/co/absa/db/balta/implicits/QueryResultRowImplicits.scala (1)
75-89: LGTM! Consistent naming convention applied.The rename from
columnLabeltocolumnNamealigns with the broader API changes throughout the codebase. The TODO comment on Line 82 appropriately flags the unusedTypeTagcontext bound for future cleanup.balta/src/test/scala/za/co/absa/db/balta/implicits/PostgresRowIntegrationTests.scala (1)
32-46: LGTM! Test updated to reflect API rename.The test correctly calls
columnNamesFromMetadatainstead of the deprecatedfieldNamesFromMetadata. Test expectations remain unchanged, confirming behavioral consistency.balta/src/main/scala/za/co/absa/db/balta/postgres/implicits/Postgres.scala (1)
28-32: LGTM! Public API parameter rename is consistent.The parameter rename from
columnLabeltocolumnNamealigns with the updatedQueryResultRowAPI and improves naming clarity.balta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scala (2)
32-54: LGTM! Test updated with typo fix.The test correctly uses
columnNamesFromMetadataand fixes a typo in the variable name (expecedResult→expectedResult). Test behavior is preserved.
195-197: LGTM! Variable naming improved.The rename from
resultOfColumnLabeltoresultOfColumnNameis consistent with the API changes.balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scala (3)
68-70: LGTM! Test data updated.The return value change is a simple test data update with no functional impact.
95-95: LGTM! Improved NULL handling.Using
Params.add("text_field", NULL)is more type-safe and idiomatic than raw SQL string manipulation.
176-236: Excellent test coverage for special column names.This test validates the new
SqlEntryandColumnReferencehandling for edge cases including:
- Quoted identifiers (
"Col1")- Mixed-case names (automatically lowercased)
- Names with spaces, hyphens, special characters, and starting with digits
The test correctly exercises both parameter-based insertion and retrieval, ensuring the quoting logic works bidirectionally.
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResult.scala (2)
19-19: LGTM! Type alias updated consistently.The import change from
FieldNamestoColumnNamesreflects the updated public API fromQueryResultRow.
34-34: LGTM! Initialization updated to use renamed method.The call to
columnNamesFromMetadatais consistent with the updatedQueryResultRowAPI.balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (2)
8-97: Well-structured fluent SQL DSL.The composition API provides a type-safe way to construct SQL queries with:
- Clear separation between query stages (SELECT...FROM, WHERE, ORDER BY, etc.)
- Mix-in traits for reusable clauses (OrderByMixIn, ReturningMixIn)
- Sealed class hierarchy preventing invalid query states
The design enforces correct SQL syntax at compile time, which is the primary goal stated in the PR objectives.
111-111: The implicit conversion is safe in practice given the sealed type hierarchy.The implicit
QueryToSqlEntryconversion cannot hide type errors because theQuerytype and all its subtypes are sealed, preventing unexpected implementations. Additionally, the codebase does not differentiate between specificQuerysubtypes—all methods accept the baseQueryclass and inherit the samesqlEntryproperty. The conversion is a simple property delegation with no loss of type information.balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (1)
91-91: The implicit conversion direction is reversed in this comment.Line 91 contains
implicit def sqlEntryToString(sqlEntry: SqlEntry): String, which convertsSqlEntrytoString— notStringtoSqlEntryas stated in the review. This conversion is safe and does not enable SQL injection: it only allows existingSqlEntryobjects to be used where strings are expected, such as for logging or string concatenation.The inverse conversion (
StringToSqlEntry) that would pose the security concern you've identified is commented out atSqlEntryComposition.scalaline 112 with a TODO, confirming the team is aware of the risk and has explicitly disabled it. No implicit conversion from raw strings toSqlEntryexists in the active codebase.Likely an incorrect or invalid review comment.
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (3)
21-21: LGTM! Import aligns with the new column handling.The import of
ColumnNamesupports the typed column reference approach introduced in this PR.
33-37: LGTM! Improved naming clarity.Renaming
columnLabelstocolumnNamesbetter reflects the intent and aligns with SQL terminology.
53-125: LGTM! Consistent parameter renaming across all accessor methods.All getter methods have been consistently updated to use
columnNameinstead ofcolumnLabel, and properly delegate to the updatedcolumnNumbermethod.balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (4)
21-36: LGTM! Excellent sealed hierarchy design.The sealed abstract class with a private constructor and well-defined abstract methods creates a type-safe, controlled API. The new imports support the typed parameter handling introduced in this PR.
39-40: LGTM! Type-safe parameter storage.The use of
ColumnNameandColumnReferenceas keys instead of raw strings provides compile-time safety and aligns with the PR's goal of safer SQL composition.
96-135: LGTM! Type-safe named parameter handling.The use of
ColumnNameas a map key with properequals/hashCodebased onsqlEntryensures that different representations of the same column name (e.g., "MyColumn" vs "mycolumn") are treated as equivalent. The newparamNamesmethod is a useful addition for introspection.
141-174: LGTM! Ordered parameter handling is correct.The implementation correctly generates 1-based
ColumnIndexkeys for SQL positioning while providing 0-based Vector access throughapply(position). The separation between SQL parameter positions (1-based) and programmatic access (0-based) is appropriate.
| type ColumnNames = Map[String, Int] | ||
| type TransformerFnc[T] = Object => T | ||
| type Extractor = ResultSet => Option[Object] | ||
| type Extractors = Vector[Extractor] | ||
|
|
||
| def apply(resultSet: ResultSet)(implicit fieldNames: FieldNames, extractors: Extractors): QueryResultRow = { | ||
| def apply(resultSet: ResultSet)(implicit columnNames: ColumnNames, extractors: Extractors): QueryResultRow = { | ||
| val fields = extractors.map(_(resultSet)) | ||
| new QueryResultRow(resultSet.getRow, fields, fieldNames) | ||
| new QueryResultRow(resultSet.getRow, fields, columnNames) | ||
| } | ||
|
|
||
| def fieldNamesFromMetadata(metaData: ResultSetMetaData): FieldNames = { | ||
| def columnNamesFromMetadata(metaData: ResultSetMetaData): ColumnNames = { | ||
| Range.inclusive(1, metaData.getColumnCount).map(i => metaData.getColumnName(i) -> i).toMap | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the QueryResultRow.scala file and examine the relevant code
fd -e scala -path "*QueryResultRow*"Repository: AbsaOSS/balta
Length of output: 225
🏁 Script executed:
# Search for the ColumnName class and its quoteLess implementation
fd -e scala | xargs rg -l "ColumnName" | head -20Repository: AbsaOSS/balta
Length of output: 492
🏁 Script executed:
# Look for test files related to QueryResultRow
fd -e scala -path "*test*" | xargs rg -l "QueryResultRow" 2>/dev/nullRepository: AbsaOSS/balta
Length of output: 1111
🏁 Script executed:
# Read the QueryResultRow.scala file
cat -n balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scalaRepository: AbsaOSS/balta
Length of output: 9930
🏁 Script executed:
# Search for ColumnName class definition and quoteLess method
rg -n "class ColumnName|def quoteLess|object ColumnName" balta/src/main/scala/Repository: AbsaOSS/balta
Length of output: 258
🏁 Script executed:
# Look for quoted column name tests
rg -n -i "quoted|special|\".*[A-Z]" balta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scalaRepository: AbsaOSS/balta
Length of output: 4021
🏁 Script executed:
# Read the ColumnName class to understand quoteLess implementation
cat -n balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala | head -100Repository: AbsaOSS/balta
Length of output: 3855
🏁 Script executed:
# Search for any column name transformation or normalization
rg -n "quoteLess|getColumnName" balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scalaRepository: AbsaOSS/balta
Length of output: 221
🏁 Script executed:
# Check how columnNamesFromMetadata is used in tests or other code
rg -n "columnNamesFromMetadata" balta/src/Repository: AbsaOSS/balta
Length of output: 968
🏁 Script executed:
# Check if there are any tests with mixed-case or quoted column names
rg -n "test.*[A-Z]|\".*[A-Z]" balta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scala | head -30Repository: AbsaOSS/balta
Length of output: 1478
🏁 Script executed:
# Look for the full integration test file
cat -n balta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scala | head -60Repository: AbsaOSS/balta
Length of output: 2303
🏁 Script executed:
# Search for any other uses of columnNamesFromMetadata or related setup
rg -B5 -A5 "columnNamesFromMetadata" balta/src/test/scala/za/co/absa/db/balta/implicits/PostgresRowIntegrationTests.scalaRepository: AbsaOSS/balta
Length of output: 497
🏁 Script executed:
# Look for all column names in all tests to see if any are mixed case or quoted
rg -n "test.*[A-Z_]|\"[A-Z]" balta/src/test/scala/za/co/absa/db/balta/Repository: AbsaOSS/balta
Length of output: 14142
🏁 Script executed:
# Search for table definitions or schema creation to understand what column names exist
fd -e sql -o -e scala | xargs rg -l "CREATE TABLE|column" | head -5Repository: AbsaOSS/balta
Length of output: 521
🏁 Script executed:
# Check if there are any schema or migration files
fd -e sql balta/src/test/Repository: AbsaOSS/balta
Length of output: 422
🏁 Script executed:
# Look at the test with strange column names
sed -n '176,230p' balta/src/test/scala/za/co/absa/db/balta/classes/DBTableIntegrationTests.scalaRepository: AbsaOSS/balta
Length of output: 2112
🏁 Script executed:
# Look at the schema for the strange columns table
cat balta/src/test/resources/db/postgres/10_testing.strange_columns.sqlRepository: AbsaOSS/balta
Length of output: 1132
🏁 Script executed:
# Look at the strange columns data insertion
cat balta/src/test/resources/db/postgres/11_testing.strange_columns_data.sqlRepository: AbsaOSS/balta
Length of output: 823
🏁 Script executed:
# Check if there's a mismatch - examine how ColumnName.quoteLess handles unquoted vs quoted names
python3 << 'EOF'
# Simulate the ColumnName behavior based on the patterns I found
def simulate_column_name_quoteless(name):
"""Simulate ColumnName.quoteLess behavior"""
trimmed = name.strip()
# Regular lowercase pattern: ^([a-z_][a-z0-9_]*)$
if all(c.islower() or c.isdigit() or c == '_' for c in trimmed) and trimmed and (trimmed[0].isalpha() or trimmed[0] == '_'):
return trimmed
# Mixed case pattern: ^[a-zA-Z_][a-zA-Z0-9_]*$
if all(c.isalpha() or c.isdigit() or c == '_' for c in trimmed) and trimmed and (trimmed[0].isalpha() or trimmed[0] == '_'):
return trimmed.lower()
# Quoted regular pattern: ^\"([a-z_][a-z0-9_]*)\"$
if trimmed.startswith('"') and trimmed.endswith('"'):
unquoted = trimmed[1:-1]
if all(c.islower() or c.isdigit() or c == '_' for c in unquoted) and unquoted and (unquoted[0].isalpha() or unquoted[0] == '_'):
return unquoted
# Quoted pattern with any content: return unescaped
return unquoted.replace('""', '"')
# Other: quote and escape
return trimmed
# Test cases from the strange_columns table
test_cases = [
("col1", "col1 (unquoted lowercase)"),
("Col1", "Col1 (unquoted mixed case)"),
('"Col1"', '"Col1" (quoted mixed case)'),
('"col 1"', '"col 1" (quoted with space)'),
('"col-1"', '"col-1" (quoted with dash)'),
('"colá"', '"colá" (quoted with accent)'),
('"1col"', '"1col" (quoted, starts with digit)'),
]
print("ColumnName.quoteLess simulation:")
for name, desc in test_cases:
result = simulate_column_name_quoteless(name)
print(f" {desc:40} → {result}")
# PostgreSQL JDBC metadata behavior
print("\nPostgreSQL JDBC getColumnName() behavior:")
pg_metadata_returns = {
"col1": "col1", # unquoted → lowercase
'"Col1"': "Col1", # quoted → unquoted, case-preserved
'"col 1"': "col 1", # quoted → unquoted, space preserved
'"col-1"': "col-1", # quoted → unquoted, dash preserved
'"colá"': "colá", # quoted → unquoted, accent preserved
'"1col"': "1col", # quoted → unquoted, leading digit
}
for original, pg_returns in pg_metadata_returns.items():
print(f" {original:15} → JDBC returns: {pg_returns}")
quoteless = simulate_column_name_quoteless(original)
match = "✓ MATCH" if quoteless == pg_returns else "✗ MISMATCH"
print(f" ColumnName.quoteLess: {quoteless:15} {match}")
EOFRepository: AbsaOSS/balta
Length of output: 143
🏁 Script executed:
# Verify the actual behavior by checking how the test passes - look at what metadata keys are created
rg -B10 -A10 "columnNamesFromMetadata" balta/src/test/scala/za/co/absa/db/balta/classes/QueryResultRowIntegrationTests.scalaRepository: AbsaOSS/balta
Length of output: 789
🏁 Script executed:
# Check if there are any tests or usage that shows accessing mixed-case unquoted columns
rg -n "getString\(\"[A-Z]" balta/src/test/scala/za/co/absa/db/balta/Repository: AbsaOSS/balta
Length of output: 39
🏁 Script executed:
# Verify the actual JDBC getColumnName behavior by checking any database-specific documentation or comments
rg -n "getColumnName|metadata|JDBC|ResultSetMetaData" balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scalaRepository: AbsaOSS/balta
Length of output: 376
🏁 Script executed:
# Search for any handling of quoted identifiers in the codebase
rg -n "quoteLess|getColumnName" balta/src/main/scala/za/co/absa/db/balta/classes/ -B2 -A2Repository: AbsaOSS/balta
Length of output: 1245
Apply ColumnName.quoteLess consistently in columnNamesFromMetadata.
The columnNamesFromMetadata method stores raw JDBC metadata column names as map keys, while the columnNumber lookup method applies ColumnName.quoteLess transformation to user input. This mismatch works for PostgreSQL only because the transformation happens to align with JDBC's metadata output (unquoted identifiers as lowercase, quoted identifiers unquoted with case preserved). However, this relationship is implicit and not enforced by the type system. To make the code more robust and database-agnostic, apply ColumnName.quoteLess to all column names when building the map, ensuring consistency between storage and lookup logic.
🤖 Prompt for AI Agents
In balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala around
lines 143 to 155, columnNamesFromMetadata currently uses raw JDBC column names
as map keys while lookups use ColumnName.quoteLess, causing inconsistent
behavior across databases; update columnNamesFromMetadata to apply
ColumnName.quoteLess to each metaData.getColumnName(i) when building the Map so
stored keys match the lookup normalization (i.e., transform the metadata name
with ColumnName.quoteLess before using it as the Map key).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (1)
40-43: ApplyColumnName.quoteLessconsistently incolumnNamesFromMetadata.The
columnNumbermethod appliesColumnName.quoteLessto the lookup key (line 41), butcolumnNamesFromMetadata(line 154) stores raw JDBC metadata column names. This inconsistency works for PostgreSQL due to alignment between JDBC's behavior and the quoteLess transformation, but the relationship is implicit and database-dependent. ApplyingColumnName.quoteLesswhen building the map incolumnNamesFromMetadatawould ensure robust, database-agnostic behavior.🔎 Recommended fix
def columnNamesFromMetadata(metaData: ResultSetMetaData): ColumnNames = { - Range.inclusive(1, metaData.getColumnCount).map(i => metaData.getColumnName(i) -> i).toMap + Range.inclusive(1, metaData.getColumnCount).map(i => ColumnName(metaData.getColumnName(i)).quoteLess -> i).toMap }
🧹 Nitpick comments (1)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (1)
33-33: Consider clarifying the custom==operator behavior.The
==method compares only theentryfield, which works for direct comparisons but doesn't integrate with Scala's equality contract. This won't affect usage in collections or pattern matching since value classes use structural equality by default. However, the custom==method may create confusion about which equality semantics apply in different contexts.If the intent is purely string comparison, consider naming this method differently (e.g.,
matches,equalsString) to avoid confusion with standard equality:🔎 Alternative naming suggestion
- def ==(other: String): Boolean = this.entry == other + def matches(other: String): Boolean = this.entry == other
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scalabalta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scalabalta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scalabalta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala
🧰 Additional context used
🧬 Code graph analysis (3)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (4)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (3)
apply(50-50)apply(56-56)apply(148-151)balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (2)
apply(9-12)apply(14-14)balta/src/main/scala/za/co/absa/db/balta/classes/DBFunction.scala (3)
apply(143-145)apply(147-149)apply(151-153)balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamValue.scala (1)
sqlEntry(26-26)
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (3)
balta/src/main/scala/za/co/absa/db/mag/core/ColumnReference.scala (8)
core(40-40)core(41-41)core(42-45)ColumnName(21-32)ColumnName(77-79)apply(47-66)apply(68-70)apply(78-78)balta/src/main/scala/za/co/absa/db/balta/implicits/MapImplicits.scala (1)
getOrThrow(28-30)balta/src/main/scala/za/co/absa/db/balta/implicits/OptionImplicits.scala (1)
getOrThrow(27-29)
balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (1)
balta/src/main/scala/za/co/absa/db/balta/typeclasses/QueryParamType.scala (16)
QueryParamType(32-92)NULL(86-86)toQueryParamValue(35-35)toQueryParamValue(39-39)toQueryParamValue(43-43)toQueryParamValue(47-47)toQueryParamValue(51-51)toQueryParamValue(55-55)toQueryParamValue(59-59)toQueryParamValue(63-63)toQueryParamValue(67-67)toQueryParamValue(71-71)toQueryParamValue(75-75)toQueryParamValue(79-79)toQueryParamValue(83-83)toQueryParamValue(89-89)
🔇 Additional comments (10)
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntry.scala (2)
47-50: Disambiguator parameter serves type-erasure workaround.The
disambiguatorparameter exists solely to differentiate this overload fromapply(params: String*)after JVM type erasure. While documented, this pattern can confuse callers who might pass values for this parameter.The documentation clearly explains the purpose, and the default empty string makes it transparent to callers.
91-91: Implicit String conversion is convenient but use with awareness.The implicit conversion from
SqlEntrytoStringprovides convenient interoperability but may mask type safety in some scenarios. SinceSqlEntryis a value class wrapping a string, the conversion is straightforward and the performance impact is minimal.This design choice aligns well with the fluent API goals of the DSL.
balta/src/main/scala/za/co/absa/db/mag/core/SqlEntryComposition.scala (3)
24-27: String-based VALUES method requires care with user input.The
VALUES(firstValue: String, otherValues: String*)method wraps strings directly inSqlEntry. While convenient for literals, this could enable SQL injection if user input is passed directly. The saferVALUES(values: Seq[SqlEntry])overload at lines 28-31 should be preferred when composing values from external sources.Ensure coding guidelines emphasize using parameterized queries or the
Seq[SqlEntry]overload for dynamic values.
111-111: Implicit Query to SqlEntry conversion is safe and ergonomic.The implicit conversion extracts the
sqlEntryfield from aQuery, enabling seamless use of query builders in contexts expectingSqlEntry. This is safe since it doesn't construct SQL from arbitrary input.
8-97: Well-structured DSL with type-safe query building.The sealed class hierarchy and stage-based builders (SelectFragment → SelectWithFieldsFragment → QuerySelect) provide compile-time safety, preventing invalid SQL construction paths. The use of mixins for ORDER BY and RETURNING clauses enables clean reuse across query types.
balta/src/main/scala/za/co/absa/db/balta/classes/QueryResultRow.scala (1)
21-21: API renaming improves clarity and aligns with SQL terminology.The transition from
FieldNames/columnLabeltoColumnNames/columnNamebetter reflects standard SQL terminology and improves code readability. The import ofColumnNameutility at line 21 supports the new typed column handling.Also applies to: 33-37
balta/src/main/scala/za/co/absa/db/balta/classes/inner/Params.scala (4)
29-40: Refactored parameter model provides unified position-based access.The abstract
Paramsclass now exposes a consistent interface with position-based access (apply(position: Int)), while concrete implementations (NamedParams,OrderedParams) manage their internal storage differently. The type aliasesNamedParamsMapandParamsListimprove code clarity.
96-131: NamedParams now uses typed ColumnName keys for safer parameter handling.The migration from string keys to
ColumnNamekeys (lines 106, 123) provides stronger typing and consistent column name normalization. TheparamNamesaccessor (line 130) exposes the typed keys, enabling better introspection.
141-174: OrderedParams provides position-indexed parameter storage.The
OrderedParamsimplementation usesColumnReference(size + 1)to create positional references (line 150), maintaining a clean vector-based storage model. The companion object factory (lines 172-174) simplifies construction.
62-62: Deprecation notices guide migration to QueryParamType.NULL.The
addNullmethods are properly deprecated with clear migration guidance pointing toQueryParamType.NULL. This maintains backward compatibility while encouraging the new pattern.Also applies to: 86-86, 118-118, 162-162
…d-safer' of https://github.com/AbsaOSS/balta into feature/62-make-composition-of-sql-expression-easier-and-safer
benedeki
changed the base branch from
master
to
bugfix/1-field-parameter-names-are-not-case-insensitive
|
|
||
| test("Composition of `INSERT INTO ...(...) VALUES ... RETURNING ...` using field names") { | ||
| val fields = SqlEntry(field1, field2) | ||
| val query = INSERT INTO tableName(fields.entry) VALUES("100, 'Sample Text'") RETURNING(ALL) |
Overview
SqlEntrywhich represents a part of and SQL expressionSqlEntrybrings a number of methods and helper classesSqlEntryCompositionoffers a way to compose a full SQL expression asSqlEntryin SQL format (almost)SqlEntryis now leveraged inSqlItem,DBFunctionandDBTableRelease Notes
SELECT (all) FROM my_table WHERE condition ORDER BY (field1)Related
Closes #62
Depends on #61
Summary by CodeRabbit
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.