GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||
GeezerVison is showing github.com — test: fill coverage gaps in the GQL query engine (stacked on #849) by rjurney · Pull Request #901 · graphframes/graphframes Conversation
|
| Suite | Tests | What it covers |
|---|---|---|
FunctionRegistrySuite |
28 | The whitelist dispatch table itself — every accepted name at its documented arity, alias equivalence, case-insensitive dispatch, the three arity guards (arity / arityBetween / arityAtLeast), the litStr / litInt literal slots, and the unknown-name error. Previously only 4 of ~90 names were exercised. |
ExpressionLoweringSuite |
22 | PrefixEnv (named vs positional prefixes, prefixFor, the raw environment) and every ExpressionLowering rule, asserted against the hand-written Spark Column it is documented to produce, including literal narrowing and the unbound-variable error. |
PropertyGroupGetDataSuite |
21 | The getData(filter, requestedProperties) contract added for the engine: column order, silent dropping of unknown names, unmasked property pass-through, id masking on/off, filter-before-masking, and the undirected-edge orientation union. |
AstBuilderSuite — NULL / boolean / empty-string / escaped-quote literals, the absence of unary minus, malformed pattern and RETURN shapes, variable-length quantifier parsing, and the reserved-word boundary.ResolverSuite — the WHERE-classification buckets that had no coverage: edge-variable scan filters, edge+node and edge+edge conjuncts, non-adjacent two-variable conjuncts, literal-only conjuncts, a single WHERE split across all four destinations at once, multi-position variables, and the label-validation error messages.QueryExecutorSuite — ScanKey equality semantics (the contract the scan memo depends on), the fixed output schema as a contract, RETURN *, and fan-out RETURN when a property is missing from one candidate group.PropertyGraphFrameQuerySuite — QueryOptions defaults and factories, the maxEnumeratedPaths guard and its boundary (previously untested), explain's bypass of it, and the convenience overloads.SchemaGraphSnapshotSuite — outgoing / incoming indexing, parallel edge groups, and the fact that DOT / toString render directed and undirected groups identically.Each is pinned by a test named KNOWN LIMITATION: … that asserts today's behaviour and carries a
comment naming the cause and a suggested fix, so the suite stays green and the issue is visible
rather than latent. None are fixed here — those calls belong to the author of #849.
nullif is unusable anywhere in a query. functions.nullif reads left.dataType in itsFunctionRegistry is an UnresolvedAttribute, so itUnresolvedException in WHERE, in RETURN and in a join predicate alike. Either dropwhen(a === b, lit(null)).otherwise(a).nvl / ifnull / coalesce are unaffected.AMBIGUOUS_REFERENCE. PrefixEnv.nodePrefix(a)-[]->(b)-[]->(c)-[]->(a) renames nodes 0a_id. Resolution already supports repeated bindings (classifyWhere'sattachNodeScanFilters), so this is an executor-side gap. CyclicWHERE a = … and WHERE a.id = … require the primary key column to be namedid. Scan-local filters are lowered with PrefixEnv.raw and applied by getDataid does not yet exist. The same predicate in aRETURN works, which isolates it to the scan-local path.outputSchema declares everypath columns from lit(...),df.schema == other.schema for callers of a documented "fixed output schema".RETURN * shares a branch with Projection.Default, so it yields the fixed six-columnexplain renders it as * (all matched variables). Pinned so that implementing a* projection is a deliberate change.Two smaller notes, not pinned by tests: keywords (as, is, in, not, …) cannot be used as
variable, property or label names and the grammar offers no quoted-identifier escape; and
QueryOptions.withDefualts is misspelled (the tests call it as-is, so a rename shows up here).
core/src/main/scala/org/graphframes/propertygraph/internal/GraphStatistics.scala — a one-line
scaladoc rewrap that scalafmtAll applies. scalafmtCheckAll is a mandatory pre-PR check per
AGENTS.md and currently fails on #849's head without it.
#849 adds roughly 3,400 lines of engine across a six-stage compiler pipeline. Its own test suite is
substantial, but the coverage is uneven: FunctionRegistry, ExpressionLowering, PrefixEnv and
the new getData overload had no suite of their own. Those untested regions turned out to be where
the defects are — four of the five findings above sit in code with zero or near-zero direct
coverage.
The KNOWN LIMITATION tests matter independently of the coverage numbers. nullif and the
repeated-variable failure are the kind of thing a user discovers after release; pinning them now
means each gets a deliberate decision before the API is public, and every one of those tests
converts into a regression guard the moment a fix lands.
Spark 3.5.8 / Scala 2.12.21, Linux, Java 21:
build/sbt core/test — 715 succeeded, 0 failed, 14 canceled. All tests passed. The 14requireSparkVersionGE(4, 1, …) gates in TriangleCountSuitebuild/sbt "core/testOnly org.graphframes.propertygraph.*" — 395 tests across 13 suites, all pass.build/sbt scalafmtCheckAll and build/sbt "scalafixAll --check" — pass.One note for anyone reproducing locally: org.graphframes.ldbc.TestLDBCCases needs the zstd
binary on PATH to unpack the benchmark data it downloads. Without it, all 8 of its tests fail with
RuntimeException: zstd is not available or not found from LDBCUtils.checkZSTD — unrelated to
this change, but easy to mistake for a regression.
Not run here: the Spark 4.0.1 / 4.1.0 matrix from the AGENTS.md checklist, which CI covers.
🤖 Generated with Claude Code
Sorry, something went wrong.
There was an error while loading. Please reload this page.
optimizations will follow up
FunctionsRegistry, parsing, tests.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Adds tests for the parts of the GQL MATCH pipeline that had no direct
coverage, and pins five defects the new coverage uncovered. No engine
behaviour changes.
New suites:
- FunctionRegistrySuite: the whitelist dispatch table itself (every
accepted name at its documented arity, aliases, case-insensitive
dispatch, all three arity guards, the litStr/litInt slots, the
unknown-name error). Only 4 of ~90 names were exercised before.
- ExpressionLoweringSuite: PrefixEnv and every ExpressionLowering rule,
asserted against the hand-written Spark Column each is documented to
produce, plus literal narrowing and unbound-variable errors.
- PropertyGroupGetDataSuite: the getData(filter, requestedProperties)
contract -- column order, silently dropped unknown names, unmasked
property pass-through, id masking, filter-before-masking, and the
undirected-edge orientation union.
Extended: AstBuilderSuite (literals, malformed patterns, quantifier
parsing, the reserved-word boundary), ResolverSuite (the WHERE
classification buckets with no coverage: edge-variable scan filters,
edge+node and edge+edge conjuncts, non-adjacent and literal-only
conjuncts, multi-position variables, label-validation messages),
QueryExecutorSuite (ScanKey equality, the output schema as a contract,
RETURN *, fan-out RETURN over a missing property),
PropertyGraphFrameQuerySuite (QueryOptions defaults and factories, the
previously untested maxEnumeratedPaths guard and its boundary, explain's
bypass of it), SchemaGraphSnapshotSuite (edge indexing, parallel groups).
Defects pinned by KNOWN LIMITATION tests, each commented with the cause
and a suggested fix:
1. nullif is whitelisted but unusable anywhere -- functions.nullif reads
left.dataType in its constructor and every lowered argument is an
UnresolvedAttribute. nvl/ifnull/coalesce are unaffected.
2. A pattern binding one variable twice fails with AMBIGUOUS_REFERENCE:
PrefixEnv.nodePrefix uses the variable as the column prefix, so both
positions rename to the same columns. Resolution already supports
repeated bindings, so this is an executor-side gap.
3. A scan-local predicate on a bare variable (or on .id) requires the
primary key column to be named literally "id", because scan-local
filters are lowered against raw group columns.
4. An empty result and a non-empty result do not share a schema: the
projection builds lit(...) columns Spark types as non-nullable, while
outputSchema declares every field nullable.
5. RETURN * shares a branch with Projection.Default, so it yields the
fixed output schema rather than the bound variables that explain
advertises.
Also applies the one-line scalafmt rewrap in GraphStatistics.scala that
scalafmtCheckAll (a mandatory pre-PR check) requires.
Verified on Spark 3.5.8 / Scala 2.12.21: core/test passes with no
regressions, scalafmtCheckAll and "scalafixAll --check" pass.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BxbDi8maZPEWKVYx81Rn1h
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.