Extended query tests

[cosminci]

Test parameterized w/ list fails. It's equivalent to the second test which uses the legacy twiddle syntax. The first test passes so the issue seems to appear when list (maybe other?) codec is involved.

[mpilquist]

I narrowed this down to the following example:

val params: continents.type *: Int *: EmptyTuple = continents *: 10_000_000 *: EmptyTuple

Or equivalently, after dealiasing *: and shapeless.HNil:

val params: continents.type :: Int :: HNil = continents :: 10_000_000 :: HNil

This fails with the following error:

ExtendedQueryTest.scala:58:50: type mismatch;
[error]  found   : rassoc$6.type (with underlying type List[String])
[error]  required: continents.type
[error]     val params: continents.type :: Int :: HNil = continents :: 10_000_000 :: HNil
[error]                 

We can work around this by manually specifying the type to cons:

val params: continents.type :: Int :: HNil =
    new ::[continents.type, Int :: HNil](continents, 10_000_000 :: HNil)

Or even letting those types be inferred while manually instantiating the cons constructor:

val params: continents.type :: Int :: HNil = new ::(continents, 10_000_000 :: HNil)

So this appears to be a bug in the Scala 2 compiler when handling right associative operators. Appears to be a manifestation of https://github.com/scala/bug/issues/11117.

[milessabin]

I don't think this is a compiler bug. The desugaring for right associated calls involves an assignment to an intermediate val (rassoc$6 in this case) and this is enough to break the identity you're expecting. You can reproduce something similar in both Scala 2 and 3,

miles@tarski:~% scala-cli --java-home /usr/lib/jvm/java-20-jdk
Welcome to Scala 3.3.0 (20, Java Java HotSpot(TM) 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> val foo = new Object
val foo: Object = java.lang.Object@5cc1bf20

scala> val bar = foo
val bar: Object = java.lang.Object@5cc1bf20

scala> val baz: bar.type = foo
-- [E007] Type Mismatch Error: -------------------------------------------------
1 |val baz: bar.type = foo
  |                    ^^^
  |                    Found:    (foo : Object)
  |                    Required: (bar : Object)
  |
  | longer explanation available when compiling with `-explain`
1 error found

scala> val quux: foo.type = foo
val quux: foo.type = java.lang.Object@5cc1bf20

So, unfortunately the name (ie. the precise Symbol) that the value is presented as matters and has to match between the value and the singleton type.

[mpilquist]

For completeness, here's the working test:

// Uses import skunk._ for *: and EmptyTuple polyfills; can replace with :: and HNil if preferred
sessionTest("parameterized w/ list") { s =>
  val continents = List("Europe", "Asia")
  val query = sql"""
    SELECT name, region FROM country
    WHERE continent IN (${varchar.list(continents)})
      AND population > $int4
  """.query(varchar *: varchar)

  val countryStream = for {
    preparedQuery <- Stream.eval(s.prepare(query))
    country <- preparedQuery.stream(new *:(continents, 10_000_000 *: EmptyTuple), chunkSize = 5)
  } yield country

  countryStream.compile.toList.map(_ => "ok")
}

[cosminci]

Thanks for looking into this @mpilquist!

I've updated the test to use the explicit twiddle constructor. Also changed the first test to use twiddles instead of cons and removed the shapeless._ import.

LMK if we want this merged, or rather add some examples to the docs, or both.

[cosminci]

P.S.: If the list codec is not the first, new *: constructors need to be chained all the way up starting from it, even if the previous codecs are simple.

[cosminci]

@mpilquist do we eventually want to merge this suite for regression purposes, or should I close the PR since it's achieved its purpose?

[mpilquist]

Yeah, I think we should include.

[codecov-commenter]

Codecov Report

Merging #898 (cbc1515) into main (d21cffb) will increase coverage by 0.05%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #898      +/-   ##
==========================================
+ Coverage   83.89%   83.94%   +0.05%     
==========================================
  Files         125      125              
  Lines        1757     1757              
  Branches      211      211              
==========================================
+ Hits         1474     1475       +1     
+ Misses        283      282       -1     

see 1 file with indirect coverage changes

[cosminci]

My bad, should have ran the tests locally for both 2.13 and 3.

Compilations fails on Scala 3 with:

[error] -- Error: /home/runner/work/skunk/skunk/modules/tests/shared/src/test/scala/ExtendedQueryTest.scala:59:45 
[error] 59 |      country <- preparedQuery.stream(new *:(continents, 10_000_000 *: EmptyTuple), chunkSize = 5)
[error]    |                                             ^^^^^^^^^^
[error]    |     too many arguments for constructor *: in class *:: (): Any *: Tuple

Trying to find a workaround.

[cosminci]

I couldn't find a way to please both Scala 2.13 and Scala 3 compilers.

val params: continents.type :: Int :: HNil =
    new ::[continents.type, Int :: HNil](continents, 10_000_000 :: HNil)

^ This isn't accepted by the Scala 3 compiler :(

[error]    |too many arguments for constructor *: in class *:: (): ((continents : List[String]), Int)

I don't feel like I have sufficient knowledge of the scala type system internals to understand what's happening here.