Support SQLView resources and view composition in SQL on FHIR operations#2639
Merged
Conversation
Spark provisions on-demand persistent volume claims for executor local storage, so the driver service account needs permission to create and remove them. Without this the dynamically created scratch volumes fail to mount.
Generalise the SQLQuery Library parser into a shared SqlLibraryParser that accepts both the SQLQuery and SQLView profiles, keyed by the Library.type code, requiring parameters be absent for a SQLView. Introduce the resolved dependency graph model (ResolvedDependency, ResolvedViewDefinition, ResolvedSqlView, ResolvedDependencyGraph) and a configurable maxDependencyDepth. Make the temp-view registration service key views by canonical identity rather than label so shared nodes materialise once and labels cannot collide across nodes.
Introduce a recursive dependency resolver that turns a top-level query's relatedArtifact references into a topologically ordered graph of resolved ViewDefinition and SQLView nodes, disambiguating each reference (explicit type prefix, else ViewDefinition-first with a SQLView fallback) and deduplicating shared nodes. The executor materialises the graph bottom-up as request-scoped temp views, rewriting and validating each node's SQL before running the top-level query. A SQLQuery can now compose a stored SQLView.
Add resolver unit tests and end-to-end run coverage for the dependency-graph guarantees: a three-level nested chain resolves in topological order, a diamond shares its node once, the same label denotes different resources in different nodes without collision, and cyclic, self-referential, and over-deep graphs are rejected before any SQL executes.
A SQLView Library is now accepted as the top-level resource of $sqlquery-run and $sqlquery-export, at the system, type, and instance levels, executing as a parameter-less query. Supplying parameters with a parameter-less SQLView is rejected, as is a top-level Library whose type is neither sql-query nor sql-view.
With authorisation enabled, reading a ViewDefinition from storage now requires READ on ViewDefinition and reading a SQLView Library requires READ on Library, enforced at every storage-read seam - the standalone $view-run and $view-export operations, the $sqlquery dependency graph, and the top-level by-reference query - layered on top of the existing per-projected-resource checks. A resource supplied inline in the request body is exempt, as it is not read from storage.
Describe SQLView dependencies and top-level SQLViews on the run and export operations, the reference-resolution and disambiguation rules, the cycle/depth rejections, and the metadata-resource READ requirements. Document the maxDependencyDepth configuration option and the ViewDefinition/Library READ authorities, with the inline-versus-stored distinction. Mark authorship on the new and significantly changed files.
Import java.util.List and java.util.ArrayList in the dependency resolver and its test rather than referencing them inline, for a cleaner reading.
Add a stored SQLView Library bundle fixture (and empty variant) mirroring the existing SQLQuery fixtures, in preparation for surfacing SQLViews in the SQL query form.
Generalise the SQLQuery Library list into listStoredLibraries(typeCode), add the sql-view token filter, and expose a useSqlViews hook so the SQL query form can fetch SQLViews alongside SQLQueries. SQLQuery listing keeps its existing query key and behaviour by delegating to the shared core.
Replace the single Library picker with a grouped picker offering stored SQLQueries and SQLViews, omitting an empty group, and rename the read-only dependency heading from Tables to Views. A selected SQLView runs and exports through the unchanged stored path, resolved as Library/<id>.
On the Select query tab the Runtime parameter values section now appears only when the selected source declares parameters, removing the empty panel for SQLViews and param-less SQLQueries. The Provide SQL tab keeps the section always visible to anchor inline parameter authoring.
Rename the inline Tables editor to Views and let each row reference a stored ViewDefinition or SQLView through a grouped source selector. A collision-safe composite option value carries the source kind so the assembled query emits ViewDefinition/<id> or Library/<id> accordingly.
The stored query picker label changed from "SQL query library" to "SQL query source" when SQLViews joined it; point the export e2e helper at the new accessible name.
The inline editor's add-row, label, and source controls were renamed when SQLView references landed; update the inline-authoring e2e paths to match.
The module-level comment described the search endpoint as listing SQLQuery Libraries only; it now lists both SQLQueries and SQLViews.
The SQL on FHIR ViewDefinition model is a CanonicalResource, but Pathling's stored encoder model dropped its url and version on ingestion. Retain both as additive nullable elements so a ViewDefinition can be matched by its canonical URL when resolving SQL on FHIR dependency references.
A relatedArtifact.resource on a SQLQuery or SQLView is a canonical URL of a ViewDefinition or SQLView, matched against the referenced resource's url rather than decomposed into a logical id. Extract the shared url/version parsing and candidate selection into one helper reused by both the Library and ViewDefinition paths, resolve ViewDefinitions by filtering on the url column, reject non-canonical references at parse time, and reject ambiguous and unresolvable references with named errors. Request-supplied export views are matched and keyed by url, and a url-less supplied view is rejected. Bumps the server's Pathling core dependency to pick up the ViewDefinition url/version retention the resolution relies on.
…n UI The inline SQL authoring form now binds each table source to its canonical URL and emits that URL as relatedArtifact.resource on save, so queries authored in the UI resolve without manual editing. Sources without a URL are listed but disabled with an explanation, and a stored reference that matches no known source is surfaced verbatim with a not-found note.
Update the SQL query run/export documentation and runnable examples to reference ViewDefinitions and SQLViews by canonical URL rather than logical id, note that a referenceable view must carry a url (and that existing ViewDefinitions must be re-ingested), and describe the strict, ambiguous, and not-found error responses.
Align the inline picker's disabled-source note with the wireframe's spaced hyphen instead of an em-dash.
The spring-boot:run goal launched the server without --add-opens=java.base/sun.util.calendar=ALL-UNNAMED, so Spark's date conversion failed with an IllegalAccessException whenever a response contained date values (for example a $sqlquery-run returning date columns). The production entrypoint and the test configuration already supplied this option; only the local run path had drifted. Centralise the server JVM module options into the pathling.runtime.jvmModuleOpts and pathling.test.jvmModuleOpts properties so the run, surefire and failsafe configurations share a single definition, and note the coupling with the production entrypoint that a shell script cannot reference at runtime.
The result card left the "Submitted SQL" section blank when running a stored SQLQuery or SQLView, because the page only captured the SQL text for inline requests. The form already resolves the selected library's SQL, so carry it on the stored request (for display only, since the server still receives just the reference) and recover it through a shared request-to-SQL helper. The helper also decodes the Base64 content as a fallback for inline requests lacking a sql-text extension.
The result card rendered the submitted SQL in an unbounded box, so a long query stretched the card far down the page. Extract the existing SQL preview into a shared, height-bounded component with a copy control and reuse it for both the stored-query preview and the submitted SQL echoed in result and error bodies.
Render each referenced view on a single line, clipping overflow with an ellipsis and surfacing the full reference in a tooltip. Allow the form column to shrink so it shares width evenly with the results column rather than being held open by long references.
The static SQL validator walked only a plan's children, but a WITH node exposes its CTE definition bodies as innerChildren. Relation references inside a CTE body were therefore never checked against the declared-label set. An undeclared table named only inside a CTE (such as an OMOP vocabulary table) slipped past validation and surfaced as an opaque 500 from Spark's analyser instead of a clean rejection. Descend into CTE bodies during the strict walk so these references are rejected early with the same "undeclared table" message as top-level relations.
The SQL validator enforces a strict allow-list of expression types. The || concatenation operator parses directly to a Concat expression rather than an UnresolvedFunction, so it was rejected as a disallowed expression even though the equivalent concat() function call was permitted as a built-in. Add Concat to the allow-list so the two spellings behave consistently.
The string-based url and version setters and the null and empty branches of the corresponding getters, copy, and isEmpty logic were not exercised, dropping new-code coverage below the SonarCloud quality gate threshold. Add unit tests covering these paths.
|
|
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
1 participant
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.
Implements the SQLView profile from SQL on FHIR v2, so queries can build on one another instead of each starting from raw FHIR resources.
A SQLView is a reusable, named query identified by its canonical URL. With this change you can:
Dependencies are resolved by canonical URL (with optional version), and a ViewDefinition's
urlandversionare now retained through encoding so stored views can be addressed this way. The resolver walks the dependency graph, guards against cycles and excessive nesting depth, and enforces authorisation on every view reached - resolving a stored view or query requires metadata READ. The behaviour is consistent across the$sqlquery-runand$sqlquery-exportoperations.The admin UI gains the ability to list stored SQLViews, pick one as a query source, and author inline SQL that references a SQLView by its canonical URL.
Documentation under
site/docs/servercovers view composition, the depth limit, and the authorisation model.Closes #2638.