jhc-systems / sqlest

Write SQL in Scala
https://jhc-systems.github.io/sqlest/latest/api/
Apache License 2.0
30 stars 17 forks source link

Cast null to a type when it is a result of an empty Option (necessary… #79

Closed joezini closed 7 years ago

joezini commented 7 years ago

… for DB2 SQL)

Casting only seemed to be necessary for Constant Columns that were options. The addTypingToLiteralColumn method (formerly addTypingToSqlColumn) seems to be necessary because to keep existing tests passing we should only be casting literals that are inside scalar functions and joins, so this function is called from those cases only.

I wanted to use OptionColumnType#isNull to check whether the column's value is null rather than checking explicitly for equality with None or null, but it always threw these type errors when I passed the column's value to that method:

[error]  found   : constantColumn.value.type (with underlying type _)
[error]  required: optionColumnType.baseColumnType.type
codecov-io commented 7 years ago

Codecov Report

Merging #79 into master will increase coverage by 0.13%. The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   81.69%   81.82%   +0.13%     
==========================================
  Files          43       43              
  Lines        1147     1150       +3     
  Branches      105       98       -7     
==========================================
+ Hits          937      941       +4     
+ Misses        210      209       -1
Impacted Files Coverage Δ
...rc/main/scala/sqlest/sql/DB2StatementBuilder.scala 83.6% <88.88%> (+2.57%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7e90855...e99cce0. Read the comment docs.

joezini commented 7 years ago

It turned out this change didn't work so I'm gonna close the pull request - it failed a test in VersioningSpec of AS by casting nulls that had already been cast one, and still failing to cast nulls that are the default value of an Option column that has been versioned at the column level.

Perhaps a better place for the fix is in VersionedStatementBuilder (in AS) instead? I started on this but hadn't got anything working yet, and am now being taken off the project so can't work on it any more :(

DavidGregory084 commented 7 years ago

Okay, thanks @joezini - hopefully we can get back to this soon