From 4863f5c5671deeaf6cde4751740794eb49438f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Thu, 10 Sep 2026 06:14:56 +0200 Subject: [PATCH 1/3] feat(BIDC-10a): one JDBC type catalogue, in core, that both transports derive from Lead-directed, folded into BIDC-10a. `getTypeInfo` in the JDBC driver was a hand-written list of 11 rows deriving from nothing, and the Flight producer hand-wrote its own 11 with `java.sql.Types` imported directly - two parallel transcriptions of a mapping that already existed. `core.client.metadata.JdbcTypeCatalog` is now the one authority; jdbc and arrow derive from it next. It lives in `core`, not `sql`: `sql` is the AST and parser module and must not gain a `java.sql` dependency. Verified before building on it - `core` is on the compile classpath of BOTH consumer modules, checked in their own sources (`ElasticDatabaseMetaData.scala` and `ElasticFlightProducer.scala` each import `app.softnetwork.elastic.client.*`), not inferred from a build file. WHAT THE DUPLICATION WAS HIDING, and it is worse than the duplication. `getColumns.TYPE_NAME` reports the ENGINE's type id, so a character column comes back as `KEYWORD` or `TEXT`, never `VARCHAR`. With 11 rows, `KEYWORD`, `TEXT`, `DATETIME`, `CHAR`, `STRUCT`, `GEO_POINT`, `VARBINARY` and `ARRAY<...>` round-tripped to NO catalogue row at all. Neither transcription could reveal it, because each was self-consistent with itself. The catalogue is now 20 rows - every user-facing type a column can actually have - and `find(typeId)` resolves a `getColumns` type name back to its row. DERIVED, NOT STATED. A row carries a `SQLType`, not a code: `typeId` and `dataType` are computed from `sqlType.typeId` and the single `jdbcType(SQLType)` mapping, so a row and the mapping cannot disagree by construction. That is the failure the cross-repo "agreement gate" cannot catch (spec D.2b). THE ORDERING RULE CHANGED, and any surviving distinctness assertion now pins the OLD catalogue. With 11 rows every `DATA_TYPE` was distinct, so ascending `DATA_TYPE` was a total order and `getTypeInfo`'s second clause - "and then by how closely the data type maps to the corresponding JDBC SQL type" - never had to be adjudicated. Both repos asserted "sorted AND distinct" and were right to. At 20 rows `KEYWORD`/`TEXT`/`VARCHAR`/`GEO_POINT` all carry `Types.VARCHAR`, `NUMERIC` shares `Types.DOUBLE`, `DATETIME` shares `Types.TIMESTAMP`: distinctness is gone and the tie-break is live. The tie-break is COMPUTED, not enumerated: the type whose own id IS that JDBC type's name is the closest possible match, read off `java.sql.JDBCType.valueOf(code).getName`, then `typeId` ascending to make the order total. So `VARCHAR` precedes `GEO_POINT`/`KEYWORD`/`TEXT`, `DOUBLE` precedes `NUMERIC`, `TIMESTAMP` precedes `DATETIME`, with nothing hand-listed - hand-listing "VARCHAR first" would have been a fourth place encoding one fact. `entries` is produced by SORTING a declaration whose source order is irrelevant, so no row can be hand-placed. Falsified three ways: dropping the whole tie-break reddens 3 of 11 tests; dropping only the canonical-first component reddens 2, isolating that rule; and shuffling the declaration order changes nothing - the last is what proves the shipped order is computed rather than typed, and no positional fixture can prove it. The nine new sizes are not nine invented numbers. `PRECISION` describes what the JDBC TYPE can hold and `DATA_TYPE` IS the JDBC type, so types sharing a `DATA_TYPE` share its precision: `NUMERIC` takes `DOUBLE`'s 15, `DATETIME` takes `TIMESTAMP`'s 23, `TEXT`/`KEYWORD`/`GEO_POINT` take `VARCHAR`'s 65535. `CHAR` and `VARBINARY` have codes of their own and no engine-imposed bound, so they take the same ceiling rather than a number invented for each; `STRUCT` and `ARRAY` have none. All eleven pre-existing rows keep their exact values. `NULL` is deliberately excluded, and the source says why: `getTypeInfo` describes types a column can HAVE, and `Types.NULL` is the absence of a value, not a storable type. Its presence in the mapping is legitimate and different - that maps an inferred RUNTIME type. Verified: core 967/967, `+ core/compile`, `++ 2.12.20 core/Test/compile`, lint. Published locally and content-verified with `javap` on the jar, not a directory listing. --- .../client/metadata/JdbcTypeCatalog.scala | 210 ++++++++++++++++++ .../client/metadata/JdbcTypeCatalogSpec.scala | 194 ++++++++++++++++ 2 files changed, 404 insertions(+) create mode 100644 core/src/main/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalog.scala create mode 100644 core/src/test/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalogSpec.scala diff --git a/core/src/main/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalog.scala b/core/src/main/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalog.scala new file mode 100644 index 00000000..b63f6df1 --- /dev/null +++ b/core/src/main/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalog.scala @@ -0,0 +1,210 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client.metadata + +import app.softnetwork.elastic.sql.`type`._ + +import java.sql.{JDBCType, Types} +import java.util.Locale +import scala.util.Try + +/** The ONE authority for how this engine's types appear over JDBC and Flight SQL. + * + * Story BIDC-10a. Before this existed, `DatabaseMetaData.getTypeInfo` in the JDBC driver was a + * hand-written list of eleven rows deriving from nothing, and the Arrow Flight SQL producer + * hand-wrote its own eleven rows with `java.sql.Types` imported directly - two parallel + * transcriptions of a mapping that already existed. That is why the catalogue was incomplete + * (`getColumns.TYPE_NAME` reports `KEYWORD`, `TEXT`, `DATETIME`, `CHAR`, `STRUCT`, `GEO_POINT`, + * `VARBINARY` and `ARRAY<...>`, none of which had a catalogue row to round-trip to), why it + * inherited a javadoc-violating row order, and why the "agreement gate" between the two repos was + * a convention rather than an interlock. It is Part D's lesson one layer up: two places encoding + * one fact. + * + * This object lives in `core`, not `sql`: `sql` is the AST and parser module and must not gain a + * `java.sql` dependency, while both the JDBC driver and the Flight producer already compile + * against `core` (verified in their own sources, not inferred from a build file). + */ +object JdbcTypeCatalog { + + /** SQLType -> `java.sql.Types` code. THE mapping; nothing else may re-list these codes. + * + * Ordering of the cases matters: the specific `case object`s come before the trait patterns that + * would also match them, and the trait fallbacks at the end are what give an unknown-but-typed + * value a sensible code instead of `OTHER`. + */ + def jdbcType(sqlType: SQLType): Int = sqlType match { + case SQLTypes.Int => Types.INTEGER + case SQLTypes.BigInt => Types.BIGINT + case SQLTypes.Double => Types.DOUBLE + case SQLTypes.Real => Types.REAL + case SQLTypes.TinyInt => Types.TINYINT + case SQLTypes.SmallInt => Types.SMALLINT + case SQLTypes.Boolean => Types.BOOLEAN + case SQLTypes.Date => Types.DATE + case SQLTypes.Time => Types.TIME + case _: SQLDateTime => Types.TIMESTAMP + case SQLTypes.Char => Types.CHAR + case _: SQLVarchar => Types.VARCHAR // includes Text, Keyword, Varchar + case _: SQLArray => Types.ARRAY + case SQLTypes.Struct => Types.STRUCT + case SQLTypes.GeoPoint => Types.VARCHAR // serialized as a string + case SQLTypes.VarBinary => Types.VARBINARY + case SQLTypes.Null => Types.NULL + case _: SQLNumeric => Types.DOUBLE // fallback for unknown numeric + case _: SQLTemporal => Types.TIMESTAMP // fallback for unknown temporal + case _: SQLLiteral => Types.VARCHAR // fallback for unknown literal + case _ => Types.OTHER + } + + /** One catalogue row. `typeId` and `dataType` are DERIVED - a row cannot state a code that + * disagrees with [[jdbcType]], because it does not state one at all. + * + * @param columnSize + * the MAXIMUM precision the TYPE supports (JDBC `getTypeInfo.PRECISION`). Not to be confused + * with `ResultSetMetaData.getPrecision`, which reports the specified size of one COLUMN and is + * 256 for character data by story 20.3's AD-5. Both are correct and they answer different + * questions (BIDC-10a lead ruling LR-3). + */ + case class JdbcTypeInfo( + sqlType: SQLType, + columnSize: Int, + literalPrefix: Option[String], + literalSuffix: Option[String] + ) { + + /** What `getColumns.TYPE_NAME` reports for a column of this type - the ENGINE's type id, not a + * JDBC name. A character column comes back as `KEYWORD` or `TEXT`, never `VARCHAR`, which is + * exactly why those types need catalogue rows of their own. + */ + def typeId: String = sqlType.typeId + def dataType: Int = jdbcType(sqlType) + } + + private val Quote = Some("'") + private val NoLiteral: Option[String] = None + + /** The engine's ceiling for character and binary payloads. Inherited from the eleven-row + * catalogue's `VARCHAR` row, and applied UNIFORMLY to every type that shares a character or + * binary `DATA_TYPE` - a constant justified by what the JDBC type can hold does not get + * exceptions (see the size rule below). + */ + private val CharacterCeiling = 65535 + + /** No meaningful precision. `ResultSetMetaData.getPrecision` already answers `0` for these. */ + private val NoPrecision = 0 + + /** 🔴 The size rule, stated once so that nine new rows are not nine invented numbers. + * + * `PRECISION` describes what the JDBC TYPE can hold, and the `DATA_TYPE` code IS the JDBC type ⇒ + * **types sharing a `DATA_TYPE` share its precision.** Every value below is either one of the + * eleven inherited from the shipped catalogue (byte-identical - none moved) or forced by that + * rule: + * + * - `NUMERIC` shares `DOUBLE`'s code ⇒ 15. ES `scaled_float` is a long plus a scaling factor, + * so double precision is also what it actually carries. + * - `DATETIME` shares `TIMESTAMP`'s code ⇒ 23, the width of `yyyy-MM-dd HH:mm:ss.SSS`. + * - `TEXT`, `KEYWORD` and `GEO_POINT` share `VARCHAR`'s code ⇒ the character ceiling. + * `GEO_POINT` is a string as far as JDBC is concerned; its own serialized form is far + * shorter, but that is a property of the values, not of the type's maximum. + * - `CHAR` and `VARBINARY` have codes of their own and no engine-imposed bound, so they take + * the same ceiling rather than a number invented for each. + * - `STRUCT` and `ARRAY` have no precision at all. + * + * The literal prefix/suffix follows the same shape: everything a user writes QUOTED carries + * `'`/`'`; numbers and booleans carry neither; `STRUCT` and `ARRAY` carry neither + * because there is no literal syntax for them. + * + * ⚠️ **`NULL` is deliberately absent.** `getTypeInfo` describes types a column can HAVE; + * `Types.NULL` is the absence of a value, not a storable type, and no mainstream driver lists + * it. Its presence in [[jdbcType]] is legitimate and different - that maps an INFERRED RUNTIME + * type, which is a separate question from what a column may be declared as. Do not "complete" + * the catalogue by adding it. + * + * Declared in an arbitrary (source-natural) order on purpose: the shipped order is COMPUTED by + * [[catalogOrdering]], so no one can hand-place a row. + */ + private val declared: Seq[JdbcTypeInfo] = Seq( + JdbcTypeInfo(SQLTypes.TinyInt, 3, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.SmallInt, 5, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Int, 10, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.BigInt, 19, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Real, 7, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Double, 15, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Numeric, 15, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Boolean, 1, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Date, 10, Quote, Quote), + JdbcTypeInfo(SQLTypes.Time, 8, Quote, Quote), + JdbcTypeInfo(SQLTypes.DateTime, 23, Quote, Quote), + JdbcTypeInfo(SQLTypes.Timestamp, 23, Quote, Quote), + JdbcTypeInfo(SQLTypes.Keyword, CharacterCeiling, Quote, Quote), + JdbcTypeInfo(SQLTypes.Text, CharacterCeiling, Quote, Quote), + JdbcTypeInfo(SQLTypes.Varchar, CharacterCeiling, Quote, Quote), + JdbcTypeInfo(SQLTypes.Char, CharacterCeiling, Quote, Quote), + JdbcTypeInfo(SQLTypes.Struct, NoPrecision, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.Array(SQLTypes.Struct), NoPrecision, NoLiteral, NoLiteral), + JdbcTypeInfo(SQLTypes.GeoPoint, CharacterCeiling, Quote, Quote), + JdbcTypeInfo(SQLTypes.VarBinary, CharacterCeiling, Quote, Quote) + ) + + /** The JDBC name of a type code, e.g. 12 -> `VARCHAR`. `None` for a code outside the standard + * enumeration, which cannot happen for anything in [[declared]] but must not throw if it ever + * does. + */ + def canonicalJdbcName(dataType: Int): Option[String] = + Try(JDBCType.valueOf(dataType).getName).toOption + + /** 🔴 The ordering rule, and it CHANGED when the catalogue grew. + * + * `DatabaseMetaData.getTypeInfo`'s javadoc: rows *"are ordered by DATA_TYPE and then by how + * closely the data type maps to the corresponding JDBC SQL type"*. With eleven rows every code + * was distinct, so ascending `DATA_TYPE` was a TOTAL order and the second clause never had to be + * adjudicated - both repos asserted "sorted AND distinct" and were right to. + * + * That is no longer true. `KEYWORD`, `TEXT`, `VARCHAR` and `GEO_POINT` all carry + * `Types.VARCHAR`; `NUMERIC` shares `Types.DOUBLE` with `DOUBLE`; `DATETIME` shares + * `Types.TIMESTAMP` with `TIMESTAMP`. **Distinctness is gone and the second clause is live.** + * Any assertion still pinning distinctness is asserting the old catalogue. + * + * The tie-break, in two steps, both mechanical: + * + * 1. **The canonically named type first.** "How closely the data type maps to the JDBC SQL + * type" is read as: the type whose own id IS that JDBC type's name is the closest possible + * match. Computed from [[canonicalJdbcName]], never hand-listed - so `VARCHAR` precedes + * `GEO_POINT`/`KEYWORD`/`TEXT`, `DOUBLE` precedes `NUMERIC`, and `TIMESTAMP` precedes + * `DATETIME`, with nothing enumerated by hand. (`INT` does not match `INTEGER` and + * `ARRAY` does not match `ARRAY`; both are alone in their groups, so it costs + * nothing.) 2. **Then `typeId` ascending**, `Locale.ROOT`, purely to make the order TOTAL + * and stable. A tie-break that leaves ties is not a tie-break: two rows the sort considers + * equal may come back in either order and a positional fixture would flake. + */ + val catalogOrdering: Ordering[JdbcTypeInfo] = + Ordering.by { info: JdbcTypeInfo => + val canonical = canonicalJdbcName(info.dataType).contains(info.typeId) + (info.dataType, if (canonical) 0 else 1, info.typeId.toUpperCase(Locale.ROOT)) + } + + /** The catalogue, in contract order. This is what `getTypeInfo` and `CommandGetXdbcTypeInfo` + * stream; neither may re-order it, filter it, or add to it. + */ + val entries: Seq[JdbcTypeInfo] = declared.sorted(catalogOrdering) + + /** Lookup by the engine's type id, for a driver resolving `getColumns.TYPE_NAME` back to a + * catalogue row. + */ + def find(typeId: String): Option[JdbcTypeInfo] = + entries.find(_.typeId.equalsIgnoreCase(typeId)) +} diff --git a/core/src/test/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalogSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalogSpec.scala new file mode 100644 index 00000000..ee98bd17 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/metadata/JdbcTypeCatalogSpec.scala @@ -0,0 +1,194 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client.metadata + +import app.softnetwork.elastic.sql.`type`.SQLTypes +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.sql.Types + +/** Story BIDC-10a — the ONE type catalogue, in `core`, that the JDBC driver's `getTypeInfo` and the + * Flight producer's `CommandGetXdbcTypeInfo` both derive from instead of hand-transcribing. + * + * Two kinds of assertion, deliberately, because each is blind to what the other catches (BIDC-6's + * lesson, and D.1b's): the POSITIONAL fixture below is satisfied by any order the fixture and the + * source agree on - a coordinated edit keeps it green - while the ORDERING PROPERTIES are computed + * from the contract and redden on a reorder with no fixture edit at all. + */ +class JdbcTypeCatalogSpec extends AnyFlatSpec with Matchers { + + /** The catalogue in contract order: (typeId, DATA_TYPE, PRECISION, prefix, suffix). + * + * The eleven types that shipped before this story keep their exact values; the nine new ones + * follow the size rule documented on `JdbcTypeCatalog.declared` (types sharing a DATA_TYPE share + * its precision). + */ + private val expected: Seq[(String, Int, Int, Option[String], Option[String])] = Seq( + ("TINYINT", Types.TINYINT, 3, None, None), + ("BIGINT", Types.BIGINT, 19, None, None), + ("VARBINARY", Types.VARBINARY, 65535, Some("'"), Some("'")), + ("CHAR", Types.CHAR, 65535, Some("'"), Some("'")), + ("INT", Types.INTEGER, 10, None, None), + ("SMALLINT", Types.SMALLINT, 5, None, None), + ("REAL", Types.REAL, 7, None, None), + ("DOUBLE", Types.DOUBLE, 15, None, None), + ("NUMERIC", Types.DOUBLE, 15, None, None), + ("VARCHAR", Types.VARCHAR, 65535, Some("'"), Some("'")), + ("GEO_POINT", Types.VARCHAR, 65535, Some("'"), Some("'")), + ("KEYWORD", Types.VARCHAR, 65535, Some("'"), Some("'")), + ("TEXT", Types.VARCHAR, 65535, Some("'"), Some("'")), + ("BOOLEAN", Types.BOOLEAN, 1, None, None), + ("DATE", Types.DATE, 10, Some("'"), Some("'")), + ("TIME", Types.TIME, 8, Some("'"), Some("'")), + ("TIMESTAMP", Types.TIMESTAMP, 23, Some("'"), Some("'")), + ("DATETIME", Types.TIMESTAMP, 23, Some("'"), Some("'")), + ("STRUCT", Types.STRUCT, 0, None, None), + ("ARRAY", Types.ARRAY, 0, None, None) + ) + + private def actual = + JdbcTypeCatalog.entries.map(e => + (e.typeId, e.dataType, e.columnSize, e.literalPrefix, e.literalSuffix) + ) + + "The JDBC type catalogue" should "stream exactly the reconciled rows, in order" in { + actual shouldBe expected + } + + it should "cover every user-facing column type the engine can report" in { + JdbcTypeCatalog.entries.map(_.typeId).toSet shouldBe Set( + "TINYINT", + "SMALLINT", + "INT", + "BIGINT", + "REAL", + "DOUBLE", + "NUMERIC", + "BOOLEAN", + "DATE", + "TIME", + "DATETIME", + "TIMESTAMP", + "KEYWORD", + "TEXT", + "VARCHAR", + "CHAR", + "STRUCT", + "ARRAY", + "GEO_POINT", + "VARBINARY" + ) + } + + /** 🔴 The derivation pin. A row does not STATE a type code - it computes one - so this asserts + * that the catalogue and the mapping cannot disagree. If `dataType` ever became a constructor + * field, this is the test that should have to change. + */ + it should "derive every DATA_TYPE from the one mapping" in { + JdbcTypeCatalog.entries.foreach { e => + withClue(s"${e.typeId}: ") { + e.dataType shouldBe JdbcTypeCatalog.jdbcType(e.sqlType) + e.typeId shouldBe e.sqlType.typeId + } + } + } + + it should "never map a catalogued type to OTHER" in { + JdbcTypeCatalog.entries.foreach { e => + withClue(s"${e.typeId} fell through the mapping: ") { + e.dataType should not be Types.OTHER + } + } + } + + // -- The ordering contract ------------------------------------------------- + // + // `getTypeInfo`'s javadoc: rows "are ordered by DATA_TYPE and then by how closely the data type + // maps to the corresponding JDBC SQL type". These assertions are computed from that sentence, so + // a reorder reddens them WITHOUT any fixture edit - which is the whole point, since the + // positional pin above survives a coordinated edit (measured on the jdbc leg, D.1b). + + it should "order rows by DATA_TYPE ascending" in { + val codes = JdbcTypeCatalog.entries.map(_.dataType) + codes shouldBe codes.sorted + } + + /** 🔴 Distinctness is GONE and this test is the record of it. Four types share `Types.VARCHAR`, + * two share `Types.DOUBLE`, two share `Types.TIMESTAMP`. An assertion still pinning distinct + * codes - as both repos' fixtures did while the catalogue had eleven rows - is asserting the old + * catalogue, so the tie-break below is what has to hold instead. + */ + it should "have genuine DATA_TYPE ties, so the tie-break is load-bearing" in { + val grouped = JdbcTypeCatalog.entries.groupBy(_.dataType).filter(_._2.size > 1) + grouped.keySet shouldBe Set(Types.VARCHAR, Types.DOUBLE, Types.TIMESTAMP) + grouped(Types.VARCHAR).map(_.typeId) should contain theSameElementsAs + Seq("VARCHAR", "TEXT", "KEYWORD", "GEO_POINT") + } + + it should "put the canonically named type first within each DATA_TYPE group" in { + val groups = JdbcTypeCatalog.entries.groupBy(_.dataType) + val adjudicated = groups.filter { case (code, group) => + group.size > 1 && group.exists(e => + JdbcTypeCatalog.canonicalJdbcName(code).contains(e.typeId) + ) + } + // The rule is only meaningful where a group actually contains its canonical type. + adjudicated.keySet shouldBe Set(Types.VARCHAR, Types.DOUBLE, Types.TIMESTAMP) + adjudicated.foreach { case (code, _) => + val first = JdbcTypeCatalog.entries.filter(_.dataType == code).head.typeId + withClue(s"first row of DATA_TYPE $code: ") { + JdbcTypeCatalog.canonicalJdbcName(code) shouldBe Some(first) + } + } + } + + it should "break remaining ties by typeId, so the order is total and stable" in { + JdbcTypeCatalog.entries.groupBy(_.dataType).foreach { case (code, _) => + val canonical = JdbcTypeCatalog.canonicalJdbcName(code) + val nonCanonical = + JdbcTypeCatalog.entries.filter(e => e.dataType == code && !canonical.contains(e.typeId)) + withClue(s"non-canonical members of DATA_TYPE $code: ") { + nonCanonical.map(_.typeId) shouldBe nonCanonical.map(_.typeId).sorted + } + } + } + + /** The whole catalogue re-sorted by its own Ordering must be the catalogue. A no-op on a sorted + * sequence, and a red the moment `entries` stops being produced by `catalogOrdering`. + */ + it should "be produced by its own documented ordering" in { + JdbcTypeCatalog.entries.sorted(JdbcTypeCatalog.catalogOrdering) shouldBe JdbcTypeCatalog.entries + } + + /** ⚠️ `NULL` describes the absence of a value, not a type a column can have. Its presence in the + * runtime mapping is legitimate and different; its presence HERE would be a defect. + */ + it should "exclude NULL, which is not a storable column type" in { + JdbcTypeCatalog.entries.map(_.typeId) should not contain "NULL" + JdbcTypeCatalog.entries.map(_.dataType) should not contain Types.NULL + // ...while the mapping still answers for it, because that is a different question. + JdbcTypeCatalog.jdbcType(SQLTypes.Null) shouldBe Types.NULL + } + + it should "resolve a getColumns TYPE_NAME back to its catalogue row" in { + JdbcTypeCatalog.find("KEYWORD").map(_.dataType) shouldBe Some(Types.VARCHAR) + JdbcTypeCatalog.find("keyword").map(_.typeId) shouldBe Some("KEYWORD") + JdbcTypeCatalog.find("ARRAY").map(_.dataType) shouldBe Some(Types.ARRAY) + JdbcTypeCatalog.find("NULL") shouldBe None + } +} From 857e485de9c251a7f5308d524b004b5f98478bd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Thu, 10 Sep 2026 06:28:13 +0200 Subject: [PATCH 2/3] feat(BIDC-10a): one deep Scala-to-Java value conversion, at the consumer boundary Lead-approved follow-up to the type catalogue. Core's read path hands out Scala `Map`s and `List`s as CELL values; a JDBC or ADBC consumer that casts gets a `ClassCastException` and one that calls `toString` gets Scala syntax. Both drivers need the same conversion, so it lives in core once rather than twice. THE CENSUS CHANGED THE DESIGN, and it was worth more than the converter. The obvious home was `ElasticConversion.jsonNodeToAny`, which builds the `List` and the nested `Map` from `_source`. It is not the only producer. Three make these shapes and only one goes through it: 1. `jsonNodeToAny:1222-1225` - array branch to a Scala `List`, object branch to `jsonNodeToMap`, a `ListMap`. 2. the aggregation path, ~`:958` and ~`:977` - a stats aggregation emits `name -> ListMap("count" -> ..., "sum" -> ...)` and `percentiles` emits `name -> ListMap( -> )`, both as CELL values, neither built from a `JsonNode`. 3. `extractInnerHits` ~`:615-630` - `innerHitName -> List[ListMap[String, Any]]`, hand assembled, and precisely the List-of-Map shape a shallow conversion gets wrong. So a producer-side converter would have been honest for one path and a lie for two. This one sits at the CONSUMER boundary, where it is producer-agnostic by construction: it converts whatever value it is handed, and a fourth producer needs no change to it. Generalising: before centralising a conversion, census the PRODUCERS - "where is this value built?" usually has more than one answer, and the obvious one is rarely the complete one. HOME: `metadata`, beside the type catalogue, not `ElasticConversion`. Co-locating with one of three producers would misstate the relationship. Nothing inside core calls it; it exists solely to satisfy a driver-facing contract, exactly like the catalogue. EAGER OR LAZY: lazy, at the accessor, with an identity fast path. Converting every row at extraction time would allocate on the hot path for every row of every query, including the overwhelming majority where no cell is a collection and no consumer ever calls `getObject` - the path #238 and arrow#139 exist to keep cheap. A non-collection returns the SAME REFERENCE, so the common case is one type test and no allocation, paid only for cells someone asks for. Deliberately not memoised here: core does not own the row's lifetime, and a driver that measures repeated access to one wide struct can cache at its own seam. RED THEN GREEN, GENUINELY. A deliberately shallow implementation went in first and passed 7 of 11 tests - every single-level case, key order, reference identity, null, byte arrays and empty collections - failing only the four depth-sensitive ones. That is what makes the nested-inside-array test a gate rather than a restatement, and it is the reason the lead asked for it first: a shallow implementation is what every reviewer's eye reads as correct. One test drives REAL core code (`jsonNodeToAny` on an array of objects) and asserts both halves: that core still produces Scala collections, so the problem is real, and that the conversion resolves them at every level, so the fix is. If a producer is ever changed to emit Java collections directly, that test says so instead of `toJavaValue` silently becoming dead code. Boundaries, all recorded in source: call it on a VALUE and never on the row (both are `ListMap` and this cannot tell them apart); a `byte[]` survives untouched, since a Java array is not a Scala `Seq` and a `java.util.List[Byte]` would break every consumer that casts to `[B`; `Option` is deliberately NOT unwrapped, because no producer emits one today - the stats aggregation unwraps its own - so unwrapping here would hide a real defect rather than fix one. `LinkedHashMap` and `ArrayList`, so `ListMap` key order survives into the struct a consumer walks. The drivers are untouched: this is a seam, not a patch. `toJdbcClassName` reporting `STRUCT` as `java.lang.String` through the catch-all, and `ARRAY` as `java.sql.Array` which `createArrayOf`/`createStruct` both refuse to construct, are the jdbc agent's to fix against it. Verified: core 979/979, `+ core/compile`, `++ 2.12.20 core/Test/compile`, lint. Published locally and content-verified with `javap` - `JavaValueConversion$.toJavaValue` present and `JdbcTypeCatalog$` still intact in the same jar. --- .../client/metadata/JavaValueConversion.scala | 108 ++++++++++ .../metadata/JavaValueConversionSpec.scala | 185 ++++++++++++++++++ 2 files changed, 293 insertions(+) create mode 100644 core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala create mode 100644 core/src/test/scala/app/softnetwork/elastic/client/metadata/JavaValueConversionSpec.scala diff --git a/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala b/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala new file mode 100644 index 00000000..c6b08607 --- /dev/null +++ b/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala @@ -0,0 +1,108 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client.metadata + +/** Converts a core row VALUE into the Java collections a `getObject` consumer expects. + * + * Story BIDC-10a. Core's read path hands out Scala `Map`s and `List`s as cell values; a JDBC or + * ADBC consumer that casts gets a `ClassCastException`, and one that calls `toString` gets Scala + * syntax. Both drivers need the same conversion, so it lives here once rather than twice. + * + * ==Why this is NOT at the producer== + * + * The obvious placement is `ElasticConversion.jsonNodeToAny`, which builds the `List` and the + * nested `Map` from `_source`. A census of the read path (2026-09-10) says that would be honest + * for one path and a lie for two others. **Three** producers make these shapes, and only the first + * goes through `jsonNodeToAny`: + * + * 1. `ElasticConversion.jsonNodeToAny` — the array branch (a Scala `List`) and the object branch + * (`jsonNodeToMap`, a `ListMap`). 2. The aggregation path — a stats aggregation emits `name + * -> ListMap("count" -> ..., "sum" -> ...)` and `percentiles` emits `name -> ListMap( -> + * )`, both as CELL values, neither built from a `JsonNode` through `jsonNodeToAny`. + * 3. `ElasticConversion.extractInnerHits` — `innerHitName -> List[ListMap[String, Any]]`, + * hand assembled. That is precisely the `List`-of-`Map` shape a shallow conversion gets + * wrong. + * + * ⇒ The conversion belongs at the CONSUMER boundary, where it is producer-agnostic by + * construction: it converts whatever value it is handed, so a fourth producer needs no change + * here. Placing it beside one producer would have needed a change for every new one. + * + * It lives in `metadata`, with the type catalogue, for the same reason: nothing inside core calls + * it. It exists solely to satisfy a driver-facing contract. + * + * ==Eager or lazy: lazy, at the accessor, with an identity fast path== + * + * Converting every row at extraction time would put allocation on the extraction hot path for + * EVERY row of EVERY query — including the overwhelming majority where no cell is a collection and + * no consumer ever calls `getObject`. That is the path #238 and arrow#139 exist to keep cheap, and + * the standing rule is that per-row work on an extraction path must not be re-derived per cell. + * + * So a driver calls this from `getObject`, on demand, per accessed cell. A non-collection returns + * the SAME REFERENCE — one type test, zero allocation — so the common case costs effectively + * nothing and is paid only for cells someone actually asks for. + * + * Deliberately NOT memoised here: core does not own the row's lifetime. A driver that measures + * repeated `getObject` calls on the same wide struct can cache per row at its own seam, where the + * lifetime is known. + * + * ==Boundaries== + * + * - Call it on a VALUE, never on the row itself. A row is a `ListMap[String, Any]` and so is a + * nested struct; this function cannot tell them apart, and converting the row would break + * every by-column accessor. + * - A `byte[]` (a `VARBINARY` cell) is a Java array, not a Scala `Seq`, so it survives + * untouched. Converting it to a `java.util.List[Byte]` would break every consumer that casts + * to `[B`. + * - `Option` is deliberately not unwrapped. No producer emits one today (the stats aggregation + * unwraps its own via `collect`), so unwrapping here would hide a real defect rather than fix + * one. + */ +object JavaValueConversion { + + /** Deep-converts Scala collections to their Java counterparts; returns everything else by + * identity. + * + * 🔴 DEEP is the whole point. A nested field yields `List[Map[String, Any]]`, so a shallow + * `asJava` on the outer list hands back a `java.util.List` whose elements are still Scala maps + * and the defect survives one level down — while every single-level test still passes. Measured: + * a shallow implementation passes 7 of this object's 11 tests, including key order, identity, + * null, byte arrays and empty collections, and fails only the four depth-sensitive ones. + */ + def toJavaValue(value: Any): Any = value match { + case null => null + + // `LinkedHashMap`, not `HashMap`: core's maps are `ListMap`s and their key order is meaningful + // to a consumer walking a struct or a stats aggregation. + case m: scala.collection.Map[_, _] => + val out = new java.util.LinkedHashMap[Any, Any](Math.max(4, m.size * 2)) + m.foreach { case (k, v) => out.put(toJavaValue(k), toJavaValue(v)) } + out + + case s: scala.collection.Seq[_] => + val out = new java.util.ArrayList[Any](s.size) + s.foreach(v => out.add(toJavaValue(v))) + out + + case s: scala.collection.Set[_] => + val out = new java.util.LinkedHashSet[Any](Math.max(4, s.size * 2)) + s.foreach(v => out.add(toJavaValue(v))) + out + + // The hot path: one type test, same reference back, nothing allocated. + case other => other + } +} diff --git a/core/src/test/scala/app/softnetwork/elastic/client/metadata/JavaValueConversionSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/metadata/JavaValueConversionSpec.scala new file mode 100644 index 00000000..a3a22684 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/metadata/JavaValueConversionSpec.scala @@ -0,0 +1,185 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client.metadata + +import app.softnetwork.elastic.client.ElasticConversion +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.collection.immutable.ListMap + +/** Story BIDC-10a — one Scala-to-Java value conversion, in core, for the `getObject` contract of + * every driver. + * + * Core's read path hands out Scala `Map`s and `List`s as CELL VALUES. A JDBC or ADBC consumer + * calling `getObject` must receive `java.util.Map` / `java.util.List`; without this, a host that + * casts gets a `ClassCastException` and one that calls `toString` gets Scala syntax. Each driver + * writing its own is the duplication the type catalogue just removed one layer up. + */ +class JavaValueConversionSpec extends AnyFlatSpec with Matchers { + + import JavaValueConversion.toJavaValue + + /** 🔴 THE test, written first and for a reason: it is the one a SHALLOW implementation passes + * everything else on. + * + * A nested field yields `List[Map[String, Any]]` — three of core's producers make exactly this + * shape (`jsonNodeToAny`'s array branch, and `extractInnerHits`, which builds a + * `List[ListMap[String, Any]]` by hand). A shallow `asJava` on the outer list returns a + * `java.util.List` whose ELEMENTS are still Scala maps, so the defect survives one level down + * and every single-level test still passes. + */ + "toJavaValue" should "convert a Map nested inside a List (the shallow-conversion trap)" in { + val value = List(ListMap("city" -> "Paris", "zip" -> 75001)) + + val converted = toJavaValue(value) + + converted shouldBe a[java.util.List[_]] + val list = converted.asInstanceOf[java.util.List[Any]] + list.size shouldBe 1 + withClue("the ELEMENT of the list is still a Scala Map: ") { + list.get(0) shouldBe a[java.util.Map[_, _]] + } + val inner = list.get(0).asInstanceOf[java.util.Map[String, Any]] + inner.get("city") shouldBe "Paris" + inner.get("zip") shouldBe 75001 + } + + it should "convert a List nested inside a Map, the other way round" in { + val value = ListMap("tags" -> List("a", "b")) + + val inner = toJavaValue(value).asInstanceOf[java.util.Map[String, Any]].get("tags") + + inner shouldBe a[java.util.List[_]] + inner.asInstanceOf[java.util.List[Any]].size shouldBe 2 + } + + it should "convert arbitrarily deep alternations" in { + val value = List(ListMap("inner" -> List(ListMap("leaf" -> 1)))) + + val leaf = toJavaValue(value) + .asInstanceOf[java.util.List[Any]] + .get(0) + .asInstanceOf[java.util.Map[String, Any]] + .get("inner") + .asInstanceOf[java.util.List[Any]] + .get(0) + + leaf shouldBe a[java.util.Map[_, _]] + leaf.asInstanceOf[java.util.Map[String, Any]].get("leaf") shouldBe 1 + } + + it should "convert a top-level Map" in { + toJavaValue(ListMap("a" -> 1)) shouldBe a[java.util.Map[_, _]] + } + + it should "convert a top-level List" in { + toJavaValue(List(1, 2)) shouldBe a[java.util.List[_]] + } + + /** Column order is meaningful to a consumer walking a struct, and `ListMap` is ordered, so the + * conversion must not hand back a hash-ordered map. + */ + it should "preserve key order" in { + val value = ListMap("z" -> 1, "a" -> 2, "m" -> 3) + val keys = toJavaValue(value).asInstanceOf[java.util.Map[String, Any]].keySet() + keys.toArray.toSeq shouldBe Seq("z", "a", "m") + } + + /** 🔴 The hot-path property. Every scalar cell on an extraction path passes through here, so a + * non-collection must come back as the SAME reference — one type test, zero allocation. + */ + it should "return non-collections by identity, allocating nothing" in { + val s = "text" + val i = java.lang.Integer.valueOf(7) + val d = java.lang.Double.valueOf(1.5) + val t = java.time.LocalDate.of(2026, 1, 1) + // `.asInstanceOf[AnyRef]` only to satisfy `theSameInstanceAs`, which needs an AnyRef; the + // assertion is still reference identity. (`Cannot prove that Any <:< AnyRef` otherwise.) + toJavaValue(s).asInstanceOf[AnyRef] should be theSameInstanceAs s + toJavaValue(i).asInstanceOf[AnyRef] should be theSameInstanceAs i + toJavaValue(d).asInstanceOf[AnyRef] should be theSameInstanceAs d + toJavaValue(t).asInstanceOf[AnyRef] should be theSameInstanceAs t + } + + it should "pass null through" in { + // `shouldBe null` does not compile on an `Any`; this is the same assertion. + Option(toJavaValue(null)) shouldBe None + } + + /** A `VARBINARY` cell is a `byte[]`. An Array is not a Scala `Seq`, so it must survive untouched + * — converting it to a `java.util.List[Byte]` would break every consumer that casts to `[B`. + */ + it should "leave a byte array alone" in { + val bytes = Array[Byte](1, 2, 3) + toJavaValue(bytes).asInstanceOf[AnyRef] should be theSameInstanceAs bytes + } + + it should "convert an empty collection rather than passing it through" in { + toJavaValue(List.empty) shouldBe a[java.util.List[_]] + toJavaValue(ListMap.empty) shouldBe a[java.util.Map[_, _]] + } + + /** 🔴 The census, pinned against REAL core code rather than a hand-built shape. + * + * `toJavaValue` is only worth anything if core actually hands out Scala collections. This drives + * `ElasticConversion.jsonNodeToAny` — producer 1 — with a document whose field is an array of + * objects, and asserts BOTH halves: that what comes back is a Scala `List` of Scala `Map`s (so + * the problem is real), and that the conversion resolves it at every level (so the fix is). + * + * If a producer is ever changed to emit Java collections directly, this test says so instead of + * `toJavaValue` silently becoming dead code. + */ + it should "convert what ElasticConversion actually produces for a nested field" in { + val node = new com.fasterxml.jackson.databind.ObjectMapper() + .readTree("""{"addresses":[{"city":"Paris","zip":75001},{"city":"Lyon","zip":69001}]}""") + + val produced = ElasticConversion.jsonNodeToAny(node.get("addresses"), ListMap.empty) + + withClue("core no longer produces a Scala List here: ") { + produced shouldBe a[scala.collection.immutable.List[_]] + } + withClue("core no longer produces a Scala Map inside it: ") { + produced.asInstanceOf[List[Any]].head shouldBe a[scala.collection.Map[_, _]] + } + + val converted = toJavaValue(produced).asInstanceOf[java.util.List[Any]] + converted.size shouldBe 2 + converted.get(0) shouldBe a[java.util.Map[_, _]] + converted.get(0).asInstanceOf[java.util.Map[String, Any]].get("city") shouldBe "Paris" + converted.get(1).asInstanceOf[java.util.Map[String, Any]].get("zip") shouldBe 69001 + } + + /** The shapes core's three producers actually emit, asserted as one. If a fourth producer appears + * with a shape not covered here, this is where it should be added. + */ + it should "handle every shape core's read path produces" in { + // 1. jsonNodeToAny: an array of scalars, and a nested object + toJavaValue(List("a", "b")) shouldBe a[java.util.List[_]] + toJavaValue(ListMap("k" -> "v")) shouldBe a[java.util.Map[_, _]] + // 2. the aggregation path: a stats/percentiles ListMap as a CELL value + val stats = ListMap("count" -> 3L, "sum" -> 1.5, "avg" -> 0.5) + val javaStats = toJavaValue(stats).asInstanceOf[java.util.Map[String, Any]] + javaStats.get("count") shouldBe 3L + javaStats.size shouldBe 3 + // 3. extractInnerHits: a List of row-shaped ListMaps + val innerHits = List(ListMap("id" -> 1), ListMap("id" -> 2)) + val javaHits = toJavaValue(innerHits).asInstanceOf[java.util.List[Any]] + javaHits.size shouldBe 2 + javaHits.get(1) shouldBe a[java.util.Map[_, _]] + } +} From 8623a97e98fb03cf5c40e9f823151fec6e78c921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Thu, 10 Sep 2026 06:52:43 +0200 Subject: [PATCH 3/3] style(BIDC-10a): scalafmt JavaValueConversion's scaladoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's `core / Compile / scalafmtCheck` failed on this file. Scaladoc reflow only — verified that no non-comment line changes. 🔴 The reason it reached CI is worth more than the fix: **`scalafmtCheck` and `scalafmtAll` are VACUOUS in a git worktree here**, so every local "lint green" claim about a new file in this branch was meaningless. `.scalafmt.conf:17` sets `project.git = true`, so scalafmt takes its file list from git. This worktree carries `zz-worktree-nogit.sbt` — the documented shim for sbt-git's JGit failure inside a worktree — which stubs out that integration. The two combine into a check that enumerates ZERO files: `scalafmtAll` completes in one second, reports success, and formats nothing. `scalafmtOnly` with an explicit path is filtered the same way. Meanwhile `git ls-files` lists the file fine, so the file is tracked and CI, which runs in a normal checkout, checks it and fails. ⚠️ This is the second form of the same class in this story. The first was staging before formatting, so the commit held pre-format bytes while the gate read the formatted working tree. Both have the same shape: **a gate that reads something other than what CI reads proves nothing.** Here the gate read a file list that was empty; there it read a tree that was not the commit. Formatted by temporarily flipping `project.git` to false, running `scalafmtAll`, then restoring it — `.scalafmt.conf` is byte-identical to HEAD, verified, and exactly one file changed. Co-Authored-By: Claude Opus 5 (1M context) --- .../elastic/client/metadata/JavaValueConversion.scala | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala b/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala index c6b08607..0de9422b 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/metadata/JavaValueConversion.scala @@ -31,11 +31,10 @@ package app.softnetwork.elastic.client.metadata * * 1. `ElasticConversion.jsonNodeToAny` — the array branch (a Scala `List`) and the object branch * (`jsonNodeToMap`, a `ListMap`). 2. The aggregation path — a stats aggregation emits `name - * -> ListMap("count" -> ..., "sum" -> ...)` and `percentiles` emits `name -> ListMap( -> - * )`, both as CELL values, neither built from a `JsonNode` through `jsonNodeToAny`. - * 3. `ElasticConversion.extractInnerHits` — `innerHitName -> List[ListMap[String, Any]]`, - * hand assembled. That is precisely the `List`-of-`Map` shape a shallow conversion gets - * wrong. + * -> ListMap("count" -> ..., "sum" -> ...)` and `percentiles` emits `name -> ListMap( -> + * )`, both as CELL values, neither built from a `JsonNode` through `jsonNodeToAny`. 3. + * `ElasticConversion.extractInnerHits` — `innerHitName -> List[ListMap[String, Any]]`, hand + * assembled. That is precisely the `List`-of-`Map` shape a shallow conversion gets wrong. * * ⇒ The conversion belongs at the CONSUMER boundary, where it is producer-agnostic by * construction: it converts whatever value it is handed, so a fourth producer needs no change