fromSchema ARRAY mapping corrected (using Chunk)

[peterlopen]

hi,
I found out original ARRAY mapping was not working, this should fix it. in schema ARRAY should be represented as Chunk.
also mapping for BIT should be more compatible (based on docs, works for postgres).
any suggestion to improve code welcome, wrapping everything in Some is not ideal, could not figure out how to do it using partial function and case statements.
thank you.

[CLAassistant]

All committers have signed the CLA.

[guizmaii]

to be more perf/put less pressure on the GC, you'll probably want to use a mutable data structure and then wrap it in a Chunk:

Suggested change

	var chunkData = Chunk.empty[DynamicValue]
	while (arrayRs.next())
	mapPrimitiveType(arrayRs, 2, metaData.getColumnType(2)).foreach { el =>
	chunkData = chunkData :+ el
	}
	Some(DynamicValue.Sequence(chunkData))
	val data = mutable.ArrayBuffer.empty[DynamicValue]
	while (arrayRs.next()) {
	mapPrimitiveType(arrayRs, 2, metaData.getColumnType(2)).foreach { el =>
	data += el
	}
	}
	Some(DynamicValue.Sequence(Chunk.fromArray(data.toArray)))

[guizmaii]

@peterlopen To do it with partial functions you need to do somethink like this:

  private[jdbc] def createDynamicDecoder(schema: Schema[_], meta: ResultSetMetaData): ResultSet => DynamicValue =
    resultSet => {
      val remapName   = createRemapTable(schema)
      var columnIndex = 1
      var listMap     = ListMap.empty[String, DynamicValue]

      while (columnIndex <= meta.getColumnCount()) {
        val name                = remapName(meta.getColumnName(columnIndex))
        val sqlType             = meta.getColumnType(columnIndex)
        val value: DynamicValue =
          (mapPrimitiveType(resultSet, columnIndex) orElse mapComplexType(resultSet, columnIndex))
            .applyOrElse(
              sqlType,
              _ =>
                throw new SQLException(
                  s"Unsupported SQL type ${sqlType} when attempting to decode result set from a ZIO Schema ${schema}"
                )
            )

        listMap = listMap.updated(name, value)

        columnIndex += 1
      }

      DynamicValue.Record(TypeId.Structural, listMap)
    }

  private def mapPrimitiveType(
    resultSet: ResultSet,
    columnIndex: Int
  ): PartialFunction[Int, DynamicValue] = {
    case SqlTypes.BIGINT =>
      val bigInt = resultSet.getBigDecimal(columnIndex).toBigInteger()

      DynamicValue.Primitive(bigInt, StandardType.BigIntegerType)

   ...
   
    case SqlTypes.VARCHAR =>
      val string = resultSet.getString(columnIndex)

      DynamicValue.Primitive(string, StandardType.StringType)
  }

  private def mapComplexType(resultSet: ResultSet, columnIndex: Int): PartialFunction[Int, DynamicValue] = {
    case SqlTypes.ARRAY =>
      val arrayRs  = resultSet.getArray(columnIndex).getResultSet
      val metaData = arrayRs.getMetaData
      val data     = mutable.ArrayBuffer.empty[DynamicValue]
      val add      = mapPrimitiveType(arrayRs, 2) andThen (el => { data += el; () })

      while (arrayRs.next())
        add.applyOrElse(metaData.getColumnType(2), _ => ())

      DynamicValue.Sequence(Chunk.fromArray(data.toArray))
  }

[peterlopen]

hi @guizmaii , thank you for comments, will look into it.

[guizmaii]

FYI, this is unsafe. If mapPrimitiveType pattern matching doesn't match with the metaData.getColumnType(2) value, this will throw. Is it what you want?

Your previous version based on Option was safe

[peterlopen]

no, I want sql exception with some info. added applyOrElse there. thanks

[peterlopen]

hi @guizmaii ,

thanks for helping me with code.
I am wondering - in my partial function attempt, I got match error exception from mapPrimitiveType when SqlTypes.ARRAY was mapped. I was not using orElse with mapComplexType, so that is the reason probably. my solution was to usy Try and Try.toOption and then Option.getOrElse which defeat all idea of using PartialFunction.
however, in your approach orElse is handling this problem. I tried to debug how it figures out mapPrimitiveType is not defined for SqlTypes.ARRAY, but could not find it. there is no exception and this code from OrElse implementation

    override def applyOrElse[A1 <: A, B1 >: B](x: A1, default: A1 => B1): B1 = {
      val z = f1.applyOrElse(x, checkFallback[B])
      if (!fallbackOccurred(z)) z else f2.applyOrElse(x, default)
    }

just magically figures out that f2 needs to be called.
Please do you know how it is handled internally?
Thank you.
regards,
peter

[guizmaii]

Please do you know how it is handled internally?

I don't know, sorry but indeed, as I mentioned here #159 (comment) your current code is unsafe and should probably adopt the way I gave you, using applyOrElse

[guizmaii]

@peterlopen Why closing?

[peterlopen]

@guizmaii I found two other issues with using arrays with zio-jdbc, so I gave up on this approach.
for the record:

1. incorrect handling of empty arrays, also JDBC approach is to use just one '?' placeholder for array value:

   zio-jdbc/core/src/main/scala/zio/jdbc/ZConnection.scala

   Line 55 in 95cf951

   case array: Array[_] =>

2. follows approach from 1), although in JDBC PreparedStatement.setArray should be used:

   zio-jdbc/core/src/main/scala/zio/jdbc/SqlFragment.scala

   Line 356 in 95cf951

   implicit def arraySetter[A](implicit setter: Setter[A]): Setter[Array[A]] =

I do not mind merging it, but without fixing other issues it still be incomplete.