fix: refuse Tableau's temp-table probe by intent, and name the real reason - #327
Merged
Merged
Conversation
…eason
SoftClient4ES already refused Tableau's connection-capability probe, but it refused
at the wrong layer for a misleading reason, and the correctness of the refusal rested
on one accident of rule ordering plus one line one edit away from creating indices in
a customer's cluster.
- The grammar now RECOGNISES `CREATE [LOCAL | GLOBAL] TEMPORARY TABLE` so the refusal
can name the construct, and raises an `err` at the statement header. Refusing before
the body is read makes the message deterministic and means `ON COMMIT { PRESERVE |
DELETE } ROWS` is refused with the statement it can only legally appear on, never
accepted and dropped. The production is FIRST in `ddlStatement` (nothing else accepts
`TEMPORARY` there, so it cannot commit to a prefix; and from the front its `Error`
short-circuits while its `Failure` yields every tie, so `CREATE TABEL` is not told
about temporary tables) and `private` (it yields `Nothing` — a refusal is not a
statement production).
- `validateIndexName` reports EVERY violated rule in one message, character rule first,
instead of returning on the first hit with the casing test ahead of the character
test. Tableau's MySQL probe names carry uppercase AND `#`, so the engine used to
answer with a complaint about casing for a name whose `#` keeps it illegal however it
is cased. A single-rule message is unchanged; the character rule now names the
characters found and lists `\`, which the old text omitted while the regex it guarded
rejected it. The character tables live in a new companion object, not on the trait,
so they cost no binary incompatibility.
- `Table.indexName` is RETAINED, against the brief: it is not dead code, it has a live
reader in softclient4es-extensions (`MaterializedViewSchemas`, via the `Schema` import
rename of this module's `Table`). Its hazard is closed instead by a scaladoc forbidding
the DDL routing path and by `IndexNameProbeRefusalSpec`, which pins that the SQL-92
probe names are refused on their casing AND NOTHING ELSE — so a `toLowerCase` anywhere
on that path goes red.
- Ships a Tableau `.tdc` declaring `CAP_CREATE_TEMP_TABLES='no'` so Tableau skips the
probe, published as explicitly UNVERIFIED against a live Tableau Desktop, with the
verification that would settle it and a pre-registered kill criterion written beside
it. `CAP_SUPPRESS_TEMP_TABLE_CHECKS` is documented only for a packaged `.taco`
connector, and the docs say so.
- Corrects the three `rejected_pending_policy` attribution notes and two `CorpusReplaySpec`
scaladocs, which said these rows were "still rejected on the absent LOCAL TEMPORARY
grammar" — false once the grammar recognises and deliberately refuses. Verdicts
unchanged. Corrects the known-limitations claim that there is "nothing to honestly
answer yes to": Tableau's `CAP_TEMP_TABLES_NOT_SESSION_SCOPED` exists for sources that
simulate temp tables with regular ones, so the reason is cost and safety, not
impossibility.
Differential no-narrowing probe, 994 statements through a parser built from each tree:
674 accept / 320 reject / 0 threw on BOTH — zero verdict changes. 16 messages change:
13 gain the named refusal, and `CREATE LOCAL TABLE` / `CREATE GLOBAL TABLE` /
`CREATE TEMPORARY VIEW` now report the token they are missing instead of the `OR` of
`CREATE OR REPLACE`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…atus The .tdc section published our own engineering uncertainty -- which capability is documented for which connection type, that the file "has NOT been exercised against a live Tableau Desktop", that one capability is "unknown to us" and "included because it is harmless if ignored, not because it is known to work" -- and then asked the reader to run the experiment that would settle it, with the JDBC capture tool named. A customer reading that learns only that we are unsure; it is an internal fact and an internal task, published as if it were product documentation. The section now publishes no confidence level and no caveat at all. What it says: - What the file changes, and what it does not. Tableau ends on the same fallback path either way -- with the file it is told there are no temporary tables, without it it finds out by having the probe refused -- so the file removes a failed round trip rather than a restriction. - The limitation the file does NOT lift: this release accepts neither subqueries nor derived tables, which is what Tableau's fallback and its Custom SQL wrapper generate, so those interactions still fail -- loudly, with an error naming the statement, never a hang and never a silently wrong answer. - How to stop using it (delete the file), which is the real risk control the removed "kill criterion" paragraph was standing in for. The install table and the "it does not make Tableau faster" trade-off are kept verbatim: that trade-off IS a limitation and was already well put. The same confession is removed from the .tdc's own comment header. known_limitations.md: "whether a plain CREATE TABLE against a probe-shaped name should be honoured is a separate open question" told a customer nothing they can check. It is replaced by the limitation they can: index names must be lowercase and cannot contain \, /, *, ?, ", <, >, |, a space, a comma or #, and the error names every rule the name breaks -- verified against validateIndexName's own tables. Pre-existing and NOT touched, reported instead: the four-tier table's "Unproven -- a connection path exists on paper, but nobody has connected it yet" and "Compatible (not formally tested)" are the same failure mode one level up. They are a lead-owned tier vocabulary spanning four surfaces in two repos, so they need their own change, not a drive-by edit inside a temp-table fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fupelaqu
force-pushed
the
fix/tableau-temp-table-refusal
branch
from
September 13, 2026 16:01
74272e9 to
75e836c
Compare
Tableau's data-source row-existence check, captured verbatim from a live Tableau Desktop over the JDBC driver against a real Elasticsearch 8.18.3, is SELECT SUM(1) AS `cnt_..._ok` FROM `docker-cluster`.`bi_events` `bi_events` HAVING (COUNT(1) > 0) and it failed the data-source preview with `illegal_argument_exception: Required one of fields [field, script], but none were specified`. It carries two defects, one loud and one silent, and fixing only the loud one would have turned a failed query into a wrong answer. COUNT(<non-null literal>) is COUNT(*) in ANSI SQL -- counting a constant counts rows. CountAgg.rowCountingOperand, applied in the parser's count_agg production, rewrites such an operand to `*` before it becomes both the CountAgg's identifier and the outer identifier, so the statement takes exactly the path COUNT(*) already takes: one mechanism, not two. COUNT(NULL) is deliberately NOT rewritten -- ANSI gives it 0, so folding it in would trade a loud failure for a wrong answer. A HAVING with no GROUP BY was SILENTLY DISCARDED: a whole-table aggregate has no buckets, so the bucket_selector the GROUP BY path attaches to each bucket had nothing to attach to. Measured on real Elasticsearch over a 703-document index, `HAVING COUNT(*) > 10000` returned 703. The root metric aggregations now move inside a synthetic single-bucket keyed `filters` aggregation that the SAME having_filter bucket_selector hangs from -- it must be `filters`, not `filter`, because a bucket_selector inside a single-bucket filter fails the search with a class_cast_exception. HAVING therefore means the same thing with and without a GROUP BY by construction rather than by agreement between two implementations. Shapes that cannot be honoured are REFUSED by SingleSearch.validate(), never dropped; a HAVING reaching into a nested relation keeps its own long-standing mechanism untouched. Also withdraws the Tableau capability file this branch previously added: CAP_SUPPRESS_TEMP_TABLE_CHECKS is not among the capabilities Tableau documents for JDBC connections, so the file could not do what its install instructions claimed. The documentation states the limitation instead. GroupByCompletenessSpec's two pins become correctness assertions, keeping the falsifiable true/false pair; 26/26 on real Elasticsearch 6.8 (rest and jest), 7.17, 8.18 and 9.0. The four corpus attribution rows stay scored `residual` on purpose -- gate G7 reserves `fixed` for Epic 21 -- so the published headline is understated by four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The issue-lifecycle rule says a `local:<slug>` attribution owner becomes `issue:<N>` once the story that fixes the defect files it remotely. That happened: #328 covers both halves — `COUNT(<literal>)` emitting a fieldless aggregation, and a whole-table `HAVING` being silently discarded — and this PR closes it. So `tableau.mysql.w1.023`, `w7.048`, `w7.051` and `tableau.sql92.wx.015` now carry `issue:328`. A reference that resolves for every reader replaces one that resolved only on the author's machine. Gate G3 format-checks both spellings, so nothing else moves. The rows stay `scored = residual`: G7 reserves `fixed` for Epic 21 and this is not Epic 21's work, which leaves the published 56 understated by four — stated in the PR body. CorpusReplaySpec 12/12. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fupelaqu
marked this pull request as ready for review
September 13, 2026 21:01
fupelaqu
added a commit
that referenced
this pull request
Sep 17, 2026
… headline is 68/99 The four `issue:328` rows carried a note handing the decision to this story. I declined and routed it up; the lead ruled to take it. The reasoning, recorded because the rule it bends is load-bearing: the published sentence claims what the engine ANSWERS, not which epic earned it. Correctness here is asserted by a merged, five-client suite against real Elasticsearch — GroupByCompletenessSpec "corpus shape: HAVING with no GROUP BY" (PR #327), one row for a true predicate and ZERO rows for a false one — which is exactly AD-10's bar. Understating Tableau's own per-data-source probe by four rows was safe in direction but inaccurate in a headline that is about BI SQL. SCORES 68/99, 68/75. `91 PARSE` is unchanged and the gap is re-derived: 91 - 68 = 23 = 21 parsing capability probes + the 2 UNMEASURED E5 rows. 🔴 The admission is a COMPILED SINGLETON (`Issue328FixedIds`), never a loosened predicate and never `owner.startsWith("issue:")`. G7 exists so that `fixed` cannot quietly come to mean "it parsed"; a widened predicate would hand that meaning to every future `issue:` owner in silence. An exception a reader can ENUMERATE keeps the rule a rule, and it is checked BOTH ways so a dead exception cannot survive either. 🔴 The series head row was EDITED, not appended, and the reasoning now sits in the README beside the append-only sentence. Append-only exists so a MERGE that flips a verdict cannot be hidden by moving the expectation; all three of its preconditions failed here — nothing had merged, the number had never left the branch, and what changed was the SCORING POLICY rather than the tree. `series.csv` has one row per measured TREE and no column that could tell two policies on one commit apart, so a second row for 7187c7d would read as a contradiction and manufacture a history nobody measured. G8 going red until the head moved is the mechanism working. A change to the TREE never qualifies. The four notes were REWRITTEN in place. They previously argued the rows were "DELIBERATELY NOT re-scored" and that the headline "UNDERSTATES the corpus by these four rows"; left standing, the artefact would contradict itself and the next reader would quote whichever half they hit first. Falsified, six mutations, all RED with the control green either side: the exception in BOTH directions; both bumped count pins reverted; the series head reverted; and — the one that proves the widening did not leak — a different, non-excepted `local:` owner scored `fixed` is still refused by G7. sql/test 1330/1330; the four census suites 57/57; CI lint line green; ++ 2.12.20 sql/Test/compile green. Neither docs branch quotes a corpus number, so the two docs commits do not move. Closed Issue #358
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #326
Closes #328
Tableau probes for temporary tables on every connection, and SoftClient4ES already refused — at the
wrong layer, for a misleading reason, with the correctness of the refusal resting on one accident of
rule ordering and one line of code one edit away from creating indices in a customer's cluster. This
makes the refusal deliberate, says so in the error, and closes the hazard.
Scope added after the first review, on live evidence. A real Tableau Desktop over the JDBC driver
against a real ES 8.18.3 was captured with p6spy. Its data-source preview failed, four times in a row
per session, on this statement:
That is Tableau's row-existence check for a data source — the most basic interaction there is — and it
carries two defects, one loud and one silent. Both are fixed here (§6 and §7). The
.tdcthat thefirst cut of this PR shipped is withdrawn (§4).
§1's grammar change narrows nothing: 994 statements, 0 verdict changes (differential probe below,
measured on that change). §6 and §7 DO change behaviour, deliberately —
COUNT(<literal>)startsworking, a whole-table
HAVINGstarts being applied, and someHAVINGstatements that parsed are nowrefused because parsing them could only ever have meant discarding them. Release notes 5-7 say exactly
how.
What changed
1. The grammar recognises
CREATE [LOCAL | GLOBAL] TEMPORARY TABLEin order to refuse itParser.temporaryTablematchesCREATE [LOCAL | GLOBAL] TEMPORARY TABLEand raises anerrnamingthe construct. The exact message, for each spelling:
(with
CREATE LOCAL TEMPORARY TABLE …/CREATE GLOBAL TEMPORARY TABLE …for the scoped spellings —the scope word is echoed as the caller spelled it). It replaces
string matching regex '(?i)OR\b' expected but 'L' found, the failure of the unrelatedCREATE OR REPLACE TABLEalternative.Three design points, each load-bearing:
errat parse time, never accept-and-ignore. Accepting the statement and refusing it laterwould make
Parser.applyreturn aRight— i.e. report the capability as PRESENT to everyconsumer that only looks at the parse, the corpus replay among them.
DropTable.cascadeis thestanding proof that a parsed-and-ignored clause survives for years; for a clause that governs data
lifetime that is a silent wrong answer.
AS <select>body, so themessage is deterministic: an unparseable body, or an
errfrom an embedded SELECT, cannotsubstitute its own reason.
ON COMMIT … ROWSis therefore refused as part of the only statement itcan legally appear on — covered by tests in both spellings — rather than accepted and dropped.
ddlStatement, andprivate. First becauseTEMPORARYis mandatory and nosibling accepts it in that position, so it cannot commit to a prefix of a statement another
alternative handles; and because from the front its
Errorshort-circuits the rest unconditionallywhile its
Failureyields every tie —Failure.appendkeeps the later result at an equal offset— so a plain
CREATE TABELtypo is still diagnosed without a word about temporary tables.Measured both ways before choosing.
privatebecause it yieldsNothing: it is a refusal, not astatement production, so it has no AST node, no help document, and no business in
HelpCorpusSpec's enumeration of the statements a user can type.TEMPORARY/LOCAL/GLOBALare registered inSQLKeywords.statementWords(the anti-drift scanrequires it) but deliberately not in
reservedKeywords:CREATE TABLE temporary (…)andSELECT temporary FROM tkeep working, pinned by test.2.
validateIndexNamereports every violated rule, character rule firstEach rule used to return early, with the lowercase test ahead of the character test — so the MySQL
probe names, which carry uppercase letters and a
#, were refused as "Index name must belowercase" and the
#was never named.Decision: report all violated rules in one
;-joined message, rather than reorder. A reorderonly swaps which single reason wins; the next name carrying a space and an uppercase letter
reproduces the defect with different characters. Of the rules that can co-occur, the character rule
comes first because it is the specific one, but none can hide another.
A name violating exactly one rule keeps its message verbatim, so every existing assertion holds
unchanged. The character rule now also names the characters actually found, and lists
\— whichthe old message omitted while the regex it guarded rejected it:
The forbidden-character tables move to a new
object ElasticClientHelpersrather than onto the trait— a
valin a Scala 2 trait is emitted as a field plus an initializer that every already-compiledimplementor must provide, which is a binary incompatibility a table of characters does not need to
cost. (The same file's
unwrapThrowablerecords that trap.)3. The
Table.indexNamehazard is documented and pinned —indexNameis NOT dead codeTable.indexNamewas to be deleted as deadcode. It is not dead:
softclient4es-extensionsreads it atcommunity/.../view/MaterializedViewSchemas.scala:359—sourceSchema.indexName, wheresourceSchema: SchemaandSchemais an import rename of this module'sschema.Table. Deleting themember breaks the extensions build. (The four other
.indexNamehits in that repo are a differentmember on its own
graph.TableNode, which is what made the member look unread.)So the hazard is closed the other way, which the brief offers as the alternative: a scaladoc naming
the single legitimate caller and forbidding the DDL routing path, plus
IndexNameProbeRefusalSpec,which asserts the probe names stay refused and that the SQL-92 family is refused on its casing
and nothing else — the falsifier, since "refused" would otherwise stay green for a reason unrelated
to the hazard. Lowercase those names anywhere on the
CREATE TABLE/DROP TABLEpath and the specgoes red.
4. No Tableau
.tdcis shipped — the limitation is documented insteadThe first cut of this PR added
documentation/client/tableau/softclient4es.tdc, declaringCAP_SUPPRESS_TEMP_TABLE_CHECKS='yes'so Tableau would skip the probe. It is deleted, and thedocumentation now states the limitation rather than offering a file.
Why:
CAP_SUPPRESS_TEMP_TABLE_CHECKSis not a capability Tableau documents for JDBC connections.Tableau's JDBC Capability Customizations
Reference lists
CAP_CREATE_TEMP_TABLESand around sixty others; that one is absent. It belongs to the Connector SDKcapability set, which a packaged
.tacoconnector declares — not a.tdcon an Other Databases(JDBC) connection. So the file could not do the one thing its install instructions promised, and
shipping it would have published an artefact with a benefit nobody can observe. This is a published,
vendor-owned fact and it is the whole argument.
What the pages say now, in
documentation/client/bi_tools.mdand the MDX twin: Tableau issues theCREATE TABLE/DROP TABLEprobe on every connection (measured across five live Tableau Desktopsessions); the refusal is a supported path in Tableau's own connector documentation; Tableau's
fallback uses subqueries, which this release does not accept, so some interactions fail with a clear
error naming the statement — never a hang, never a silently wrong answer; and a
.tdccannotsuppress the probe, for the reason above. No install table, no file, and nothing about what we tested.
5. Documentation and the Epic 21 scoreboard
documentation/sql/known_limitations.md— the temp-table section now states the refusal asdeliberate and gives the remedy. Its claim that "an Elasticsearch index is global, permanent and
not session-scoped, so there is nothing for the engine to honestly answer yes to" was too strong:
Tableau's own
CAP_TEMP_TABLES_NOT_SESSION_SCOPEDexists for sources that "use regular tables tosimulate temp tables", so session scope is not something Tableau requires. The section now says
what is actually true — the reason is cost and safety, not impossibility — and says what the cost is
(a cluster-state update serialised through the elected master per temporary table, and an
accumulation that ends at the per-node shard limit refusing all index creation).
documentation/client/bi_tools.md— the temp-table section, rewritten as the limitation (§4).documentation/sql/known_limitations.mdalso drops "whether a plainCREATE TABLEagainst aprobe-shaped name should be honoured is a separate open question" — an open question is not a
limitation and tells a customer nothing they can check. It is replaced by the one they can: index
names must be lowercase and cannot contain
\,/,*,?,",<,>,|, a space, a commaor
#, and the error names every rule the name breaks. The product decision behind the old sentenceis unchanged and still unowned.
sql/src/test/resources/corpus/epic-21-attribution.csv— the threerejected_pending_policynotessaid these rows were "still rejected on the absent LOCAL TEMPORARY grammar, which Epic 21 does not
add". That is now false: the grammar recognises and deliberately refuses.
expectedstaysrejectedandscoredstaysrejected_pending_policy— only the reason changed. Two scaladoccomments in
CorpusReplaySpeccarried the same dead sentence and were corrected with it.6.
COUNT(<non-null literal>)isCOUNT(*)— the loud half of Tableau's failureANSI SQL (ISO/IEC 9075-2,
<general set function>) definesCOUNT(<expr>)as the number of rows forwhich
<expr>is not null, so a constant, non-null operand counts every row.COUNT(1)isCOUNT(*). This engine instead emitted avalue_countaggregation with an EMPTY field and noscript, which every Elasticsearch major rejects outright.
The operand is normalised where it is built —
CountAgg.rowCountingOperand, applied in the parser'scount_aggproduction, rewrites a bare non-null literal operand to*before it becomes both theCountAgg's identifier and the outer identifier. From there the statement is indistinguishable from aCOUNT(*)the user typed:metricNameiscount_all, the bridge's existingsourceField == "*"armmaps it to
_index, and the render isCOUNT(*), which re-parses to the same AST. One mechanism, nottwo —
Parser("SELECT COUNT(1) AS c FROM t") == Parser("SELECT COUNT(*) AS c FROM t")is asserted.🔴
COUNT(NULL)is deliberately NOT rewritten. It is not a row count — ANSI gives it 0 — sofolding it into
COUNT(*)would turn a loud failure into a wrong answer. It keeps today's behaviour(an aggregation over no field, which Elasticsearch refuses), which is the safe direction;
Nullisthe one carve-out from
SingleSearch.isRowInvariantLiteral's allow-list, and it is carved out hereand only here.
COUNT(DISTINCT <literal>)needs no arm: it does not parse at all, before or after.7. A whole-table
HAVINGis honoured — or refused — but never discardedThis is the silent half, and it is the dangerous one: fixing §6 alone would have converted Tableau's
loud failure into a wrong answer. With a field-bearing aggregate the statement already SUCCEEDED and
the
HAVINGevaporated — a whole-table aggregation has no buckets, so thebucket_selectortheGROUP BYpath attaches to each bucket had nothing to attach to and the criteria went unused.Measured on real Elasticsearch over a 703-document index:
… HAVING COUNT(*) > 10000returned 703,… HAVING SUM(amount) > 99999returned the unfiltered sum. HTTP 200, wrong answer — the#205/#209/#224/#253 family.
Decision: option (b2), an Elasticsearch-side single synthetic bucket — NOT (b1), engine-side
evaluation. (b1) was the recommended default and (b3), a loud refusal, would have left Tableau's
preview broken. Two measured reasons for taking (b2):
NOT carry the values the predicate reads.
ElasticConversion.extractMetricsdrops every metricwhose aggregation is
auxiliary— issue Non-selected aggregations from HAVING/WHERE/ORDER BY leak into result columns #55's rule, which exists so that an aggregate referencedonly in
HAVING/WHERE/ORDER BYdoes not leak into the result columns. Straight from the liverun below, the row for Tableau's own statement is
ListMap(cnt_ok -> 5000.0):count_all, themetric
COUNT(1) > 0compares, is not in it. Evaluating there would therefore have had toun-strip those metrics and thread a second statement-derived parameter through the same shared,
statement-less client seams that
rowInvariantsneeded (an arity change onElasticConversion,SearchApiandElasticClientDelegator) — strictly more plumbing than (b2), for a worse result.HAVINGsemantics, and this repository has been bitten bythat shape repeatedly — most recently in BIDC-8, where
MetricSelectorScriptkept its own copy ofthe negation table beside
Criteria.negatedand the two disagreed. Null handling in a bucketpipeline is a stated contract here (a bucket whose compared metric is missing never passes a
HAVINGcomparison); two implementations of it will drift, and the drift is silent. Under (b2)HAVING Xmeans exactly what it means under aGROUP BY— same script, samebuckets_path, samenull guards — by construction rather than by agreement.
How it works.
SingleSearch.wholeTableHaving(groupBy.isEmpty && having.criteria.isDefined && !returnsRows) is the single discriminator; the bridge'sElasticAggregation.wholeTableHavingAggregationwraps the root metric aggregations in a keyed
filtersaggregation holding onematch_allbucket andhangs the SAME
having_filterbucket_selectoroff it, built by the samemetricSelectorForBucket/extractMetricsPathForBucketpair theGROUP BYpath uses. A falsepredicate removes the only bucket, Elasticsearch answers
"buckets": {}, and the statement returns norow.
🔴 It must be
filters, notfilter. Measured on ES 8.18.3: abucket_selectorinside asingle-bucket
filteraggregation fails the search withclass_cast_exception: InternalFilter cannot be cast to InternalMultiBucketAggregation. A keyedfiltersaccepts it and drops the bucket exactly as required.The synthetic bucket is transparent:
ElasticConversioncontributes no key column for it, becauserowNormalizerAPPENDS keys nobody requested rather than dropping them, so without that it wouldsurface as an extra result column. One reserved name (
SingleSearch.WholeTableHavingAgg), two readers,so they cannot drift.
And what cannot be honoured is REFUSED, not dropped.
SingleSearch.validate()now rejects, for aHAVINGwith noGROUP BYand no nested relation in it:SELECT COUNT(*) AS c FROM t HAVING category = 'x'Non-aggregated fields category cannot be used in HAVING when GROUP BY is absent; use WHERE, or add a GROUP BYSELECT * FROM t HAVING category = 'x'SELECT category FROM t HAVING COUNT(*) > 1Non-aggregated fields category cannot be selected when HAVING is present without a GROUP BYHAVING without GROUP BY is only supported for an aggregate query over the whole table; this statement returns document rows(the catch-all, so nothing can fall through silently)A
HAVINGthat reaches into a NESTED relation is explicitly out of scope and untouched: it has its ownlong-standing mechanism (
requestToNestedFilterAggregationscopes it to afilteraggregation on theinner-hits path), it is exercised by the bridge fixtures, and it is not discarded. Narrowing the new
rules to the flat case is what the
SQLQuerySpec"nested count with filter" fixture measured for us.8. The Epic 21 attribution rows say what happened — and are deliberately NOT re-scored
The four rows owned
issue:328(tableau.mysql.w1.023,w7.048,w7.051,tableau.sql92.wx.015) recorded both defects above. Their notes now say both are fixed, how, and thatGroupByCompletenessSpecasserts the correct answers on all five clients.They stay
scored = residual. Gate G7 enforcesscored = fixed ⇒ owner = epic21, and this is notEpic 21's work — extending that vocabulary belongs to story 22.7.
direction, and re-scoring on the side would break the gate that keeps the scoreboard honest.
CorpusReplaySpecstays 12/12.Verification
sql/testOnly *ParserSpecsql/testOnly *CorpusReplaySpecsql/testcore/testsoftclient4es-sql-bridge/test·es6bridge/test+ compile(2.12 + 2.13, all modules)headerCheck·scalafmtCheckAll·Test/scalafmtCheckTemporaryTableRefusalSpecIndexNameProbeRefusalSpecGroupByCompletenessSpecon real ESTests are run on Scala 2.13 only, as CI does;
+ compilestill covers both legs because therelease publishes 2.12 artifacts from main sources.
Live acceptance, against a real Elasticsearch 8.18.3 through the es8 Java client. Tableau's own
failing statement, and its false-predicate twin:
and the body it sends for the first of those:
{"query":{"match_all":{}},"size":0,"_source":false, "aggs":{"__whole_table_having__":{"filters":{"filters":{"_all":{"match_all":{}}}}, "aggs":{"cnt_ok":{"sum":{"script":{"lang":"painless","source":"1"}}}, "count_all":{"value_count":{"field":"_index"}}, "having_filter":{"bucket_selector":{"buckets_path":{"count_all":"count_all"}, "script":{"source":"(params.count_all == null ? false : (params.count_all > 0))"}}}}}}}Differential no-narrowing probe. 🔴 These numbers are from the first review round (§1's
grammar change) and were NOT re-run after §6 / §7 — say so rather than imply a coverage that was
not measured. They still stand for what they were built to prove, and §7 deliberately narrows the
language anyway (release note 7): a
HAVINGwith noGROUP BYover a non-aggregated column used toparse and is now refused, because parsing it could only ever have meant discarding it. What covers
§6 and §7 instead is the corpus replay over all 99 captured BI statements (12/12),
sql1093,core994 and the five real-Elasticsearch legs above.An alternation's order is a contract about what it DECLINES, so a new first alternative has to be
shown not to narrow anything — branch tests cannot do that, because the inputs a narrowing breaks are by
construction the ones nobody wrote a test for. A parser was built from
origin/mainand from thisbranch and fed the same 994 statements (every published help-corpus example and syntax line, the
dialect census's per-form examples, all 99 captured BI statements, and hand-written DDL/DML shapes):
Release notes owed (0.23.0)
validateIndexNamenow reports every violated rule in one;-joined message instead of the first, and the forbidden-character rule names the characters itfound plus the full forbidden list (including
\, previously omitted). A single-rule message isunchanged, so an exact match on one of those still holds; anything matching the old
multiple-violation behaviour — which reported only the first rule — does not. Reaches JDBC
verbatim through
SQLException.CREATE [LOCAL | GLOBAL] TEMPORARY TABLEnow reports a named refusal instead of a combinatorfailure.
CREATE LOCAL TABLE/CREATE GLOBAL TABLE/CREATE TEMPORARY VIEWreport the tokenthey are missing. No statement changes verdict. Never pin a grammar-internal message.
Table.indexNameis retained, not removed — the brief called for deleting it; it has a livereader in
softclient4es-extensions. No binary change. Its scaladoc now forbids the DDL routingpath, and a spec pins the outcome.
object ElasticClientHelperswithforbiddenIndexNameCharsandforbiddenIndexPatternChars. Nothing removed, nothing moved off the trait that was ever on it —no rebuild obligation from this change.
COUNT(<non-null literal>)now works, and renders asCOUNT(*).COUNT(1),COUNT('x'),COUNT(TRUE)previously failed against every Elasticsearch major; they now count rows. Thestatement's own render changes (
SHOW CREATE, materialized-view persistence, anything reading.sql) —COUNT(1)comes back asCOUNT(*).COUNT(NULL)is unchanged.HAVINGwith noGROUP BYis now applied. It used to be silently ignored, so a querythat returned the unfiltered aggregate may now return NO row — which is the correct answer, and is
a change in what callers see. The emitted query gains a
__whole_table_having__filtersaggregation wrapping the metric aggregations; downstream repos pinning generated JSON for this
shape need updating.
HAVINGstatements that parsed are now rejected, because they could only ever havebeen discarded: a
HAVINGnaming a non-aggregated column with noGROUP BY, a SELECT listcarrying one, and any remaining row-shaped statement. Nested-relation
HAVINGis unaffected..tdcis shipped. The first cut of this PR added one; it is withdrawn becauseCAP_SUPPRESS_TEMP_TABLE_CHECKSis not among the capabilities Tableau documents for JDBCconnections. The documentation states the limitation instead.
Reported, not changed: the four-tier table says the same thing one level up
documentation/client/bi_tools.mddefines Unproven as "a connection path exists on paper, butnobody has connected it yet" and gives Tableau "Compatible (not formally tested)"; the Power BI row
and
known_limitations.md's "One tool has a path nobody has walked yet" repeat it, and so do theMDX twins. That is our verification status again, phrased as a fact about us rather than about the
product. The limitation-shaped form exists and is already half-written in the same rows: Power BI has
no JDBC connector; the only candidate path is a generic Arrow Flight SQL ODBC driver against the
sidecar, which is not a supported route in this release.
Recommendation: fix it, but not here. Three reasons. The tier vocabulary is lead-owned — nobody
promotes or rewrites a tier on the side. It spans four surfaces in two repos plus the marketing pages,
so the correct change is a sweep. And it predates this PR by two stories; folding it in would widen a
temp-table fix into a claims review. Left untouched on purpose.
For the lead, not acted on
COUNT(<literal>)and the whole-tableHAVINGare two distinct defects that happen to meet in oneTableau statement. Whether they deserve their own remote issue number or ride #326 is a call for the
lead; nothing has been filed.
Documentation twin (the MDX side of the same change, closes nothing):
SOFTNETWORK-APP/softclient4es-web#59
🤖 Generated with Claude Code