From e3003e4db2d20b679f76ba091d39666be5bd0f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Wed, 9 Sep 2026 14:59:11 +0200 Subject: [PATCH 1/3] fix(BIDC-10a Part D): SHOW TABLES reports the client vocabulary, through one authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Story BIDC-10a Part D (AC 10, AC 12, T7). `SHOW TABLES` published elasticsql's INTERNAL table-type vocabulary: the `type` column was `TableType.name.toUpperCase`, so a plain index came back as `REGULAR` - a value no client consumes. The sidecar's ADBC leg failed on exactly that (`Expected 'TABLE' in ['REGULAR']`), and the same column is what a JDBC `getTables`, a Flight SQL `GET_TABLES` and an ODBC object browser receive, because all three execute `SHOW TABLES` through the gateway. Fixing it in each transport would mean a translation layer per client, all of which must be kept in step; the lead's ruling is to fix it once, at the source. `TableType` gains `sqlName`, the single authority for what clients are told: Regular -> TABLE · View -> VIEW · MaterializedView -> MATERIALIZED_VIEW External -> EXTERNAL · Changelog -> CHANGELOG · Enrichment -> ENRICHMENT `sqlName` is ABSTRACT on the sealed trait on purpose: a seventh table type cannot compile until someone decides what clients should call it. That is stronger than a test, and it is precisely the default that let `REGULAR` ship. A materialized view keeps its OWN type rather than collapsing into `VIEW` (AD-A-6): it has storage, a refresh schedule and a watcher, and JDBC permits arbitrary type strings. STORAGE IS UNTOUCHED. `TableType.name` remains the `_meta.type` key, `TableType.apply` still parses only the stored name, and an existing index whose mapping says `"regular"` still reads back as `Regular`. No reindex, no migration. Evidence, both halves of AC 10: - `TableTypeVocabularySpec` (sql, 8 tests) enumerates `TableType` by walking the COMPILED package - the trait is sealed, so the walk is complete and needs no allow-list - asserts the stored name and the client name for every type, pins `Regular.sqlName != "REGULAR"` literally, and asserts the round-trip `TableType(t.name) == t`. - `ShowTablesTableTypeSpec` (core, 4 tests) drives the real `TableExecutor` projection Docker-free from mapping JSON carrying the STORED `_meta.type`, covering all six types, the no-`_meta` default and MV-vs-VIEW. Falsified: reverting the projection turns 3 of 4 core tests red; `Regular.sqlName = "REGULAR"` turns 2 of 8 sql tests red; dropping one type from the expectation table turns 3 red with a clue naming it, which is what proves the walk is live. Release note: the `SHOW TABLES` `type` column changes value for every plain index (`REGULAR` -> `TABLE`), and materialized views become their own node type in BI browsers - knowingly reversing part of story 20.3's PD-4, on the measurement that did not exist then. The driver-side shims (arrow `normaliseTableType`, jdbc `contains("regular")` / `contains("view")`) are deliberately NOT removed here: they come out at the repin onto a published core carrying this change, never before (AD-A-7). --- .../elastic/client/GatewayApi.scala | 5 +- .../client/ShowTablesTableTypeSpec.scala | 116 ++++++++++++ documentation/client/repl.md | 4 +- documentation/sql/dql_statements.md | 2 +- .../elastic/sql/schema/package.scala | 37 ++++ .../sql/schema/TableTypeVocabularySpec.scala | 174 ++++++++++++++++++ 6 files changed, 334 insertions(+), 4 deletions(-) create mode 100644 core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala create mode 100644 sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala diff --git a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala index ad83f879d..7d928fe3e 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala @@ -551,7 +551,10 @@ class TableExecutor( .map { case (index, mappings) => ListMap( "name" -> index, - "type" -> mappings.tableType.name.toUpperCase, + // BIDC-10a Part D / AD-A-6 — the CLIENT vocabulary, not the stored name. + // `TableType.sqlName` is the single authority; `name` stays the + // `_meta.type` storage key and never reaches a client. + "type" -> mappings.tableType.sqlName, "pk" -> mappings.primaryKey.mkString(","), "partitioned" -> mappings.partitionBy .map(p => s"PARTITION BY ${p.column} (${p.granularity})") diff --git a/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala new file mode 100644 index 000000000..c95701b40 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala @@ -0,0 +1,116 @@ +/* + * 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 + +import akka.actor.ActorSystem +import app.softnetwork.elastic.client.result._ +import app.softnetwork.elastic.sql.schema.TableType +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.slf4j.{Logger, LoggerFactory} + +import scala.collection.immutable.ListMap +import scala.concurrent.duration._ + +/** Story BIDC-10a Part D (AC 10) — the `type` column of `SHOW TABLES`, end to end, Docker-free. + * + * `TableTypeVocabularySpec` (sql) pins `TableType.sqlName` itself. This one pins the only thing + * that makes it matter: that the projection in `TableExecutor` actually publishes it, starting + * from a mapping whose `_meta.type` carries the STORED name. The same seam serves the JDBC + * `getTables`, the Flight SQL `GET_TABLES` and the REPL, all three of which execute `SHOW TABLES` + * through the gateway. + */ +class ShowTablesTableTypeSpec + extends AnyFlatSpec + with Matchers + with ScalaFutures + with BeforeAndAfterAll { + + implicit private val system: ActorSystem = ActorSystem("show-tables-table-type") + override implicit val patienceConfig: PatienceConfig = + PatienceConfig(timeout = scaled(5.seconds)) + + override def afterAll(): Unit = { + system.terminate() + super.afterAll() + } + + /** A mapping exactly as this engine writes it: the STORED `_meta.type` name, never the display + * value. If `TableType.apply` ever stopped reading these, every one of these rows would fall + * back to `Regular` and the expectations below would redden. + */ + private def mappingOf(t: TableType): String = + s"""{"_meta":{"type":"${t.name}"},"properties":{"name":{"type":"keyword"}}}""" + + private class StubMappings(mappings: Map[String, String]) extends NopeClientApi { + override protected def logger: Logger = LoggerFactory.getLogger(getClass) + override private[client] def executeGetAllMappings( + indices: Seq[String] + ): ElasticResult[Map[String, String]] = ElasticResult.success(mappings) + } + + private def showTables(mappings: Map[String, String]): Seq[ListMap[String, Any]] = + new StubMappings(mappings).run("SHOW TABLES").futureValue match { + case ElasticSuccess(QueryRows(rows, _)) => rows + case other => fail(s"expected QueryRows, got $other") + } + + private val allTypes: Seq[TableType] = Seq( + TableType.Regular, + TableType.View, + TableType.MaterializedView, + TableType.External, + TableType.Changelog, + TableType.Enrichment + ) + + "SHOW TABLES" should "report the client vocabulary for every table type" in { + val indices = allTypes.map(t => s"idx_${t.name}" -> mappingOf(t)).toMap + val byName = showTables(indices).map { r => + r("name").toString -> r("type").toString + }.toMap + + byName shouldBe allTypes.map(t => s"idx_${t.name}" -> t.sqlName).toMap + } + + it should "report a plain index as TABLE, never REGULAR" in { + val rows = showTables(Map("orders" -> mappingOf(TableType.Regular))) + rows.map(_("type").toString) shouldBe Seq("TABLE") + } + + /** A mapping with no `_meta.type` at all — every index created before table types existed. It + * defaults to `Regular` and must therefore also read as `TABLE`. + */ + it should "report an index with no _meta as TABLE" in { + val rows = showTables(Map("legacy" -> """{"properties":{"name":{"type":"keyword"}}}""")) + rows.map(_("type").toString) shouldBe Seq("TABLE") + } + + it should "keep a materialized view distinct from a view" in { + val rows = showTables( + Map( + "orders_mv" -> mappingOf(TableType.MaterializedView), + "orders_v" -> mappingOf(TableType.View) + ) + ).map(r => r("name").toString -> r("type").toString).toMap + + rows("orders_mv") shouldBe "MATERIALIZED_VIEW" + rows("orders_v") shouldBe "VIEW" + } +} diff --git a/documentation/client/repl.md b/documentation/client/repl.md index f1d0deb9e..2ce8cfd67 100644 --- a/documentation/client/repl.md +++ b/documentation/client/repl.md @@ -697,7 +697,7 @@ sql> SHOW TABLES LIKE 'show_%'; | name | type | pk | partitioned | |------------|---------|----|-------------| -| show_users | REGULAR | id | | +| show_users | TABLE | id | | 📊 1 row(s) (7ms) ``` @@ -964,7 +964,7 @@ sql> tables | name | type | pk | partitioned | |------------|---------|----|-------------| -| demo_users | REGULAR | id | | +| demo_users | TABLE | id | | 📊 1 row(s) (5ms) sql> \dt demo_users diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index bce126201..76cadd3e5 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -1312,7 +1312,7 @@ SHOW TABLES LIKE 'show_%'; | name | type | pk | partitioned | |------------|---------|----|-------------| -| show_users | REGULAR | id | | +| show_users | TABLE | id | | 📊 1 row(s) (7ms) --- diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala index 09d91feb4..28522a594 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala @@ -1655,30 +1655,67 @@ package object schema { } } + /** The type of a table, as recorded in the index mapping's `_meta.type`. + * + * Two names, deliberately, because they answer two different questions and only one of them is + * allowed to change: + * + * - `name` is what is STORED. It is written into `_meta.type` and read back by + * `TableType.apply`, so an existing index whose mapping says `"regular"` depends on it byte + * for byte. It is never a display value. + * - `sqlName` is what is SHOWN — the `type` column of `SHOW TABLES`, and therefore the value a + * JDBC `getTables`, a Flight SQL `GET_TABLES` and an ODBC object browser receive, since all + * three execute `SHOW TABLES` through the gateway. It must be a table type those clients + * recognise. + * + * The projection used to be `name.toUpperCase`, which published the internal `REGULAR` to every + * client. Measured: the sidecar's ADBC leg failed `Expected 'TABLE' in ['REGULAR']`. Reasoned + * from that, not measured: a client that filters on the standard vocabulary sees no tables at + * all. Story BIDC-10a Part D / AD-A-6. + * + * `sqlName` is ABSTRACT on purpose: a seventh table type cannot compile until someone decides + * what clients should call it. That decision must not default silently. + */ sealed trait TableType { def name: String + def sqlName: String } object TableType { case object Regular extends TableType { override def name: String = "regular" + override def sqlName: String = "TABLE" } case object External extends TableType { override def name: String = "external" + override def sqlName: String = "EXTERNAL" } case object Changelog extends TableType { override def name: String = "changelog" + override def sqlName: String = "CHANGELOG" } case object Enrichment extends TableType { override def name: String = "enrichment" + override def sqlName: String = "ENRICHMENT" } case object View extends TableType { override def name: String = "view" + override def sqlName: String = "VIEW" } + + /** A materialized view keeps its OWN type rather than collapsing into `VIEW`: it has storage, a + * refresh schedule and (optionally) a watcher, JDBC permits arbitrary type strings, and + * collapsing it would lose information at the source for every client at once. + */ case object MaterializedView extends TableType { override def name: String = "materialized_view" + override def sqlName: String = "MATERIALIZED_VIEW" } + /** Parses the STORED name (`_meta.type`), never the display `sqlName`. Widening it to accept + * `"TABLE"` would make two spellings of the same type storable and is not what any caller + * needs: every caller reads a mapping this engine wrote. + */ def apply(name: String): TableType = name.toLowerCase match { case "regular" => Regular diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala new file mode 100644 index 000000000..5cb0f06da --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala @@ -0,0 +1,174 @@ +package app.softnetwork.elastic.sql.schema + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.io.File +import java.lang.reflect.Modifier +import java.net.URL +import scala.util.Try + +/** Story BIDC-10a Part D (AC 10) — `SHOW TABLES` publishes the CLIENT vocabulary, and it does so + * through one authority so the display value cannot drift from the stored one. + * + * The projection used to be `TableType.name.toUpperCase`, so a plain index was advertised as + * `REGULAR` — a value no client consumes. The ADBC leg of the sidecar's integration suite failed + * with `Expected 'TABLE' in ['REGULAR']`, and an ODBC object browser filtering on the standard + * vocabulary shows no tables at all. `TableType.sqlName` is now the single source of that value + * and `TableType.name` remains the `_meta.type` storage key. + * + * 🔴 The enumeration is derived from the COMPILED package, never from a hand-written list. + * `TableType` is `sealed`, so the compiler guarantees every subtype is declared in the same file + * and compiled into the same package directory; listing that directory is therefore complete and + * needs no allow-list. A seventh table type lands here and reddens `expected`, which is the point: + * this is where the vocabulary is re-decided. (Same mechanism, and the same lesson, as + * `HelpCorpusSpec`'s AST walk — an enumeration by anything narrower silently misses a leaf.) + */ +class TableTypeVocabularySpec extends AnyFlatSpec with Matchers { + + /** The reconciled table (story spec, section Reconciled values D.3): TableType -> (stored + * `_meta.type` name, `SHOW TABLES` value). The stored column is what must NOT move. + */ + private val expected: Map[TableType, (String, String)] = Map( + TableType.Regular -> (("regular", "TABLE")), + TableType.View -> (("view", "VIEW")), + TableType.MaterializedView -> (("materialized_view", "MATERIALIZED_VIEW")), + TableType.External -> (("external", "EXTERNAL")), + TableType.Changelog -> (("changelog", "CHANGELOG")), + TableType.Enrichment -> (("enrichment", "ENRICHMENT")) + ) + + /** Every `TableType` the compiler knows about, read off the compiled package directory. + * + * 🔴 `getResources`, not `getResource`. This spec lives in the SAME package as the type it + * enumerates, so `sql/target/.../test-classes/app/.../schema` shadows the main classes directory + * and the singular lookup returns the test tree — which holds no `TableType` at all. The first + * version of this walk therefore enumerated NOTHING, and every per-type assertion below passed + * VACUOUSLY over an empty sequence. Union every classpath root that carries the package, and + * make emptiness a failure HERE so no caller can be vacuous. + * + * A non-`file:` classpath entry FAILS rather than skips, for the same reason. A concrete subtype + * that is not a Scala `object` likewise fails rather than being dropped. + */ + private def declaredTableTypes: Seq[TableType] = { + val pkgPath = classOf[TableType].getName.split('.').init.mkString("/") + val loader = classOf[TableType].getClassLoader + val roots: Seq[URL] = { + val e = loader.getResources(pkgPath) + val b = Seq.newBuilder[URL] + while (e.hasMoreElements) b += e.nextElement() + b.result() + } + if (roots.isEmpty) fail(s"the schema package `$pkgPath` is not on the test classpath") + roots.foreach { url => + if (url.getProtocol != "file") + fail( + s"the schema package `$pkgPath` resolves to a ${url.getProtocol} URL ($url). This walk " + + "lists the compiled classes of a SEALED hierarchy and needs a directory; teach it to " + + "read the archive rather than letting the vocabulary guard go vacuous." + ) + } + val subtypes: Seq[Class[_]] = roots + .flatMap(url => Option(new File(url.toURI).listFiles).getOrElse(Array.empty[File]).toSeq) + .filter(f => f.isFile && f.getName.endsWith(".class")) + .map(f => pkgPath.replace('/', '.') + "." + f.getName.stripSuffix(".class")) + .distinct + .sorted + // The `Option[Class[_]]` needs its type written out: on the 2.12 leg the existential defeats + // `flatMap`'s inference. + .flatMap { n => + val loaded: Option[Class[_]] = Try(Class.forName(n, false, loader)).toOption + loaded.toSeq + } + .filter(c => classOf[TableType].isAssignableFrom(c)) + .filterNot(c => c.isInterface || Modifier.isAbstract(c.getModifiers)) + .distinct + + if (subtypes.isEmpty) + fail( + s"no concrete TableType was found under $roots. Every assertion in this spec iterates " + + "this sequence, so an empty result would make them all pass vacuously." + ) + + subtypes.map { c => + val instance: Option[TableType] = Try( + Class.forName(c.getName, true, loader).getField("MODULE$").get(null).asInstanceOf[TableType] + ).toOption + instance.getOrElse( + fail( + s"`${c.getName}` is a concrete TableType that is not a Scala `object`; this walk can " + + "only instantiate case objects. Teach it, or the vocabulary guard silently loses a type." + ) + ) + } + } + + "The TableType hierarchy" should "be enumerated exactly by the reconciled vocabulary table" in { + val declared = declaredTableTypes + declared should not be empty + val missing = declared.toSet.diff(expected.keySet) + val stale = expected.keySet.diff(declared.toSet) + withClue( + s"undocumented table types: ${missing.map(_.name).mkString(", ")}; " + + s"table types in the expectation that no longer exist: ${stale.map(_.name).mkString(", ")}. " + + "A new TableType must decide what clients call it -- see TableType.sqlName. " + ) { + missing shouldBe empty + stale shouldBe empty + } + declared.size shouldBe expected.size + } + + it should "keep the STORED name unchanged for every type" in { + declaredTableTypes.foreach { t => + withClue(s"stored `_meta.type` name of $t: ") { + t.name shouldBe expected(t)._1 + } + } + } + + it should "publish the client vocabulary as sqlName for every type" in { + declaredTableTypes.foreach { t => + withClue(s"`SHOW TABLES` value of $t: ") { + t.sqlName shouldBe expected(t)._2 + } + } + } + + /** The literal pin. Property assertions survive a vocabulary change; a literal one catches it — + * that asymmetry is precisely what let `REGULAR` ship (BIDC-6's `TableTypeDomainSpec` asserted + * the defect as intended behaviour and stayed green). + */ + it should "never advertise the internal name of a plain index" in { + TableType.Regular.sqlName shouldBe "TABLE" + TableType.Regular.sqlName should not be "REGULAR" + declaredTableTypes.map(_.sqlName) should not contain "REGULAR" + } + + it should "keep MATERIALIZED_VIEW distinct from VIEW" in { + TableType.MaterializedView.sqlName should not be TableType.View.sqlName + } + + it should "give every type a distinct, well-formed client name" in { + val names = declaredTableTypes.map(_.sqlName) + names.distinct.size shouldBe names.size + names.foreach(n => n should fullyMatch regex "[A-Z][A-Z_]*") + } + + /** 🔴 The migration guard. Adding a display name must NOT make an existing index's stored + * `_meta.type` unreadable: `TableType.apply` still parses the stored name, and only the stored + * name. + */ + it should "still parse every stored name back to its own type" in { + declaredTableTypes.foreach { t => + withClue(s"round-trip of $t through _meta.type: ") { + TableType(t.name) shouldBe t + TableType(t.name.toUpperCase) shouldBe t + } + } + } + + it should "keep parsing the legacy stored value of a plain index" in { + TableType("regular") shouldBe TableType.Regular + } +} From 460b5fb7743d6cf80d9a7169abae4cc1397c3dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Wed, 9 Sep 2026 16:30:55 +0200 Subject: [PATCH 2/3] fix(BIDC-10a Part D): the SHOW TABLE header is the SECOND display projection Independent review round. Five findings, all addressed here; nothing deferred. 1. `ResultRenderer.scala:225` renders the `SHOW TABLE ` header (REPL `\st `) as `s"[${table.tableType}]"` - the case-object toString, `Regular` / `MaterializedView`. That is why no `tableType.name` search found it, and why the first commit called `GatewayApi.scala:554` the only display projection. Before this story both surfaces spoke the internal vocabulary and merely disagreed on case. Fixing one of them did not remove a divergence, it CREATED one - measurable in this repo's own walkthrough, where `SHOW TABLES` says `TABLE` at `documentation/sql/dql_statements.md:1315` and `SHOW TABLE users` said `[Regular]` 35 lines below at :1350; same pairing at `documentation/client/repl.md:967` versus :846. Both projections now read `TableType.sqlName`, the two doc lines are updated, and three new tests pin the header per type, pin that it is never the Scala name, and pin that it AGREES with the `SHOW TABLES` type column for every type. Lesson worth keeping: enumerate a concept's renderings BY TYPE, not by expression. A grep for the expression you are replacing is blind to the interpolation of the value itself. 2. The new scaladoc asserted that `name` "is never a display value". That is falsified 80 lines below it: `Table.merge` throws `Cannot alter table of type ${tableType.name}`, a user-facing message. So `name` has THREE consumers, not the two the design gate counted - and that count was the justification for the storage/display split being safe. The message stays: an engine refusing an operation on its own construct legitimately names the engine's own type. What changes is that the exception is now WRITTEN DOWN, in `TableType`'s scaladoc and at the throw site, with the rule that a new read of `name` outside `_meta.type` and that message is a design change. Silence about a projection is exactly what let `REGULAR` reach clients. 3. elasticsql had no live-cluster assertion of the new value, and ES 6.8 had none anywhere (the only real-ES assertion lives in extensions, inside an `assume` that 6.8 cancels). `GatewayApiIntegrationSpec` (template) now asserts `show_users -> TABLE`. Green on all five clients: ES 6.8 rest 73+1 pre-existing cancel, 6.8 jest 73+1, 7.17 74/74, 8.18 74/74, 9.0 74/74. 4. `ShowTablesTableTypeSpec` hand-listed the six types, so a seventh would redden the sql vocabulary spec and leave the projection guard green - silently under-covering the thing it exists to guard. The sealed-hierarchy walk is extracted to `TableTypeEnumeration` and BOTH specs derive from it. `core` reaches it because `core -> macros -> sql` carries `test->test`. 5. One of the walk's exits swallowed `ClassNotFoundException` while the other two failed loudly, contrary to its own comment. All five exits now fail: no root, non-`file:` root, unloadable class, non-`object` subtype, empty result. The scaladoc says when a `jar:` root actually happens - running these assertions against a published `softclient4es-sql` jar - rather than leaving the reader to guess. Verified: sql 1043/1043, core 956/956, `+ sql/compile` `+ core/compile`, `++ 2.12.20 {sql,core}/Test/compile`, lint. Falsified: reverting `:225` turns exactly the three new header tests red. Release note (extended): BOTH client-facing renderings change together - the `type` column of `SHOW TABLES` and the `SHOW TABLE ` header, which printed `Table: users [Regular]` and now prints `[TABLE]`. --- .../client/result/ResultRenderer.scala | 7 +- .../client/ShowTablesTableTypeSpec.scala | 54 ++++++++-- documentation/client/repl.md | 2 +- documentation/sql/dql_statements.md | 2 +- .../elastic/sql/schema/package.scala | 25 ++++- .../sql/schema/TableTypeEnumeration.scala | 100 ++++++++++++++++++ .../sql/schema/TableTypeVocabularySpec.scala | 72 +------------ .../client/GatewayApiIntegrationSpec.scala | 9 ++ 8 files changed, 186 insertions(+), 85 deletions(-) create mode 100644 sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeEnumeration.scala diff --git a/core/src/main/scala/app/softnetwork/elastic/client/result/ResultRenderer.scala b/core/src/main/scala/app/softnetwork/elastic/client/result/ResultRenderer.scala index 874eb2432..ea7d0e7ae 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/result/ResultRenderer.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/result/ResultRenderer.scala @@ -222,7 +222,12 @@ object ResultRenderer { // Table header output.append( s"${emoji("📋")} ${bold(cyan(s"Table: ${table.name}"))} " + - gray(s"[${table.tableType}]") + + // BIDC-10a Part D - the SECOND display projection. This is the `SHOW TABLE ` header + // (REPL `\st
`), which used to interpolate the case object itself and print + // `[Regular]` - the Scala type name, which no client vocabulary contains and which a + // `tableType.name` search cannot find. It now reads the SAME authority as the `type` + // column of `SHOW TABLES`, so the two cannot disagree inside one REPL session. + gray(s"[${table.tableType.sqlName}]") + "\n\n" ) diff --git a/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala index c95701b40..2467ab45a 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala @@ -18,7 +18,7 @@ package app.softnetwork.elastic.client import akka.actor.ActorSystem import app.softnetwork.elastic.client.result._ -import app.softnetwork.elastic.sql.schema.TableType +import app.softnetwork.elastic.sql.schema.{Table, TableType, TableTypeEnumeration} import org.scalatest.BeforeAndAfterAll import org.scalatest.concurrent.ScalaFutures import org.scalatest.flatspec.AnyFlatSpec @@ -71,14 +71,14 @@ class ShowTablesTableTypeSpec case other => fail(s"expected QueryRows, got $other") } - private val allTypes: Seq[TableType] = Seq( - TableType.Regular, - TableType.View, - TableType.MaterializedView, - TableType.External, - TableType.Changelog, - TableType.Enrichment - ) + /** Derived from the SAME sealed-hierarchy walk `TableTypeVocabularySpec` uses (reachable here + * because `core -> macros -> sql` carries `test->test`), never hand-listed. + * + * A hand-written list would leave THIS spec green when a seventh table type is added while the + * sql-side vocabulary spec goes red - i.e. the projection guard would silently stop covering the + * projection, which is the exact shape of the defect this story closes. + */ + private val allTypes: Seq[TableType] = TableTypeEnumeration.declaredTableTypes "SHOW TABLES" should "report the client vocabulary for every table type" in { val indices = allTypes.map(t => s"idx_${t.name}" -> mappingOf(t)).toMap @@ -102,6 +102,42 @@ class ShowTablesTableTypeSpec rows.map(_("type").toString) shouldBe Seq("TABLE") } + // -- The SECOND display projection: the `SHOW TABLE ` header (REPL `\st
`) ---------- + // + // `ResultRenderer.renderTableDefinition` interpolated the case object itself and printed + // `[Regular]` - the Scala type name. Both projections now read `TableType.sqlName`, so a REPL + // session cannot show one vocabulary in `SHOW TABLES` and another in `SHOW TABLE`. + + private def showTableHeader(t: TableType): String = + ResultRenderer.renderAscii(TableResult(Table("users", Nil, tableType = t)), 1.milli) + + "SHOW TABLE" should "head its definition with the same vocabulary for every type" in { + allTypes.foreach { t => + withClue(s"`SHOW TABLE` header for $t: ") { + showTableHeader(t) should include(s"[${t.sqlName}]") + } + } + } + + it should "never head a plain table with the Scala type name" in { + val header = showTableHeader(TableType.Regular) + header should include("[TABLE]") + header should not include "[Regular]" + } + + /** The divergence this closes was measurable in the repo's own walkthrough: `SHOW TABLES` and + * `SHOW TABLE` appear 35 lines apart in `documentation/sql/dql_statements.md` and used to + * disagree. + */ + it should "agree with the SHOW TABLES type column" in { + allTypes.foreach { t => + val listed = showTables(Map("users" -> mappingOf(t))).head("type").toString + withClue(s"SHOW TABLES says `$listed` for $t; SHOW TABLE header: ") { + showTableHeader(t) should include(s"[$listed]") + } + } + } + it should "keep a materialized view distinct from a view" in { val rows = showTables( Map( diff --git a/documentation/client/repl.md b/documentation/client/repl.md index 2ce8cfd67..5d1196ca8 100644 --- a/documentation/client/repl.md +++ b/documentation/client/repl.md @@ -843,7 +843,7 @@ Show detailed table information: ``` sql> \st users -📋 Table: users [Regular] +📋 Table: users [TABLE] ... ``` diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index 76cadd3e5..228facf02 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -1347,7 +1347,7 @@ CREATE TABLE IF NOT EXISTS users ( SHOW TABLE users; ``` -📋 Table: users [Regular] +📋 Table: users [TABLE] | Field | Type | Null | Key | Default | Comment | Script | Extra | |-------------------|-----------|------|-----|-------------------|-----------------|-------------------------------------------------|---------------------------------------------------| diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala index 28522a594..751e63444 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala @@ -1662,11 +1662,11 @@ package object schema { * * - `name` is what is STORED. It is written into `_meta.type` and read back by * `TableType.apply`, so an existing index whose mapping says `"regular"` depends on it byte - * for byte. It is never a display value. - * - `sqlName` is what is SHOWN — the `type` column of `SHOW TABLES`, and therefore the value a - * JDBC `getTables`, a Flight SQL `GET_TABLES` and an ODBC object browser receive, since all - * three execute `SHOW TABLES` through the gateway. It must be a table type those clients - * recognise. + * for byte. + * - `sqlName` is what is SHOWN to a CLIENT — the `type` column of `SHOW TABLES` and the `SHOW + * TABLE ` header, and therefore the value a JDBC `getTables`, a Flight SQL `GET_TABLES`, + * an ODBC object browser and the REPL receive, since all of them go through the gateway. It + * must be a table type those clients recognise. * * The projection used to be `name.toUpperCase`, which published the internal `REGULAR` to every * client. Measured: the sidecar's ADBC leg failed `Expected 'TABLE' in ['REGULAR']`. Reasoned @@ -1675,6 +1675,18 @@ package object schema { * * `sqlName` is ABSTRACT on purpose: a seventh table type cannot compile until someone decides * what clients should call it. That decision must not default silently. + * + * ⚠️ EXACTLY ONE place outside storage reads `name`, and it is deliberate: `Table.merge` throws + * `Cannot alter table of type ` (below, in this file) when an ALTER targets a + * non-regular table. That message is the ENGINE refusing an operation on its own construct, not + * a catalogue answer to a client, so it legitimately names the engine's own type. It is listed + * here rather than left silent, because the whole point of this split is that no display of a + * table type may be discovered by surprise — that is how `REGULAR` reached clients, and how the + * `SHOW TABLE` header (`ResultRenderer`) went on printing the Scala type name `Regular` + * unnoticed: neither could be found by searching for `tableType.name`. + * + * A new read of `name` outside `_meta.type` and that one error message is a design change, not a + * detail: use `sqlName`, or amend this list. */ sealed trait TableType { def name: String @@ -1884,6 +1896,9 @@ package object schema { def merge(statements: Seq[AlterTableStatement]): Table = { if (!isRegular) + // BIDC-10a Part D - the deliberate exception recorded in TableType's scaladoc: this is the + // engine refusing an operation on its own construct, so it names the STORED type. Every + // client-facing rendering of a table type uses `TableType.sqlName` instead. throw new Exception(s"Cannot alter table $name of type ${tableType.name}") statements .foldLeft(this) { (table, statement) => diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeEnumeration.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeEnumeration.scala new file mode 100644 index 000000000..95961be34 --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeEnumeration.scala @@ -0,0 +1,100 @@ +package app.softnetwork.elastic.sql.schema + +import org.scalatest.Assertions + +import java.io.File +import java.lang.reflect.Modifier +import java.net.URL + +/** The ONE enumeration of `TableType`, shared by every spec that must not miss a type. + * + * Story BIDC-10a Part D. `TableType` is `sealed`, so the compiler guarantees every subtype is + * declared in the same file and compiled into the same package directory; listing that directory + * is therefore COMPLETE and needs no allow-list. A spec that hand-lists the types instead stays + * green when a seventh is added, silently under-covering the very thing it guards - which is why + * this lives here rather than being written twice. + * + * It is reachable from `core` because `core -> macros -> sql` carries `test->test`, so + * `sql/target/scala-2.13/test-classes` is on core's test classpath. + * + * 🔴 Every way out of this walk FAILS; none skips. + * + * 1. `getResources`, not `getResource`. A spec living in the SAME package as `TableType` puts a + * `test-classes/app/.../schema` directory on the classpath ahead of the main one, and the + * singular lookup then returns the test tree, which holds no `TableType` at all. The walk + * returns EMPTY and every per-type assertion downstream passes VACUOUSLY. Measured, on the + * first version of `TableTypeVocabularySpec`. Union every root. 2. A non-`file:` root fails + * rather than being skipped. It is not hypothetical: running these assertions against a + * PUBLISHED `softclient4es-sql` jar (an overlay run, or a spec moved to a module that + * consumes the artifact rather than the project) makes the package resolve to a `jar:` URL. + * Better a loud "teach me to read the archive" than a guard that quietly stops guarding. 3. A + * `.class` in the package that cannot be loaded fails. A swallowed `ClassNotFoundException` + * here would mean a `TableType` whose client vocabulary was never decided, which is the whole + * defect this story closes. 4. A concrete subtype that is not a Scala `object` fails. 5. An + * empty result fails HERE, inside the enumerator, so that no caller can be vacuous even if it + * forgets to check. + */ +object TableTypeEnumeration extends Assertions { + + def declaredTableTypes: Seq[TableType] = { + val pkgPath = classOf[TableType].getName.split('.').init.mkString("/") + val loader = classOf[TableType].getClassLoader + val roots: Seq[URL] = { + val e = loader.getResources(pkgPath) + val b = Seq.newBuilder[URL] + while (e.hasMoreElements) b += e.nextElement() + b.result() + } + if (roots.isEmpty) fail(s"the schema package `$pkgPath` is not on the test classpath") + roots.foreach { url => + if (url.getProtocol != "file") + fail( + s"the schema package `$pkgPath` resolves to a ${url.getProtocol} URL ($url). This walk " + + "lists the compiled classes of a SEALED hierarchy and needs a directory; teach it to " + + "read the archive rather than letting the vocabulary guard go vacuous." + ) + } + val subtypes: Seq[Class[_]] = roots + .flatMap(url => Option(new File(url.toURI).listFiles).getOrElse(Array.empty[File]).toSeq) + .filter(f => f.isFile && f.getName.endsWith(".class")) + .map(f => pkgPath.replace('/', '.') + "." + f.getName.stripSuffix(".class")) + .distinct + .sorted + .map { n => + try Class.forName(n, false, loader) + catch { + case t: Throwable => + fail( + s"`$n` is a compiled class in the schema package and could not be loaded " + + s"(${t.getClass.getSimpleName}: ${t.getMessage}). This walk must not skip a class " + + "silently - a skipped TableType is a client vocabulary nobody decided.", + t + ) + } + } + .filter(c => classOf[TableType].isAssignableFrom(c)) + .filterNot(c => c.isInterface || Modifier.isAbstract(c.getModifiers)) + .distinct + + if (subtypes.isEmpty) + fail( + s"no concrete TableType was found under $roots. Every assertion built on this walk " + + "iterates its result, so an empty one would make them all pass vacuously." + ) + + subtypes.map { c => + val module = + try Class.forName(c.getName, true, loader).getField("MODULE$").get(null) + catch { + case t: Throwable => + fail( + s"`${c.getName}` is a concrete TableType that is not a Scala `object`; this walk " + + "can only instantiate case objects. Teach it, or the vocabulary guard silently " + + "loses a type.", + t + ) + } + module.asInstanceOf[TableType] + } + } +} diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala index 5cb0f06da..75ca4f978 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala @@ -3,11 +3,6 @@ package app.softnetwork.elastic.sql.schema import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import java.io.File -import java.lang.reflect.Modifier -import java.net.URL -import scala.util.Try - /** Story BIDC-10a Part D (AC 10) — `SHOW TABLES` publishes the CLIENT vocabulary, and it does so * through one authority so the display value cannot drift from the stored one. * @@ -38,70 +33,11 @@ class TableTypeVocabularySpec extends AnyFlatSpec with Matchers { TableType.Enrichment -> (("enrichment", "ENRICHMENT")) ) - /** Every `TableType` the compiler knows about, read off the compiled package directory. - * - * 🔴 `getResources`, not `getResource`. This spec lives in the SAME package as the type it - * enumerates, so `sql/target/.../test-classes/app/.../schema` shadows the main classes directory - * and the singular lookup returns the test tree — which holds no `TableType` at all. The first - * version of this walk therefore enumerated NOTHING, and every per-type assertion below passed - * VACUOUSLY over an empty sequence. Union every classpath root that carries the package, and - * make emptiness a failure HERE so no caller can be vacuous. - * - * A non-`file:` classpath entry FAILS rather than skips, for the same reason. A concrete subtype - * that is not a Scala `object` likewise fails rather than being dropped. + /** The enumeration is derived from the COMPILED package, never from a hand-written list, and it + * is shared with `ShowTablesTableTypeSpec` so the two cannot disagree about how many types + * exist. Every drop path in that walk FAILS rather than skipping - see [[TableTypeEnumeration]]. */ - private def declaredTableTypes: Seq[TableType] = { - val pkgPath = classOf[TableType].getName.split('.').init.mkString("/") - val loader = classOf[TableType].getClassLoader - val roots: Seq[URL] = { - val e = loader.getResources(pkgPath) - val b = Seq.newBuilder[URL] - while (e.hasMoreElements) b += e.nextElement() - b.result() - } - if (roots.isEmpty) fail(s"the schema package `$pkgPath` is not on the test classpath") - roots.foreach { url => - if (url.getProtocol != "file") - fail( - s"the schema package `$pkgPath` resolves to a ${url.getProtocol} URL ($url). This walk " + - "lists the compiled classes of a SEALED hierarchy and needs a directory; teach it to " + - "read the archive rather than letting the vocabulary guard go vacuous." - ) - } - val subtypes: Seq[Class[_]] = roots - .flatMap(url => Option(new File(url.toURI).listFiles).getOrElse(Array.empty[File]).toSeq) - .filter(f => f.isFile && f.getName.endsWith(".class")) - .map(f => pkgPath.replace('/', '.') + "." + f.getName.stripSuffix(".class")) - .distinct - .sorted - // The `Option[Class[_]]` needs its type written out: on the 2.12 leg the existential defeats - // `flatMap`'s inference. - .flatMap { n => - val loaded: Option[Class[_]] = Try(Class.forName(n, false, loader)).toOption - loaded.toSeq - } - .filter(c => classOf[TableType].isAssignableFrom(c)) - .filterNot(c => c.isInterface || Modifier.isAbstract(c.getModifiers)) - .distinct - - if (subtypes.isEmpty) - fail( - s"no concrete TableType was found under $roots. Every assertion in this spec iterates " + - "this sequence, so an empty result would make them all pass vacuously." - ) - - subtypes.map { c => - val instance: Option[TableType] = Try( - Class.forName(c.getName, true, loader).getField("MODULE$").get(null).asInstanceOf[TableType] - ).toOption - instance.getOrElse( - fail( - s"`${c.getName}` is a concrete TableType that is not a Scala `object`; this walk can " + - "only instantiate case objects. Teach it, or the vocabulary guard silently loses a type." - ) - ) - } - } + private def declaredTableTypes: Seq[TableType] = TableTypeEnumeration.declaredTableTypes "The TableType hierarchy" should "be enumerated exactly by the reconciled vocabulary table" in { val declared = declaredTableTypes diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala index 96477cd2a..4501c95d3 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala @@ -71,6 +71,15 @@ trait GatewayApiIntegrationSpec extends GatewayIntegrationTestKit { rows.size should be >= 1 rows.exists(_("name") == "show_users") shouldBe true + // Story BIDC-10a Part D (AC 10) - the ONLY live-cluster assertion of the client vocabulary in + // this repo, and it runs on all five clients (ES 6.8 rest + jest, 7.17, 8.18, 9.0). The + // Docker-free specs pin the projection; this pins that a REAL cluster's `_meta` round-trips + // through it. `TABLE`, never the engine's internal `REGULAR`. + rows.find(_("name") == "show_users") match { + case Some(row) => row("type") shouldBe "TABLE" + case None => fail("show_users not found in SHOW TABLES LIKE 'show_%'") + } + rows = assertQueryRows(System.nanoTime(), client.run("SHOW TABLES LIKE '.%'").futureValue) rows.size shouldBe 0 } From d65e06784d9e9c4fbd7cb2568495c19be045f940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Wed, 9 Sep 2026 17:30:57 +0200 Subject: [PATCH 3/3] fix(BIDC-10a Part D): a materialized view is spelled MATERIALIZED VIEW, with a space Lead ruling AD-A-6-SUPERSEDED, PM concurring. `TableType.MaterializedView.sqlName` `MATERIALIZED_VIEW` -> `MATERIALIZED VIEW`. The stored `name` is untouched, as always. The reason is self-consistency with our own SQL, NOT another engine's convention. Every statement that names this object is spaced - `CREATE MATERIALIZED VIEW`, `SHOW MATERIALIZED VIEW`, `SHOW MATERIALIZED VIEW STATUS`, `SHOW CREATE MATERIALIZED VIEW`, `DESCRIBE MATERIALIZED VIEW`. An underscore in the `SHOW TABLES` `type` column would be the only place the product spells its own object differently from the statement that creates it. (The Postgres argument is deliberately NOT used: with the transports reporting `VIEW`, no external tool reads this string, and the reviewer had already caught the previous scaladoc citing Postgres's spaced form while shipping an underscore.) The scaladoc is corrected on a second point it was getting wrong: `sqlName` is the ENGINE surface, not the JDBC/Flight `TABLE_TYPE`. Those drivers map it onto the two values their `getTableTypes` advertises, and for a materialized view that is `VIEW` - permanently, not as a shim, because `getTables` filters by exact string match, so a third value with the advertised list unchanged would make every materialized view vanish from an object browser. That is jdbc#34's dead-end, and it is written at the case object so a future cleanup cannot mistake the mapping for temporary. One assertion had to change to accept the ruling, and it is the interesting part: the well-formedness pin read `fullyMatch regex "[A-Z][A-Z_]*"`, which a space fails. It is now `"[A-Z]+( [A-Z]+)*"` - which also REJECTS an underscore, so the same assertion that admits the new spelling is what stops it silently regressing to the old one. A literal pin sits beside it (`shouldBe "MATERIALIZED VIEW"`, `should not include "_"`), because a property assertion is structurally blind to a spelling choice. `Feature.fromString("MATERIALIZED_VIEWS")` in `FeatureFromStringSpec` was checked and left alone: a licence feature identifier, unrelated to the table-type vocabulary, and the only false positive in the repo. No documentation example shows a materialized view in a `SHOW TABLES` `type` column, so no doc change was needed. The anti-drift assertion added in the previous commit - the `SHOW TABLE` header agreeing with the `SHOW TABLES` column - is kept and now agrees on the spaced value. Verified: sql 1044/1044 (+1), core 956/956, `+ sql/compile` `+ core/compile`, `++ 2.12.20 {sql,core}/Test/compile`, lint; `JavaClientGatewayApiSpec` 74/74 on real ES 8.18. Falsified: restoring the underscore turns exactly 3 of 9 sql tests red - the expectation, the new literal pin, and the well-formedness assertion. Release note, now SMALLER: a BI tool observes NO change from Part D. A plain index already reached a browser as `TABLE` through the driver's own mapping and a materialized view already reached it as `VIEW`, so the BI-browser warning is withdrawn, and so is the claim that this reverses story 20.3's PD-4 - PD-4's frozen advertised list is respected. What changes is the engine surface: `SHOW TABLES` `type` goes `REGULAR` -> `TABLE` and `MATERIALIZED_VIEW` -> `MATERIALIZED VIEW`, and the `SHOW TABLE` header goes `[Regular]` -> `[TABLE]`. --- .../client/ShowTablesTableTypeSpec.scala | 4 +- .../elastic/sql/schema/package.scala | 42 +++++++++++++------ .../sql/schema/TableTypeVocabularySpec.scala | 29 +++++++++++-- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala index 2467ab45a..b8732fded 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/ShowTablesTableTypeSpec.scala @@ -146,7 +146,9 @@ class ShowTablesTableTypeSpec ) ).map(r => r("name").toString -> r("type").toString).toMap - rows("orders_mv") shouldBe "MATERIALIZED_VIEW" + // AD-A-6-SUPERSEDED - SPACED, matching `CREATE MATERIALIZED VIEW`. The JDBC/Flight + // `TABLE_TYPE` contracts collapse this onto `VIEW`; that mapping lives in the drivers. + rows("orders_mv") shouldBe "MATERIALIZED VIEW" rows("orders_v") shouldBe "VIEW" } } diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala index 751e63444..bfc45b6d5 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala @@ -1663,18 +1663,20 @@ package object schema { * - `name` is what is STORED. It is written into `_meta.type` and read back by * `TableType.apply`, so an existing index whose mapping says `"regular"` depends on it byte * for byte. - * - `sqlName` is what is SHOWN to a CLIENT — the `type` column of `SHOW TABLES` and the `SHOW - * TABLE ` header, and therefore the value a JDBC `getTables`, a Flight SQL `GET_TABLES`, - * an ODBC object browser and the REPL receive, since all of them go through the gateway. It - * must be a table type those clients recognise. + * - `sqlName` is what is SHOWN on the ENGINE surface — the `type` column of `SHOW TABLES` and + * the `SHOW TABLE ` header, i.e. what the REPL and anything reading `SHOW TABLES` through + * the gateway receives. It must be a table type a SQL user recognises. * - * The projection used to be `name.toUpperCase`, which published the internal `REGULAR` to every - * client. Measured: the sidecar's ADBC leg failed `Expected 'TABLE' in ['REGULAR']`. Reasoned - * from that, not measured: a client that filters on the standard vocabulary sees no tables at - * all. Story BIDC-10a Part D / AD-A-6. + * ⚠️ `sqlName` is not, by itself, the JDBC/Flight `TABLE_TYPE`. Those drivers map it onto the + * two values their `getTableTypes` advertises (`TABLE` / `VIEW`) — see `MaterializedView` below, + * where that mapping is permanent and load-bearing rather than a temporary shim. + * + * The projection used to be `name.toUpperCase`, which published the internal `REGULAR`. + * Measured: the sidecar's ADBC leg failed `Expected 'TABLE' in ['REGULAR']`. Story BIDC-10a Part + * D. * * `sqlName` is ABSTRACT on purpose: a seventh table type cannot compile until someone decides - * what clients should call it. That decision must not default silently. + * what to call it. That decision must not default silently. * * ⚠️ EXACTLY ONE place outside storage reads `name`, and it is deliberate: `Table.merge` throws * `Cannot alter table of type ` (below, in this file) when an ALTER targets a @@ -1715,13 +1717,27 @@ package object schema { override def sqlName: String = "VIEW" } - /** A materialized view keeps its OWN type rather than collapsing into `VIEW`: it has storage, a - * refresh schedule and (optionally) a watcher, JDBC permits arbitrary type strings, and - * collapsing it would lose information at the source for every client at once. + /** A materialized view keeps its OWN name on the ENGINE surface (`SHOW TABLES`, the REPL) + * rather than collapsing into `VIEW`: it has storage, a refresh schedule and (optionally) a + * watcher, and collapsing it would lose information for the human reading the listing. + * + * 🔴 The spelling is SPACED, and the reason is self-consistency with our own SQL, not any + * other engine's convention. Every statement that names this object is spaced - `CREATE + * MATERIALIZED VIEW`, `SHOW MATERIALIZED VIEW`, `SHOW MATERIALIZED VIEW STATUS`, `SHOW CREATE + * MATERIALIZED VIEW`, `DESCRIBE MATERIALIZED VIEW` (`sql/.../query/package.scala`). An + * underscore here would be the only place the product spells its own object differently from + * the statement that creates it. + * + * ⚠️ This is NOT what a JDBC or Flight SQL client sees. Those `TABLE_TYPE` contracts report + * `VIEW` for a materialized view, because `getTableTypes` advertises exactly `TABLE` and + * `VIEW` and `getTables` filters by EXACT string match - so a third value, with the advertised + * list unchanged, makes every materialized view vanish from an object browser that asks for + * the advertised types. The drivers own that collapse; it is PERMANENT and load-bearing, not a + * shim to be cleaned up. Story BIDC-10a, AD-A-6-SUPERSEDED. */ case object MaterializedView extends TableType { override def name: String = "materialized_view" - override def sqlName: String = "MATERIALIZED_VIEW" + override def sqlName: String = "MATERIALIZED VIEW" } /** Parses the STORED name (`_meta.type`), never the display `sqlName`. Widening it to accept diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala index 75ca4f978..db9207fa9 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/TableTypeVocabularySpec.scala @@ -27,7 +27,7 @@ class TableTypeVocabularySpec extends AnyFlatSpec with Matchers { private val expected: Map[TableType, (String, String)] = Map( TableType.Regular -> (("regular", "TABLE")), TableType.View -> (("view", "VIEW")), - TableType.MaterializedView -> (("materialized_view", "MATERIALIZED_VIEW")), + TableType.MaterializedView -> (("materialized_view", "MATERIALIZED VIEW")), TableType.External -> (("external", "EXTERNAL")), TableType.Changelog -> (("changelog", "CHANGELOG")), TableType.Enrichment -> (("enrichment", "ENRICHMENT")) @@ -81,14 +81,37 @@ class TableTypeVocabularySpec extends AnyFlatSpec with Matchers { declaredTableTypes.map(_.sqlName) should not contain "REGULAR" } - it should "keep MATERIALIZED_VIEW distinct from VIEW" in { + /** AD-A-6-SUPERSEDED: the ENGINE surface keeps the two apart. The JDBC and Flight `TABLE_TYPE` + * contracts deliberately collapse a materialized view onto `VIEW` - that mapping lives in the + * drivers and is pinned there, not here. + */ + it should "keep MATERIALIZED VIEW distinct from VIEW" in { TableType.MaterializedView.sqlName should not be TableType.View.sqlName } + /** 🔴 The spelling is SPACED because every statement naming this object is spaced (`CREATE + * MATERIALIZED VIEW`, `SHOW MATERIALIZED VIEW STATUS`, ...). An underscore would make the `SHOW + * TABLES` `type` column the only place the product spells its own object differently from the + * statement that creates it. Pinned literally: a property assertion cannot see this. + */ + it should "spell a materialized view the way our own SQL spells it" in { + TableType.MaterializedView.sqlName shouldBe "MATERIALIZED VIEW" + TableType.MaterializedView.sqlName should not include "_" + } + + /** Well-formed = upper-case words separated by single spaces. A SPACE is legal (AD-A-6- + * SUPERSEDED, `MATERIALIZED VIEW`); an underscore is not, so this assertion is also what stops + * the spaced spelling silently regressing. Distinctness matters because two types sharing a + * client name would make `SHOW TABLES` ambiguous. + */ it should "give every type a distinct, well-formed client name" in { val names = declaredTableTypes.map(_.sqlName) names.distinct.size shouldBe names.size - names.foreach(n => n should fullyMatch regex "[A-Z][A-Z_]*") + names.foreach { n => + withClue(s"client name `$n`: ") { + n should fullyMatch regex "[A-Z]+( [A-Z]+)*" + } + } } /** 🔴 The migration guard. Adding a display name must NOT make an existing index's stored