diff --git a/toolbox/mdcode/docs/semantic-model/profiles.md b/toolbox/mdcode/docs/semantic-model/profiles.md index e01d6036..eb776a77 100644 --- a/toolbox/mdcode/docs/semantic-model/profiles.md +++ b/toolbox/mdcode/docs/semantic-model/profiles.md @@ -516,13 +516,14 @@ writes nothing. - **Unrestated column** — the push fails when a named profile neither restates nor excludes a field whose expression reads a column. The error names the field, the profile and the column. -- **Deployments and field dialects** — every push checks the model file's own - bindings against the rules of each deployment's engine, whichever binding the - push deploys. Each field they bind also needs a text its entity's database - runs. See [Validation](reference.md#validation). +- **Deployments and field dialects** — every push checks each deployment + against the rules of its engine, under the model file's own bindings or the + profile it names, whether or not the push deploys it. Each field the model + file binds also needs a text its entity's database runs. See + [Validation](reference.md#validation). - **Unbound inline bindings** — when the model file's own bindings leave an - entity without a source or a relationship without join columns, and the - model declares no deployment, push warns and still pushes. The warning says + entity without a source or a relationship without join columns, and no + deployment uses them, push warns and still pushes. The warning says the model is non-executable when it declares no named profile, and that `default` is unbound when it declares one. `kcmd profiles` prints it under the `default` binding. diff --git a/toolbox/mdcode/docs/semantic-model/reference.md b/toolbox/mdcode/docs/semantic-model/reference.md index 8035b497..322da48d 100644 --- a/toolbox/mdcode/docs/semantic-model/reference.md +++ b/toolbox/mdcode/docs/semantic-model/reference.md @@ -118,10 +118,10 @@ BigQuery dataset or Spanner database exists and, on a push without `--validate-only`, the Knowledge Catalog checks on the entry group. Push's lookup of the file-copy type also runs only at push time, because it calls a service. It exits non-zero when push would -reject a binding. When a plain `kcmd push` of the scope does not run in two -phases, the report checks each profile as `kcmd push --profile ` does. -When it does, the report gives the errors of that push and stops where that -push stops. Under each profile it prints the errors of that profile's own +reject a binding. The report checks each profile as +`kcmd push --profile ` does, unless a plain `kcmd push` of the scope +runs in two phases. In that case it gives the errors of that push and stops +where that push stops. Under each profile it prints the errors of that profile's own checks and of each deployment that uses it. The errors of a deployment that names no profile print under `default`. With `--profile `, the other profiles' errors print above the profiles too, since any of them stops the @@ -137,7 +137,7 @@ instance. | Flag | Effect | |------|--------| | `--profile [name]` | Report only this profile. Naming one the model does not declare is an error, not an empty report. Defaults to every profile. | -| `--print-store` | Print only the store the profile deploys to, on one line and nothing else, for a script to read rather than parse out of the report: `project/instance/database` for a Spanner store, and the backend named ahead of the path for any other (`alloydb:project/region/cluster/instance/database`, `bigquery:project/dataset`). The store is the one the deployments for the profile name: those that name it in `profile`, or that name no profile or `default` for `default`. A legacy profile in `.profiles/` that no deployment names takes the deployments for `default`, unless it sets its own `deployment_target`. Of several, it is the single Spanner or AlloyDB store. For an AlloyDB target named by its cluster, the instance printed is the cluster's primary instance. For a legacy instance-based AlloyDB target, it is the instance the target names. Errors when the scope holds more than one model, since those may name different databases, before any AlloyDB call. Also errors when a deployment names a profile the model does not declare, and when no single store results. Like push, it errors when the model's `extends` do not resolve: one names an entity the model does not declare, they form a cycle, or an entity inherits one field from two unrelated ancestors. A target in no accepted form in any deployment fails it under every profile, and so does a target a `GOOGLE` block lists twice. Like push, it reads every model's profiles first: its profile files, and the profiles a vanilla model carries in its `GOOGLE` block. It errors on a profile file that breaks one of the layout's file rules, such as a `.profile..yaml` file whose `name:` does not match its file name, on a `GOOGLE` block profile that does not load, and on a profile named `default` in any letter case. Like push, it also errors on a profile file that does not load, such as a file in `.profiles/` whose `name:` does not match its file name. It does not run push's checks that every named profile applies to the model, that each is complete, and that each has valid keys and join columns. So a profile that leaves an entity without a source does not stop it. It does not run push's deployment checks, its dialect check of the model file's fields, or its check that each concrete entity restates the inherited fields that read a column either. It does not run push's checks on the copies of the model and profile files that push stores in Knowledge Catalog, such as whether each file is valid UTF-8. For an AlloyDB cluster it also errors when the cluster has no primary instance, when the primary is not READY, or when the instance listing fails. | +| `--print-store` | Print only the store the profile deploys to, on one line and nothing else, for a script to read rather than parse out of the report: `project/instance/database` for a Spanner store, and the backend named ahead of the path for any other (`alloydb:project/region/cluster/instance/database`, `bigquery:project/dataset`). The store is the one the deployments for the profile name: those that name it in `profile`, or that name no profile or `default` for `default`. A legacy profile in `.profiles/` that no deployment names takes the deployments for `default`, unless it sets its own `deployment_target`. Of several, it is the single Spanner or AlloyDB store. For an AlloyDB target named by its cluster, the instance printed is the cluster's primary instance. For a legacy instance-based AlloyDB target, it is the instance the target names. Errors when the scope holds more than one model, since those may name different databases, before any AlloyDB call. Also errors when a deployment names a profile the model does not declare, and when no single store results. Like push, it errors when the model's `extends` do not resolve: one names an entity the model does not declare, they form a cycle, or an entity inherits one field from two unrelated ancestors. A target in no accepted form in any deployment fails it under every profile, and so does a target a `GOOGLE` block lists twice. It reads every model's profiles first: its profile files, and the profiles a vanilla model carries in its `GOOGLE` block. So does `kcmd push --profile `. A plain push that runs in two phases loads the model files before it checks the profile files. It errors on a profile file that breaks one of the layout's file rules, such as a `.profile..yaml` file whose `name:` does not match its file name, on a `GOOGLE` block profile that does not load, and on a profile named `default` in any letter case. Like push, it also errors on a profile file that does not load, such as a file in `.profiles/` whose `name:` does not match its file name. It does not run push's checks that every named profile applies to the model, that each is complete, and that each has valid keys and join columns. So a profile that leaves an entity without a source does not stop it. It does not run push's deployment checks, its dialect check of the model file's fields, or its check that each concrete entity restates the inherited fields that read a column either. It does not run push's checks on the copies of the model and profile files that push stores in Knowledge Catalog, such as whether each file is valid UTF-8. For an AlloyDB cluster it also errors when the cluster has no primary instance, when the primary is not READY, or when the instance listing fails. | This is a read of the binding, never a choice of one. Nothing on any `kcmd` command line names a store directly; `--profile` selects a binding and the @@ -171,15 +171,20 @@ declared profile, as push does, but also one that names a legacy `semantic_model:` profile, which push refuses. It accepts a Spanner deployment that names no profile in a model with metrics, which push refuses. It fails on two BigQuery deployments that use one profile, since they leave no single -store, while push deploys both. It also accepts a profile file that no -deployment uses, as push does, and writes a skill whose store line says there +store, while push deploys both. It accepts a model that binds a table outside +the database a deployment deploys to, which push refuses, unless the model has +actions and that database is the Spanner or AlloyDB store of the selected +profile. It also accepts a profile file that no deployment uses, as push does, +and writes a skill whose store line says there is none. A vanilla model lists its deployments or targets in a `GOOGLE` block. The command fails the same way when a target there is in no accepted form or -listed twice, or when the block's data is not valid JSON. Like push, it reads -every model's profiles first: its profile files, and the profiles a vanilla -model carries in its `GOOGLE` block. It fails on a profile file that breaks one -of the layout's file rules, such as a `.profile..yaml` file whose -`name:` does not match its file name, on a `GOOGLE` block profile that does not +listed twice, or when the block's data is not valid JSON. It reads every +model's profiles first: its profile files, and the profiles a vanilla model +carries in its `GOOGLE` block. So does `kcmd push --profile `. A plain +push that runs in two phases loads the model files before it checks the +profile files. It fails on a profile file that breaks one of the layout's file +rules, such as a `.profile..yaml` file whose `name:` does not +match its file name, on a `GOOGLE` block profile that does not load, and on a profile named `default` in any letter case. Like push, it also fails on a profile file that does not load, such as a file in `.profiles/` whose `name:` does not match its file name. It does not run @@ -636,7 +641,7 @@ and [§4.1](model_spec.md#41-narrowings-stricter-than-ossie). metrics by design and imposes no such requirement. Defined in [model spec §4.1](model_spec.md#41-narrowings-stricter-than-ossie). *(static)* * **Each deployment meets the rules of its engine.** Every push checks each - deployment the model declares, whichever binding the push deploys. A + deployment the model declares, whether or not the push deploys it. A deployment that names no profile uses the model file's own bindings. They must give every concrete entity a source and every relationship both join column lists. They must also bind every field that each deployed metric @@ -646,6 +651,9 @@ and [§4.1](model_spec.md#41-narrowings-stricter-than-ossie). measure, so each metric must be `SUM`, `AVG`, `COUNT`, `MIN` or `MAX` over one operand, and the entity it belongs to needs a primary key or a unique key. The measure rule skips a metric that has only a vendor text. + A deployment that names a profile is held to the measure rule under that + profile, with the keys it states, except for the metrics it leaves + unavailable. On Spanner, that profile must leave every metric unavailable. *(static)* * **Each field the model file binds has a text its entity's database runs.** The field needs an entry in the dialect of its entity's source, an @@ -751,11 +759,22 @@ and [§4.1](model_spec.md#41-narrowings-stricter-than-ossie). `escalate` / `warn` and `critical` / `high` / `medium` / `low` — so an unrecognized word is a hard load error rather than a value that publishes and means nothing. *(static)* +* **Each deployment's tables are in the database it deploys to.** A Spanner or + AlloyDB deployment reads only tables in its target database. A BigQuery + deployment reads only BigQuery or BigLake tables, in any dataset of any + project. A Cloud SQL table fails every deployment. A push in two phases checks + every deployment this way, whether or not `--deployment` names it, and under + `--no-deployment` too. It reads each entity's `source` from the profile the + deployment names, or from the model file when it names none. *(static)* * **Every entity's source table is reachable.** For a **BigQuery-targeting** model, each `source` is probed with a dry-run query, so BigQuery resolves it exactly as the deploy will — a BigQuery table or a BigLake table (e.g. an Iceberg table in a federated REST catalog). A table that does not exist or that - you cannot access fails the push, naming the table and the entity. A + you cannot access fails the push, naming the table and the entity. On a push + that uses deployments, push probes the tables of the deployments it would + deploy, each in the location of the graph's dataset when push can read it. A + table in another location fails the push, and the error names the table's + location when BigQuery reports it. A **Spanner-targeting** model's sources live in Spanner (a different system) and are **not** probed here. The `source` construct and the resource names it takes are defined in [model spec §7.1](model_spec.md#71-table-sources). diff --git a/toolbox/mdcode/src/libts/semantic/resolve_profiles.ts b/toolbox/mdcode/src/libts/semantic/resolve_profiles.ts index 6a7a4a31..3e57e9f6 100644 --- a/toolbox/mdcode/src/libts/semantic/resolve_profiles.ts +++ b/toolbox/mdcode/src/libts/semantic/resolve_profiles.ts @@ -1993,10 +1993,6 @@ export interface CompletenessOptions extends CloudSqlLoad { // applied, the caller stops reading what the SQL reads. This check then // reads what the profile's field expressions read, and gives the warning. applied?: boolean; - // The caller warns about the profile's SQL that the parser cannot read. This - // check then gives that warning only when the hierarchy does not resolve - // with the profile applied. - sqlWarned?: boolean; // The caller warns about the model file's SQL that it reads, which it does // only when the hierarchy of the model it checks resolves. It reads an // ANSI_SQL text in `dialect`, or as generic SQL when that is unset, and @@ -2004,9 +2000,9 @@ export interface CompletenessOptions extends CloudSqlLoad { // text of a field in `rebound`, written as 'Entity.field', because the // profile it applies rebinds or excludes that field. A warning about the // model file's SQL is left out when the field is not in `rebound` and the - // caller reads each text this check cannot read the same way: an entry in a + // caller also warns about each text this check cannot read: an entry in a // dialect other than ANSI_SQL, or an ANSI_SQL text when this profile's - // dialect is `dialect`. + // dialect is `dialect` or the caller cannot read it either. ownSqlOnly?: {dialect?: string; rebound?: string[]}; // The caller transpiles a field or metric whose only text is in a vendor // dialect its translator reads into BigQuery SQL, so a BigQuery profile can @@ -2285,7 +2281,10 @@ export function validateProfileCompleteness( const asWritten = text.dialect !== 'ANSI_SQL' && (owner.dialects ?? []).some( d => d.dialect === text.dialect && d.expression === text.text); - if (!asWritten) readOtherwise.add(what); + // The caller warns about an ANSI_SQL text it cannot read either. + const callerFails = !!opts.ownSqlOnly && + !columnsForSource(text.text, 'ANSI_SQL', opts.ownSqlOnly.dialect); + if (!asWritten && !callerFails) readOtherwise.add(what); }; // Every bound field, and every metric the profile keeps, has a text this // profile's database can run, checked once its sources name one database. @@ -2396,9 +2395,7 @@ export function validateProfileCompleteness( return what === fieldLabel(f.slice(0, dot), f.slice(dot + 1)); }); for (const what of unreadable) { - if (applied ? !profiledError : - opts.sqlWarned && !profiledError || - !own.has(what) && callerWarns(what)) { + if (applied ? !profiledError : !own.has(what) && callerWarns(what)) { continue; } warnings.push( diff --git a/toolbox/mdcode/src/libts/semantic/runtime/runtime.ts b/toolbox/mdcode/src/libts/semantic/runtime/runtime.ts index 87997268..3defcdd6 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/runtime.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/runtime.ts @@ -154,13 +154,13 @@ export async function createSemanticRuntimes(options: CreateRuntimeOptions = {}) return {error: `${err?.message ?? err}`}; } - // Push reads every model's profiles first, whichever profile it deploys: - // its profile files, then the profiles a vanilla model carries in its - // GOOGLE block. It refuses the scope when profileDocuments rejects a file, - // such as a `.profile..yaml` file whose `name:` does not match - // its file name, or when a GOOGLE block profile does not load. It also - // refuses it when a model declares a profile named `default` in any letter - // case. + // The runtime reads every model's profiles first, as + // `kcmd push --profile ` does: its profile files, then the profiles + // a vanilla model carries in its GOOGLE block. It refuses the scope when + // profileDocuments rejects a file, such as a `.profile..yaml` + // file whose `name:` does not match its file name, or when a GOOGLE block + // profile does not load. It also refuses it when a model declares a profile + // named `default` in any letter case. const profilesByDoc = new Map(); // Each model's profile files alone, which an error applying one names. const filesByDoc = diff --git a/toolbox/mdcode/src/libts/semantic/runtime/store.ts b/toolbox/mdcode/src/libts/semantic/runtime/store.ts index 29c9eec9..f509400b 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/store.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/store.ts @@ -195,7 +195,7 @@ function strayBindings(model: SemanticModel, store: ChosenStore): string[] { * Neither is set when the profile names no store, unless a target is in no * accepted form or listed twice, or a GOOGLE block is not valid JSON. Neither * is set either when a BigQuery model binds a table outside the graph's - * dataset, which push accepts. + * dataset. */ export interface StoreError { error: string; @@ -233,7 +233,7 @@ export interface StoreError { * 6. There is no target, so there is nothing to run against. * 7. The model binds a table outside the store. For a Spanner or AlloyDB * store, a write would land in the wrong place, and the error is named. A - * BigQuery graph may read tables in any dataset, and push accepts that, so + * BigQuery graph may read tables in any dataset, so * for a BigQuery store the error is not named. */ export function resolveStore( @@ -347,7 +347,7 @@ export function resolveStore( `target store's table of that name rather than in the bound one. ` + `Bind both to the same store.`, // A BigQuery graph may read tables in any dataset, BigLake included, - // and push accepts that. The model then has no store, and a caller + // so the model then has no store, and a caller // does not fail. ...(store.kind === 'bigquery' ? {} : {named: true}), }; diff --git a/toolbox/mdcode/src/libts/semantic/validate.ts b/toolbox/mdcode/src/libts/semantic/validate.ts index 4e8e31d8..150cfa1f 100644 --- a/toolbox/mdcode/src/libts/semantic/validate.ts +++ b/toolbox/mdcode/src/libts/semantic/validate.ts @@ -12,14 +12,14 @@ import {BigQueryClient} from '../gcp/bigquery'; import {SpannerClient} from '../gcp/spanner'; import {googleDeploymentTargets} from './deploy_bigquery'; -import {resolveModelDeployments, SpannerGraphTarget, spannerGraphTargets, targetError} from './deployment_target'; -import {Action, ActionParameter, Constraint, DATA_TYPES, Executor, expressionForDialect, Field, FIELD_DEFINITION_KEYS, isFieldBound, Metric, SemanticModel, SQL_EXECUTOR_VERBS} from './ir'; +import {ResolvedDeployment, resolveModelDeployments, SpannerGraphTarget, spannerGraphTargets, targetError} from './deployment_target'; +import {Action, ActionParameter, Constraint, DATA_TYPES, Entity, Executor, expressionForDialect, Field, FIELD_DEFINITION_KEYS, isFieldBound, Metric, ProfileSpec, SemanticModel, SQL_EXECUTOR_VERBS} from './ir'; import {relationshipLinkId} from './kc_ids'; -import {DEPLOYMENTS_VERSION_HINT, HALF_BOUND_JOIN_PROBLEM, inheritedNoSqlProblems, joinLengthProblem, listed, LoadedModel, noSqlProblemOf, noSqlReason, OSSIE_VERSION, undefinedEndpointProblem} from './loader'; +import {databaseOf, DEPLOYMENTS_VERSION_HINT, HALF_BOUND_JOIN_PROBLEM, inheritedNoSqlProblems, joinLengthProblem, listed, LoadedModel, noSqlProblemOf, noSqlReason, OSSIE_VERSION, SemanticModelError, undefinedEndpointProblem} from './loader'; import {MAX_ENTRY_DESCRIPTION, NameKind, nameProblem} from './names'; import {bindScalar, sentence, storeCodeFor} from './parameters'; import {DeclaredConcept, declaredConceptFields, InheritanceError, resolveInheritance} from './resolve_inheritance'; -import {applyProfileExclusions, columnBinding, DEFAULT_PROFILE, evaluatedTexts, expressionSources, inheritanceErrorOf, inheritedNamesOf, ProfileExclusion, pruneUnavailable, reservedProfileNameError, resolvedEntities, sourcesDialect, statesKey} from './resolve_profiles'; +import {applyProfile, applyProfileExclusions, columnBinding, DEFAULT_PROFILE, evaluatedTexts, expressionSources, inheritanceErrorOf, inheritedNamesOf, ProfileExclusion, pruneUnavailable, reservedProfileNameError, resolvedEntities, sourcesDialect, statesKey} from './resolve_profiles'; import {columnsForSource, coversKeyByColumns, entitiesReached, fieldsReadOn, isColumnName, isSingleAggregate, referencedEntityNames} from './sql_expr_utils'; import {leadingDmlVerb, referencedParameters} from './sql_identifiers'; import {SqlColumn, withoutComments} from './sql_parser'; @@ -247,29 +247,45 @@ function nameAndDescriptionErrors( return errors; } -// Checks the model file's own bindings against the engine of each deployment -// of `model` that names no profile, and returns one message per problem, -// naming every deployment it applies to. With `opts.joinsChecked`, missing -// join columns are left to the caller. +// Checks each deployment of `model` against the rules of its engine, under the +// profile it names in `model.profiles` or the model file's own bindings, and +// returns one message per problem, naming every deployment it applies to. export function validateDeployments( - model: SemanticModel, document: string, - opts: {joinsChecked?: boolean} = {}): string[] { + model: SemanticModel, document: string): string[] { // The deployments each problem applies to, keyed by the text that follows // their names in the message. const problems = new Map(); const add = (deployment: string, problem: string) => problems.set(problem, [...(problems.get(problem) ?? []), deployment]); - const unbound = unboundInline(model, !opts.joinsChecked); + const unbound = unboundInline(model); // The metrics the reference rules report, which the BigQuery rule leaves to // them. let referenced: Set|undefined; for (const d of model.deployments ?? []) { - if ((d.profile ?? DEFAULT_PROFILE) !== DEFAULT_PROFILE) continue; + // A legacy wrapper is not in `model.profiles`, and is left out. + const named = model.profiles?.find(p => p.name === d.profile); + if ((d.profile ?? DEFAULT_PROFILE) !== DEFAULT_PROFILE && !named) continue; const resolved = resolveModelDeployments(model, [d.name]); const engine = 'error' in resolved ? undefined : resolved.deployments[0].engine; const alloyDb = engine === 'alloydb'; - if (unbound) { + // The model the deployment's bindings give, less what they leave + // unavailable. The completeness check reports a named profile that does + // not apply, and the fields its metrics read. + const bound = named ? profiled(model, named) : pruneUnavailable( + model, DEFAULT_PROFILE, alloyDb ? 'ALLOYDB' : sourcesDialect(model), + {noGraph: alloyDb}); + if (!bound) continue; + const available = bound.model.metrics ?? []; + if (named) { + if (engine === 'spanner' && available.length) { + add(d.name, + `targets Spanner, which has no measures, and profile '${ + named.name}' leaves metric(s) ${ + listed(available.map(m => `'${m.name}'`))} available; name ` + + `them in its 'metrics_exclude', or set it to "*".`); + } + } else if (unbound) { add(d.name, `uses the model file's own bindings, which leave ${ unbound}; a deployment needs every entity and relationship ` + @@ -277,15 +293,12 @@ export function validateDeployments( } // A Spanner deployment deploys no metric. if (engine === 'spanner') continue; - const {report} = pruneUnavailable( - model, DEFAULT_PROFILE, alloyDb ? 'ALLOYDB' : sourcesDialect(model), - {noGraph: alloyDb}); - for (const {name, field} of report.droppedMetrics) { + for (const {name, field} of named ? [] : bound.report.droppedMetrics) { if (field === undefined) continue; add(d.name, `uses the model file's own bindings, under which metric '${ name}' reads field '${field}', which ${ - report.unboundFields.includes(field) ? + bound.report.unboundFields.includes(field) ? 'has no expression' : 'reads a field with no expression, directly or through ' + 'other fields'}; a deployment needs every field its ` + @@ -296,11 +309,9 @@ export function validateDeployments( referenced = new Set(); validateExpressionReferences([{document, model}], false, [], referenced); } - for (const m of model.metrics ?? []) { - // The loop above reports a metric that pruning drops. - const problem = !referenced.has(m.name) && - !report.droppedMetrics.some(x => x.name === m.name) && - measureProblem(model, m); + for (const m of available) { + const problem = + !referenced.has(m.name) && measureProblem(bound.model, m); if (problem) { add(d.name, `targets BigQuery, where metric '${m.name}' cannot be a measure: ${ @@ -317,6 +328,18 @@ export function validateDeployments( }); } +// `model` with `profile` applied and what it leaves unavailable pruned, or +// undefined when the profile does not apply. +function profiled(model: SemanticModel, profile: ProfileSpec): + ReturnType|undefined { + try { + return pruneUnavailable(applyProfile(model, profile), profile.name); + } catch (err) { + if (!(err instanceof SemanticModelError)) throw err; + return undefined; + } +} + // Why `metric` cannot be a BigQuery measure, or undefined when it can. Leaves // out a metric that validateBigQueryMetrics or the inheritance rules report: // one that reads several entities or none, or one on an abstract or extended @@ -404,13 +427,15 @@ export function validateInheritedColumns( return errors; } -// The warning for a model file whose own bindings are incomplete and that -// declares no deployment. With `namedProfiles`, it says 'default' is unbound, -// and otherwise that the model cannot be queried at all. +// The warning for a model file whose own bindings are incomplete and that no +// deployment uses. With `namedProfiles`, it says 'default' is unbound, and +// otherwise that the model cannot be queried at all. export function inlineBindingWarnings( model: SemanticModel, document: string, namedProfiles: boolean): string[] { - const unbound = unboundInline(model, true); - if (!unbound || model.deployments?.length) return []; + const unbound = unboundInline(model); + const used = (model.deployments ?? []).some( + d => (d.profile ?? DEFAULT_PROFILE) === DEFAULT_PROFILE); + if (!unbound || used) return []; const where = `model '${model.name}' (${document})`; return [ namedProfiles ? @@ -423,13 +448,12 @@ export function inlineBindingWarnings( } // What the model file's own bindings leave unbound, such as "entity 'a' -// without a source", or '' when nothing is. Relationships count only with -// `joins`. -function unboundInline(model: SemanticModel, joins: boolean): string { +// without a source", or '' when nothing is. +function unboundInline(model: SemanticModel): string { const sourceless = (model.entities ?? []) .filter(e => !e.abstract && !e.dataSource) .map(e => e.name); - const unjoined = !joins ? [] : + const unjoined = (model.relationships ?? []) .filter(r => !r.association && !(r.source.columns.length && r.destination.columns.length)) @@ -1734,6 +1758,48 @@ function missingExecutorFields(ex: Executor): string[] { } +// One error for each entity that `deployment` binds to a table outside the +// database it deploys to. Sources come from the deployment's profile in +// `model.profiles`, or from the model file for `default`. +export function deploymentTableErrors( + document: string, model: SemanticModel, + deployment: ResolvedDeployment): string[] { + let sourceOf = (e: Entity): string|undefined => e.authoredSource; + let bindings = 'the model file'; + if (deployment.profile !== DEFAULT_PROFILE) { + const profile = model.profiles?.find(p => p.name === deployment.profile); + // `model.profiles` lacks a legacy wrapper, so it is not checked. + if (!profile) return []; + const bound = new Map(profile.entities.map(pe => [pe.name, pe.source])); + sourceOf = e => bound.get(e.name); + bindings = `profile '${profile.name}'`; + } + // The database the tables must be in, as databaseOf gives it, and how an + // error names the target and a table outside it. + const {engine, coordinates: c} = deployment; + let database = 'bigquery'; + let target = 'a BigQuery graph'; + let outside = 'which is not a BigQuery or BigLake table'; + if (engine !== 'bigquery') { + const [system, name] = engine === 'spanner' ? + ['Spanner', `${c.project}/${c.instance}/${c.database}`] : + ['AlloyDB', `${c.project}/${c.location}/${c.cluster}/${c.database}`]; + database = `${engine}/${name}`; + target = `${system} database '${name}'`; + outside = 'which is not a table in that database'; + } + const errors: string[] = []; + for (const e of model.entities ?? []) { + const source = sourceOf(e)?.trim(); + if (!source || databaseOf(source) === database) continue; + errors.push( + `[${document}] deployment '${deployment.name}' targets ${target}, ` + + `but ${bindings} binds entity '${e.name}' to '${source}', ${outside}.`); + } + return errors; +} + + // Live pre-flight over the BigQuery-targeting models: confirms every entity's // BigQuery source table is reachable BEFORE any destination leg runs, so a push // fails fast when the model could not deploy, rather than surfacing a missing @@ -1754,9 +1820,15 @@ function missingExecutorFields(ex: Executor): string[] { // the deploy runs against), falling back to `defaultProject`. Each distinct // (billing project, reference) pair is probed once. Returns one message per // unreachable table (empty when all pass). +// +// With `graph`, each probe runs in `graph.location`, the location of dataset +// `graph.dataset`, and a probe that fails there runs again with no location. +// When that run finds the table in another location, the error says so, +// starting with `graph.at`, which names the deployment. export async function validateBigQueryDataSources( - models: LoadedModel[], bq: BigQueryClient, - defaultProject: string): Promise { + models: LoadedModel[], bq: BigQueryClient, defaultProject: string, + graph?: {at: string; dataset: string; location: string}): + Promise { // Dedup by billing project + reference so a table shared across // entities/models is probed once; keep the first reference for a locatable // error message. @@ -1788,9 +1860,22 @@ export async function validateBigQueryDataSources( const errors: string[] = []; for (const {project, ref, document, model, entity} of refs.values()) { - const res = await bq.query( - project, `SELECT 1 FROM \`${ref}\` WHERE FALSE`, undefined, true); + const sql = `SELECT 1 FROM \`${ref}\` WHERE FALSE`; + const res = await bq.query(project, sql, graph?.location, true); if (res.status === 200) continue; + if (graph) { + const anywhere = await bq.query(project, sql, undefined, true); + const found = anywhere.result?.jobReference?.location; + if (found && found !== graph.location) { + errors.push( + `${graph.at} targets a BigQuery graph in dataset ` + + `'${graph.dataset}', which is in location '${graph.location}', ` + + `but entity '${entity}' reads table '${ref}', which is in ` + + `location '${found}'. A property graph can use only tables in ` + + `its dataset's location.`); + continue; + } + } const msg = res.message?.trim() || `HTTP ${res.status}`; const why = /not found/i.test(msg) ? 'does not exist' : diff --git a/toolbox/mdcode/src/tool/commands.ts b/toolbox/mdcode/src/tool/commands.ts index fc89b7dc..52615c64 100644 --- a/toolbox/mdcode/src/tool/commands.ts +++ b/toolbox/mdcode/src/tool/commands.ts @@ -29,7 +29,7 @@ import {profileTargets, storeLine} from '../libts/semantic/runtime/store'; import {generateSkill, SkillPackage} from '../libts/semantic/skills'; import {loadSqlEngine} from '../libts/semantic/sql_parser'; import {transpileModels} from '../libts/semantic/transpile'; -import {inlineBindingWarnings, storeMessage, validateActions, validateBigQueryActionStatements, validateBigQueryDataSources, validateDeployments, validateExpressionReferences, validateInheritance, validateInheritedColumns, validateInlineDialects, validatePushRequirements, validateSpannerActionStatements} from '../libts/semantic/validate'; +import {deploymentTableErrors, inlineBindingWarnings, storeMessage, validateActions, validateBigQueryActionStatements, validateBigQueryDataSources, validateDeployments, validateExpressionReferences, validateInheritance, validateInheritedColumns, validateInlineDialects, validatePushRequirements, validateSpannerActionStatements} from '../libts/semantic/validate'; import {YAML_OPTIONS} from '../libts/semantic/yaml_options'; import {Sources} from '../libts/source'; import {SemanticModelSource} from '../libts/sources/semantic-model'; @@ -668,8 +668,7 @@ interface LocalCheckError { // `pushedProfile`, so this check leaves it out. The warnings go to `warnings`. function namedProfileErrors( docs: ProfiledDoc[], checkedByDoc: Map, - pushChecks: (doc: ProfiledDoc, profile: string) => - {applied?: boolean; sqlWarned?: boolean}, + pushChecks: (doc: ProfiledDoc, profile: string) => {applied?: boolean}, profileDocs: ProfileDocs, declared: DeclaredProfiles, warnings: string[], transpile: boolean, pushedProfile: string, load: CloudSqlLoad): LocalCheckError[] { @@ -768,9 +767,9 @@ function namedProfileErrors( return errors; } -// The errors validateInlineDialects, validateInheritedColumns and -// validateDeployments give for each model file in `docs`, whichever binding -// push deploys. The warnings from inlineBindingWarnings go to `warnings`. +// The errors validateInlineDialects and validateInheritedColumns give for +// each model file in `docs`, whichever binding push deploys. The warnings from +// inlineBindingWarnings go to `warnings`. function modelFileBindingErrors( docs: ProfiledDoc[], checkedByDoc: Map, declared: DeclaredProfiles, warnings: string[], @@ -791,13 +790,6 @@ function modelFileBindingErrors( if (named !== undefined) { warnings.push(...inlineBindingWarnings(base, d.name, named)); } - // A legacy wrapper push deploys the wrapper's bindings, not these. - if (d.excluded) continue; - // Push reports missing join columns itself for a model with a graph target. - const loaded = {document: d.name, model: base}; - const joinsChecked = !d.applied && - (hasTargetType(loaded, 'bigquery') || hasTargetType(loaded, 'spanner')); - errors.push(...validateDeployments(base, d.name, {joinsChecked})); } return errors; } @@ -1875,6 +1867,7 @@ async function checkBeforeWrites( // Every deployment's target must be in an accepted form, and its profile one // the model declares, whether or not this push selects the deployment. const selectedByDoc = new Map(); + const everyByDoc = new Map(); for (const {document, model} of models) { const every = resolveModelDeployments(model); if ('error' in every) return stop(ofModel(document, every.error)); @@ -1884,6 +1877,7 @@ async function checkBeforeWrites( const undeclared = undeclaredProfileError( document, model, declaredNames(document)); if (undeclared) return stop(ofModel(document, undeclared)); + everyByDoc.set(document, every.deployments); selectedByDoc.set(document, resolved.deployments); } const byDefault = defaultProfileCheck( @@ -1896,23 +1890,27 @@ async function checkBeforeWrites( // are the model file's own bindings and each deployment. The catalog's model // is checked as a model that deploys nowhere, though one with no deployments // list keeps its GOOGLE block targets. A selected deployment's own checks - // run only after these pass, so these give every error. They leave to those - // checks only the warning about its profile's SQL that kcmd cannot read. + // run only after these pass, so these give every error and every warning + // about SQL kcmd cannot read. const profileWarnings: string[] = []; const profileErrors = namedProfileErrors( - docs, new Map(loaded.models.map(m => [m.document, m.model])), - (d, name) => ({ - sqlWarned: !!selectedByDoc.get(d.name)?.some(s => s.profile === name), - }), + docs, new Map(loaded.models.map(m => [m.document, m.model])), () => ({}), profileDocs, declaredBy, profileWarnings, !!options.transpile, DEFAULT_PROFILE, load); for (const {document, model} of models) { - profileErrors.push(...ofProfile( - document, DEFAULT_PROFILE, - ...validateInlineDialects( - model, document, {transpiled: !!options.transpile}), - ...validateInheritedColumns(model, document), - ...validateDeployments(model, document))); + profileErrors.push( + ...ofProfile( + document, DEFAULT_PROFILE, + ...validateInlineDialects( + model, document, {transpiled: !!options.transpile}), + ...validateInheritedColumns(model, document)), + // One call per profile, so each error carries its deployments' profile. + ...[...new Set(everyByDoc.get(document)!.map(d => d.profile))].flatMap( + profile => ofProfile(document, profile, ...validateDeployments({ + ...model, + deployments: model.deployments!.filter( + d => (d.profile ?? DEFAULT_PROFILE) === profile), + }, document)))); const named = declaredNames(document).some(n => n.toLowerCase() !== DEFAULT_PROFILE); profileWarnings.push(...inlineBindingWarnings(model, document, named)); @@ -1938,6 +1936,14 @@ async function checkBeforeWrites( if (profileErrors.length || kcErrors.length) { return stop([...profileErrors, ...kcErrors]); } + // Each deployment's tables must be in the database it deploys to, whether + // or not this push selects the deployment. + const tableErrors = models.flatMap( + ({document, model}) => (everyByDoc.get(document) ?? []) + .flatMap(d => ofProfile( + document, d.profile, + ...deploymentTableErrors(document, model, d)))); + if (tableErrors.length) return stop(tableErrors); const plans: DeploymentPlan[] = []; const notes: string[] = []; @@ -2099,10 +2105,10 @@ async function deploymentPlan( for (const w of transpiled.warnings) warnDeployment(w); } // Pruning can remove the field these rules are about, so they run first. - const unprunedWarnings: string[] = []; + // checkBeforeWrites has already warned about the SQL they cannot read. const unprunedErrors = [ ...validateInheritance([checked]), - ...validateExpressionReferences([checked], false, unprunedWarnings), + ...validateExpressionReferences([checked]), ]; const {model: pruned, report} = pruneUnavailable(checked.model, deployment.profile); @@ -2126,7 +2132,6 @@ async function deploymentPlan( v2Aspects: kcV2Aspects(), }), ]; - for (const w of unprunedWarnings) warnDeployment(w); if (errors.length) return {errors}; if (deployment.engine === 'alloydb') { return {deployment, at, model: narrowed}; @@ -2140,8 +2145,8 @@ async function deploymentPlan( } // The errors of one deployment's live checks, which change nothing. They read -// the dataset or database, probe each BigQuery source, dry-run the BigQuery DDL -// in the dataset's location and check each action's DML. A dataset whose +// the dataset or database, probe each BigQuery source and dry-run the BigQuery +// DDL in the dataset's location, and check each action's DML. A dataset whose // location cannot be read adds a warning to `warnings`. async function liveErrors( plan: DeploymentPlan, ctx: context.ApiContext, defaultProject: string, @@ -2161,7 +2166,7 @@ async function liveErrors( } if (deployment.engine !== 'bigquery') return []; const bq = new BigQueryClient(ctx); - const {project} = deployment.coordinates; + const {project, dataset} = deployment.coordinates; let location: string|undefined; try { location = @@ -2169,8 +2174,10 @@ async function liveErrors( } catch (err: any) { return [`${at}: ${err.message}`]; } + const graph = + location ? {at, dataset: `${project}.${dataset}`, location} : undefined; const accessErrors = - await validateBigQueryDataSources([model], bq, defaultProject); + await validateBigQueryDataSources([model], bq, defaultProject, graph); if (accessErrors.length) return accessErrors; for (const ddl of plan.generated?.ddl ?? []) { const res = await bq.query(project, ddl, location, true); @@ -2365,15 +2372,8 @@ export async function profiles(options: ProfilesOptions = {}): Promise { } }; - // The profiles are read before anything prints, as a graph push reads them. - // A profile file that breaks a layout rule throws. When a model's GOOGLE - // block profiles cannot be read, the error is printed and the command exits - // 1. - for (const doc of docs) { - if (!readDeclaredProfiles(declaredBy, doc)) return 1; - } - // The errors of a plain push's checks, when a plain push runs in two - // phases. They replace the checks below. + // The errors of a plain push's checks, when it runs in two phases. They + // replace the checks below. let twoPhase: LocalCheckError[]|undefined; if (takesTwoPhasePath({}, layout, docs)) { const checked = await checkBeforeWrites( @@ -2391,6 +2391,22 @@ export async function profiles(options: ProfilesOptions = {}): Promise { } const twoPhaseErrors = (belongs: (e: LocalCheckError) => boolean) => [...new Set((twoPhase ?? []).filter(belongs).map(e => e.error))]; + // Reads every model's profiles. When that fails after a two-phase push + // stopped at another error, prints that push's errors instead. + for (const doc of docs) { + try { + declaredBy(doc); + } catch (err: any) { + const stopped = twoPhaseErrors(() => true); + if (stopped.length && !stopped.includes(err.message)) { + printRejects('', stopped); + return 1; + } + if (!(err instanceof SemanticModelError)) throw err; + console.error(`Error: ${err.message}`); + return 1; + } + } // The two-phase errors that belong to `document` and `profile`. With // neither, those of the whole push. const twoPhaseErrorsOf = (document?: string, profile?: string) => @@ -2490,9 +2506,9 @@ export async function profiles(options: ProfilesOptions = {}): Promise { const res = profileForDocument(doc, available, name, load); // Narrows the type; `name` comes from `available`, so it is found. if (res.kind === 'missing') continue; - // Push applies the profile to every model in the scope, so a model that - // does not declare it fails the push. - // A two-phase push applies it only for the deployments that use it. + // `kcmd push --profile ` applies the profile to every model in + // the scope, so a model that does not declare it fails the push. A + // two-phase push applies it only for the deployments that use it. const errors = docs.filter(d => d.name !== doc.name).flatMap(d => { const names = declaredBy(d).map(p => p.name); return twoPhase || names.includes(name) ? diff --git a/toolbox/mdcode/tests/libts/semantic/resolve_profiles.test.ts b/toolbox/mdcode/tests/libts/semantic/resolve_profiles.test.ts index 0bb1dbca..248c44e2 100644 --- a/toolbox/mdcode/tests/libts/semantic/resolve_profiles.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/resolve_profiles.test.ts @@ -1492,6 +1492,8 @@ describe('validateProfileCompleteness', () => { `alloydb.googleapis.com/projects/p/locations/us/clusters/c/databases/db/schemas/public/tables/${t}`; const m = irModel(); m.entities[0].fields.push({name: 'code', expression: 'Customer.name + 0x1F'}); + // Neither dialect reads this text, so the caller warns about it itself. + m.entities[0].fields.push({name: 'broken', expression: 'LOWER(Customer.name'}); const pg = profile({ name: 'pg', entities: [ @@ -2784,12 +2786,12 @@ relationships: [] "profile 'prod': kcmd could not read the SQL of field 'F.f', so the " + 'fields it reads were not checked', ]); - // A --deployment push warns about the model file's SQL when it loads the - // model file, so this check leaves that warning out. - const deferred: string[] = []; - validateProfileCompleteness(model, spec, deferred, - {sqlWarned: true, ownSqlOnly: {dialect: 'BIGQUERY'}}); - expect(deferred).toEqual([]); + // Push warns about the model file's SQL in its own check of the model + // file, so this check leaves that warning out. + const leftOut: string[] = []; + validateProfileCompleteness( + model, spec, leftOut, {ownSqlOnly: {dialect: 'BIGQUERY'}}); + expect(leftOut).toEqual([]); }); // Push's own check of what a field reads stands down on a broken hierarchy diff --git a/toolbox/mdcode/tests/libts/semantic/validate.test.ts b/toolbox/mdcode/tests/libts/semantic/validate.test.ts index 866b2569..fe0e82c7 100644 --- a/toolbox/mdcode/tests/libts/semantic/validate.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/validate.test.ts @@ -3,7 +3,7 @@ import {describe, expect, test} from 'bun:test'; -import {Action, CustomExtension, Entity, Field, Metric, SemanticModel} from '../../../src/libts/semantic/ir'; +import {Action, CustomExtension, Entity, Field, Metric, ProfileSpec, SemanticModel} from '../../../src/libts/semantic/ir'; import {listed, LoadedModel, loadModels} from '../../../src/libts/semantic/loader'; import {mergeProfileOntoDoc} from '../../../src/libts/semantic/resolve_profiles'; import {isColumnName} from '../../../src/libts/semantic/sql_expr_utils'; @@ -415,6 +415,35 @@ describe('validateBigQueryDataSources', () => { expect(errs).toEqual([]); expect(calls).toBe(1); }); + + test('in the graph dataset\'s location, gives the probe\'s own error unless the retry finds another location', async () => { + // Probes table p.x.t with a BigQuery that answers `located` to a probe + // that names a location and `anywhere` to one that names none, and gives + // each probe's location, or null for none. + async function probe(located: unknown, anywhere: unknown) { + const bq = new BigQueryClientMock(); + const locations: unknown[] = []; + bq.query = (async (_p: string, _sql: string, location?: string) => { + locations.push(location ?? null); + return location ? located : anywhere; + }) as any; + const errors = await validateBigQueryDataSources( + [loaded(model({entities: [entity('o', 'p.x.t')]}))], bq, 'p', + {at: "[doc] deployment 'g'", dataset: 'p.d', + location: 'us-central1'}); + return {errors, locations}; + } + const missing = {status: 404, message: 'Not found: Table'}; + expect(await probe(missing, missing)).toEqual({ + errors: [expect.stringContaining( + "BigQuery table 'p.x.t', which does not exist")], + locations: ['us-central1', null], + }); + const there = await probe({status: 503, message: 'Backend error'}, + {status: 200, result: {jobReference: {location: 'us-central1'}}}); + expect(there.errors).toEqual([expect.stringContaining( + 'which could not be verified (Backend error)')]); + }); }); describe('action parameters', () => { @@ -2859,8 +2888,8 @@ const TARGET = { 'alloydb.googleapis.com/projects/p/locations/us-central1/clusters/c/databases/db', }; -// Every deployment is held to the rules of its engine on the model file's own -// bindings, whatever profile it names. +// Every deployment is held to the rules of its engine, under the profile it +// names or the model file's own bindings. describe('validateDeployments', () => { type Engine = keyof typeof TARGET; // A deployment entry named after its engine. @@ -2891,9 +2920,7 @@ ${customer ? ` source: ${BQ_TABLE('customer')}\n` : ''}\ - { name: placed_by, from: purchase, to: customer${ join ? ', from_columns: [p_customer], to_columns: [c_id]' : ''} } ${metric && ` metrics:\n - ${metric}\n`}`); - const check = (m: SemanticModel, - opts: Parameters[2] = {}) => - validateDeployments(m, 'commerce', opts); + const check = (m: SemanticModel) => validateDeployments(m, 'commerce'); // The start of the error saying deployment `engine` uses the model file's // own bindings. A list of engines gives the plural form that names each. const OWN = (engine: Engine|Engine[]) => typeof engine === 'string' ? @@ -2985,7 +3012,7 @@ ${shop ? ` - { name: shop, source: ${BQ_TABLE('shop')}, primary_key: [s_id] [READS(['alloydb', 'bigquery'], 'm', 'customer.label', false)]); }); - test('names every concrete entity and relationship left unbound, and leaves out the relationships under joinsChecked', () => { + test('names every concrete entity and relationship left unbound', () => { const m = commerce( {customer: false, join: false, deployments: deployment('alloydb')}); m.entities[1].dataSource = ''; @@ -2994,12 +3021,6 @@ ${shop ? ` - { name: shop, source: ${BQ_TABLE('shop')}, primary_key: [s_id] 'alloydb', "entities 'customer' and 'purchase' without a source and " + "relationships 'placed_by' and 'billed_to' without join columns")]); - // The caller reports a relationship with no join columns itself. - const noJoin = (customer: boolean) => check( - commerce({customer, join: false, deployments: BQ}), - {joinsChecked: true}); - expect(noJoin(true)).toEqual([]); - expect(noJoin(false)).toEqual([UNBOUND('bigquery')]); // An abstract entity has no table, so it needs no source. const exempt = commerce({deployments: BQ}); exempt.entities.push( @@ -3024,7 +3045,7 @@ ${abstract ? ' abstract: true\n' : ' primary_key: [c_id]\n'}\ expect(check(hierarchy(true))).toEqual([]); }); - test('incomplete inline bindings give one warning when the model declares no deployment', () => { + test('incomplete inline bindings give one warning when no deployment uses them', () => { const warned = (m: SemanticModel, named = false) => inlineBindingWarnings(m, 'commerce', named); const NON_EXECUTABLE = (left: string) => @@ -3154,6 +3175,62 @@ ${abstract ? ' abstract: true\n' : ' primary_key: [c_id]\n'}\ expect(check(extended)).toEqual([]); }); }); + + // A deployment that names a profile reads it from `model.profiles`: its + // metrics_exclude, the metrics it prunes and the keys it states. + describe('under a named profile', () => { + // The errors for metric `metric` on deployment `engine`, which names + // profile ops, with ops set to `ops` and purchase keyed by `keys`. + const underOps = (engine: Engine, metric: string, + ops: Partial = {}, keys = ['p_id']) => { + const m = commerce({deployments: deployment(engine, 'ops'), metric}); + m.entities[1].keys = keys; + m.profiles = [{name: 'ops', entities: [], relationships: [], ...ops}]; + return check(m); + }; + const SUM = + '{ name: m, entity: purchase, expression: "SUM(purchase.amount)" }'; + + test('a Spanner deployment fails while its profile leaves a metric available', () => { + expect(underOps('spanner', SUM, {metricsExclude: '*'})).toEqual([]); + // The completeness check reports a profile that does not apply. + expect(underOps('spanner', SUM, {metricsExclude: ['m', 'other']})) + .toEqual([]); + expect(underOps('spanner', SUM)).toEqual([ + "model 'commerce' (commerce) deployment 'spanner' targets Spanner, " + + "which has no measures, and profile 'ops' leaves metric(s) 'm' " + + "available; name them in its 'metrics_exclude', or set it to \"*\".", + ]); + }); + + test('a BigQuery deployment holds each metric its profile leaves to the measure rules, with the keys the profile states', () => { + const ratio = '{ name: m, entity: purchase, ' + + 'expression: "SUM(purchase.amount) / 2" }'; + // A metric the profile excludes, or prunes for a field it excludes, is + // held to none of this. + expect(underOps('bigquery', ratio, {metricsExclude: ['m']})).toEqual([]); + const noAmount = + {entities: [{name: 'purchase', fieldsExclude: ['amount']}]}; + expect(underOps('bigquery', ratio, noAmount)).toEqual([]); + expect(underOps('bigquery', ratio)).toEqual([ + "model 'commerce' (commerce) deployment 'bigquery' targets BigQuery, " + + "where metric 'm' cannot be a measure: it is not SUM, AVG, COUNT, " + + 'MIN or MAX over one operand.', + ]); + // Purchase has no key in the model file, and profile ops states one. + const keyed = {entities: [{name: 'purchase', primaryKey: ['p_id']}]}; + expect(underOps('bigquery', SUM, keyed, [])).toEqual([]); + expect(underOps('bigquery', SUM, {}, [])).toEqual([ + expect.stringContaining("which has no primary key or unique key"), + ]); + }); + + test('a deployment whose profile is a legacy wrapper is left out', () => { + const m = commerce( + {customer: false, deployments: deployment('bigquery', 'old')}); + expect(check(m)).toEqual([]); + }); + }); }); describe('validateInlineDialects', () => { diff --git a/toolbox/mdcode/tests/tool/commands.test.ts b/toolbox/mdcode/tests/tool/commands.test.ts index 880b8a92..3c321625 100644 --- a/toolbox/mdcode/tests/tool/commands.test.ts +++ b/toolbox/mdcode/tests/tool/commands.test.ts @@ -2895,8 +2895,9 @@ semantic_model: 'must be a JSON object.'); }); - // Push reads the GOOGLE block before the load under every profile. Beside a - // third-party block that fails the load, the runtime names the same problem. + // A plain push of a model that lists no deployments reads the GOOGLE block + // before the load. Beside a third-party block that fails the load, the + // runtime names the same problem. test('a GOOGLE block that does not load stops the runtime with the error push gives', async () => { writeModel('{'); const file = path.join(dir, 'catalog', 'EntryGroups', 'commerce_eg', 'commerce.yaml'); @@ -2916,9 +2917,9 @@ semantic_model: expect(messages(errorSpy)).toBe(`Error: Model document 'commerce': ${error}`); }); - // The runtime reads every model's profiles before it applies one, as push - // does, so a later model's GOOGLE block is reported under a named profile - // too, whichever model comes first. + // The runtime reads every model's profiles before it applies one, as + // `kcmd push --profile ` does, so a later model's GOOGLE block is + // reported under a named profile too, whichever model comes first. test("the runtime reads every model's GOOGLE block before it applies a profile", async () => { for (const bad of ['alpha', 'beta']) { writeModel('{}'); @@ -3035,7 +3036,14 @@ semantic_model: "'commerce.yaml', which is a 0.2.0.dev0 document"; await expect(push({validateOnly: true, profile: 'prod'})).rejects.toThrow(beside); - await expect(profiles()).rejects.toThrow(beside); + // A plain push loads the model file first, and so does the report. + const errorSpy = spyOn(console, 'error').mockImplementation(() => {}); + expect(await push({validateOnly: true})).toBe(1); + expect(messages(errorSpy)).toContain(problem); + expect(messages(errorSpy)).not.toContain(beside); + errorSpy.mockClear(); + expect(await profiles()).toBe(1); + expect(messages(errorSpy)).toContain(problem); }); } diff --git a/toolbox/mdcode/tests/tool/profiles.test.ts b/toolbox/mdcode/tests/tool/profiles.test.ts index 88dc8666..1b326069 100644 --- a/toolbox/mdcode/tests/tool/profiles.test.ts +++ b/toolbox/mdcode/tests/tool/profiles.test.ts @@ -987,14 +987,19 @@ semantic_model: 'commerce.yaml': PARTIES, 'commerce.profile.prod.yaml': partiesProfile(true), }); - expect(await pushResult('prod')).toEqual({code: 0, errors: []}); + for (const binding of [undefined, 'prod']) { + expect({binding, ...await pushResult(binding)}) + .toEqual({binding, code: 0, errors: []}); + } expect(await profiles()).toBe(0); expect(profileSection('prod')).toContain(' customer -> bigquery.googleapis.com/projects/acme/datasets/prod/tables/customer'); expect(errs).toEqual([]); writeScope({'commerce.profile.prod.yaml': partiesProfile(false)}); - const {code, errors} = await pushResult('prod'); - expect(code).toBe(1); - expect(errors.join('\n')).toContain(NOTES_INHERITED); + for (const binding of [undefined, 'prod']) { + const {code, errors} = await pushResult(binding); + expect({binding, code}).toEqual({binding, code: 1}); + expect(errors.join('\n')).toContain(NOTES_INHERITED); + } }); // The text merged from a legacy wrapper cannot say what the wrapper @@ -1030,7 +1035,8 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; } }); - // Each scope makes `kcmd push --profile prod` fail. A new check push gains + // Each scope makes a push fail: a plain push when the scope has a sibling + // profile file, `kcmd push --profile prod` otherwise. A new check push gains // needs only a new row here. const rejected: Array<[string, Record]> = [ [ @@ -1093,13 +1099,11 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; 'another model in the scope does not declare the profile', { 'commerce.yaml': commerce(), - 'commerce.profile.prod.yaml': profileFile('prod'), + 'commerce.profiles/prod.yaml': legacyProfile(false), 'shop.yaml': commerce().replace('name: commerce', 'name: shop'), }, ], ]; - // A plain push of a model with a sibling profile file runs in two phases, - // and the report gives its errors instead. for (const [label, files] of rejected) { test(`prints only push's errors when ${label}`, async () => { writeScope(files); @@ -1108,7 +1112,15 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; expect(pushed.code).toBe(1); expect(pushed.errors).not.toEqual([]); expect(await profiles()).toBe(1); - expect(allRejects()).toEqual(pushed.errors); + // A second model adds push's one-model error above the models. + expect(allRejects()).toEqual([ + ...(files['shop.yaml'] ? + [expect.stringContaining('would receive 2 models')] : []), + ...pushed.errors, + ]); + if (!twoPhase) { + expect(rejects(profileSection('prod'))).toEqual(pushed.errors); + } }); } @@ -1182,13 +1194,16 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; 'name: b\nrelationships:\n - name: placed_by\n to_columns: [c_email]\n', }); const {errors} = await pushResult(); - const own = (p: string) => - errors.filter(e => e.startsWith(`[commerce.profile.${p}.yaml]`)); - expect(own('a')).not.toEqual([]); - expect(own('b')).not.toEqual([]); + const [a, b] = ['a', 'b'].map(p => errors.filter( + e => e.startsWith(`[commerce.profile.${p}.yaml]`))); + expect(a).not.toEqual([]); + expect(b).not.toEqual([]); expect(await profiles()).toBe(1); - expect(rejects(profileSection('a'))).toEqual(own('a')); - expect(rejects(profileSection('b'))).toEqual(own('b')); + expect(rejects(profileSection('a'))).toEqual(a); + expect(rejects(profileSection('b'))).toEqual(b); + expect(rejects(logs, ' ')) + .toEqual(errors.filter(e => !a.includes(e) && !b.includes(e))); + expect(allRejects().sort()).toEqual([...errors].sort()); }); // Push generates each graph's DDL offline, and a generator can fail. Its @@ -1307,12 +1322,6 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; ' - name: commerce\n', ` - name: commerce\n custom_extensions:\n - {vendor_name: DBT, ` + `data: ${'x'.repeat(130 * 1024)}}\n`); - // Customer's c_id field and 400 more, each reading its own column. A - // profile file restates them all, so each profile adds 401 expressions to - // customer's aspect. - const WIDE = `fields: [{ name: c_id, expression: c_id }, ${ - Array.from({length: 400}, (_, i) => `{ name: f${i}, expression: col_${i} }`) - .join(', ')}]`; // Push gives a scope with two models its one-model error, once, and checks // neither the copies nor the aspects. for (const [label, model, v2] of [ @@ -1386,44 +1395,60 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; }); } + // Customer's c_id and 200 fields that each read a column with a long name. + // Each file that binds them all adds about 48 KB to customer's aspect. + const WIDE = `fields: [{ name: c_id, expression: c_id }, ${ + Array.from({length: 200}, (_, i) => `{ name: f${i}, expression: c${ + 'x'.repeat(200)}${i} }`).join(', ')}]`; + // The model file and a profile file for each of `names`, each binding WIDE. + const wideScope = (...names: string[]) => ({ + 'commerce.yaml': commerce().replace( + 'fields:\n - { name: c_id, expression: c_id }', WIDE), + ...Object.fromEntries(names.map(p => [ + `commerce.profile.${p}.yaml`, + profileFile(p).replace(/fields: \[\{ name: c_id.*/, WIDE), + ])), + }); // With KC_V2_ASPECTS on, push reads the overview sidecars and checks every // aspect the catalog leg writes, before any graph deploys. A malformed // sidecar is reported once, for the scope. For a push in two phases, an // aspect too large belongs to the model. - for (const [label, files, error, profile, indent = ' '] of [ + for (const [label, files, error, indent = ' '] of [ ['an aspect over the size limit in a graph push', {'commerce.yaml': bigBlock(commerce({targets: 1}))}, - 'too large to store in Knowledge Catalog', undefined, ' '], + 'too large to store in Knowledge Catalog'], ['an aspect over the size limit beside a profile file', { 'commerce.yaml': bigBlock(commerce()), 'commerce.profile.prod.yaml': profileFile('prod'), - }, 'too large to store in Knowledge Catalog', undefined, ' '], - ['an aspect only six profile files together make too large', { - 'commerce.yaml': commerce().replace( - 'fields:\n - { name: c_id, expression: c_id }', WIDE), - ...Object.fromEntries(['a', 'b', 'c', 'd', 'e', 'f'].map(p => [ - `commerce.profile.${p}.yaml`, - profileFile(p).replace(/fields: \[\{ name: c_id.*/, WIDE), - ])), - }, 'too large to store in Knowledge Catalog', undefined, ' '], + }, 'too large to store in Knowledge Catalog'], + ['an aspect only two profile files together make too large', + wideScope('a', 'b'), 'too large to store in Knowledge Catalog'], ['a malformed overview sidecar', { 'commerce.yaml': commerce(), 'commerce.overview.md': '---\ncontentType: markdown\n---\nOrders and customers.\n', - }, "not 'markdown'", undefined, ''], - ] as [string, Record, string, string?, string?][]) { + }, "not 'markdown'", ''], + ] as [string, Record, string, string?][]) { test(`prints push's error for ${label} with KC_V2_ASPECTS on`, async () => { process.env.KC_V2_ASPECTS = '1'; writeScope(files); - const pushed = await pushResult(profile); + const pushed = await pushResult(); expect(pushed).toEqual( {code: 1, errors: [expect.stringContaining(error)]}); expect(await profiles()).toBe(1); - expect(rejects(profile ? profileSection(profile) : logs, indent)) - .toEqual(pushed.errors); + expect(rejects(logs, indent)).toEqual(pushed.errors); }); } + test('exits 0 for an aspect one profile file keeps under the size limit ' + + 'with KC_V2_ASPECTS on', + async () => { + process.env.KC_V2_ASPECTS = '1'; + writeScope(wideScope('a')); + expect(await pushResult()).toEqual({code: 0, errors: []}); + expect(await profiles()).toBe(0); + }); + test('prints push\'s warnings for a custom ai_context member push leaves ' + 'out and a sidecar that names no entry, with KC_V2_ASPECTS on', async () => { @@ -1488,10 +1513,10 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; .toHaveLength(1); }); - // A graph push reads the profile files before anything else, so a profile - // file that breaks a layout rule throws before anything prints. + // When push stops at a profile file that breaks a layout rule, the report + // throws push's message before anything prints. for (const v2Aspects of [false, true]) { - test(`throws as push does for a profile file that breaks a layout rule${ + test(`throws push's error for a profile file that breaks a layout rule${ v2Aspects ? ' with KC_V2_ASPECTS on' : ''}`, async () => { if (v2Aspects) process.env.KC_V2_ASPECTS = '1'; @@ -1499,7 +1524,7 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; 'commerce.yaml': commerce(), 'commerce.profile.prod.yaml': profileFile('other'), }); - const pushed = await pushResult('prod'); + const pushed = await pushResult(); expect(pushed).toEqual({ code: 1, errors: [expect.stringContaining("declares name 'other'")], @@ -1726,7 +1751,7 @@ ${exclude ? ' fields_exclude: [notes]\n' : ''}`; // The model file's join columns do not stand in for a side a named // profile leaves out. A profile that states one side gets the // completeness error from a push of any binding, from `kcmd profiles` - // under each binding, and from the runtime. + // under the profile, and from the runtime. test('when a profile states one side of a join', async () => { const oneSide = (columns: string) => profileFile('staging').replace( /relationships:[\s\S]*/, @@ -2061,9 +2086,8 @@ semantic_model: fs.rmSync(path.join(dir, 'catalog'), {recursive: true, force: true}); writeScope(files); // A sibling profile file makes a plain push run in two phases. - await (Object.keys(files).some(f => f.includes('.profile.')) ? - expectReport(pushes, label) : - expectPushes(pushes, label)); + const twoPhase = Object.keys(files).some(f => f.includes('.profile.')); + await (twoPhase ? expectReport : expectPushes)(pushes, label); } }); @@ -3030,9 +3054,9 @@ semantic_model: }); // Push warns and still pushes when the model file's own bindings are - // incomplete and the model declares no deployment. `kcmd profiles` prints - // the warning under the default binding, unless it reports another profile. - test('warns about incomplete inline bindings when the model declares no deployment, on push and in the report', async () => { + // incomplete and no deployment uses them. `kcmd profiles` prints the + // warning under the default binding, unless it reports another profile. + test('warns about incomplete inline bindings that no deployment uses, on push and in the report', async () => { const warned: string[] = []; spyOn(console, 'warn').mockImplementation((m: any) => { warned.push(String(m)); @@ -3069,15 +3093,15 @@ semantic_model: expect(await profiles({profile: 'prod'})).toBe(0); expect(newWarnings()).toEqual([]); - // A deployment that names a profile gets no warning, in push or in the - // report. + // Push and the report still warn when every deployment names a profile. writeScope({ 'commerce.yaml': customerUnbound(commerce({targets: 1})) .replace(/(target: \S+)\n/, '$1\n profile: prod\n'), }); expect(await pushResult()).toEqual({code: 0, errors: []}); + expect(newWarnings()).toEqual([`Warning: ${NO_DEFAULT}`]); expect(await profiles()).toBe(0); - expect(newWarnings()).toEqual([]); + expect(newWarnings()).toEqual(reported(NO_DEFAULT)); }); // A named profile that keeps the field unchanged gets no error of its @@ -3132,15 +3156,10 @@ semantic_model: } // The report gives the model file's error under its own bindings. expect(await profiles()).toBe(1); - expect({ - label, - prod: rejects(profileSection('prod')), - own: rejects(section(INLINE)), - }).toEqual({ - label, - prod: errors.filter(e => e !== hinted), - own: errors.filter(e => e === hinted), - }); + const inline = errors.filter(e => e === hinted); + expect({label, inline: rejects(section(INLINE)), + prod: rejects(profileSection('prod'))}) + .toEqual({label, inline, prod: errors.filter(e => e !== hinted)}); } }); }); @@ -3347,8 +3366,7 @@ semantic_model: }); // A model file whose own source is a Cloud SQL table is read in that table's - // dialect too: its ANSI_SQL metric is parsed as T-SQL, so its bracketed - // names resolve the metric's entity, which a BigQuery graph push needs. + // dialect too, so a field with only a BigQuery text fails the report. test('the inline bindings are read in the dialect its catalog entry gives', async () => { fs.writeFileSync(path.join(dir, 'catalog.yaml'), catalogYaml(undefined)); const eg = path.join(dir, 'catalog', 'EntryGroups', 'commerce_eg'); @@ -3364,16 +3382,12 @@ semantic_model: primary_key: [key] fields: - { name: key, expression: o_key } - - { name: amount, expression: {dialects: [{dialect: ANSI_SQL, expression: o_amount}]} } - metrics: - - name: revenue - expression: - dialects: - - { dialect: ANSI_SQL, expression: "SUM([Order].[amount])" } + - { name: amount, expression: {dialects: [{dialect: BIGQUERY, expression: o_amount}]} } `); stubCloudSqlEntry(200, 'cloudsql-sqlserver-table'); - await profiles({}); - expect(logs.join('\n')).not.toContain("metric 'revenue'"); + expect(await profiles({})).toBe(1); + expect(logs.join('\n')).toContain( + "the model file binds entity 'Order' to a SQLSERVER source"); }); test('stops kcmd profiles and push when its entry cannot be read', async () => { diff --git a/toolbox/mdcode/tests/tool/store_agreement.test.ts b/toolbox/mdcode/tests/tool/store_agreement.test.ts index c3d30ec2..25f5893b 100644 --- a/toolbox/mdcode/tests/tool/store_agreement.test.ts +++ b/toolbox/mdcode/tests/tool/store_agreement.test.ts @@ -8,10 +8,11 @@ // deployment that names no profile in a model with metrics, which the runtime // accepts. The `kcmd profiles` report gives the errors a plain push gives, so // it accepts `--profile` on a model that lists deployments, which push -// refuses. Push also accepts a Spanner model that binds a table outside its -// database today. The runtime refuses it on purpose: a write would land in -// the target database's table of that name, so --print-store gives no store -// and skills-generate fails for a model with actions. +// refuses. Push refuses a deployment that binds a table outside the database +// it deploys to. The runtime checks the tables against the one store it +// picks, so it accepts a model whose other deployment binds such a table. +// skills-generate also accepts a model whose picked deployment binds one, +// unless the model has actions and that deployment is Spanner or AlloyDB. // // Of push's checks on the model itself, the runtime runs only the one on // inheritance. A model that push refuses for an entity with no source, a @@ -595,7 +596,7 @@ semantic_model: expect(r.report.out).toContain(msg.spannerMetrics('prod')); }); - test('Spanner beside BigQuery: push\'s local checks pass, the runtime takes Spanner', + test('Spanner beside BigQuery: push refuses BigQuery over a Spanner table, the runtime takes Spanner', async () => { writeScope({'commerce.yaml': `version: "0.2.0.dev0/google" semantic_model: @@ -613,7 +614,10 @@ semantic_model: - { name: key, expression: OrderId } `}); const r = await runAll(); - expect(codes(r)).toEqual({push: 0, report: 0, printStore: 0, skills: 0}); + expect(codes(r)).toEqual({push: 1, report: 1, printStore: 0, skills: 0}); + expect(r.push.out).toContain( + "deployment 'bq' targets a BigQuery graph, but the model file " + + "binds entity 'Order'"); expect(r.printStore.out).toBe('acme-ops/prod/commerce'); }); @@ -1152,6 +1156,9 @@ semantic_model: describe('a vanilla model with GOOGLE blocks agrees with the Google flavor', () => { type Deployment = {name: string; target: string; profile?: string}; + // Profile prod, bound to the database deployment prod deploys to. + const prodProfile = PROD_PROFILE.replace(SPANNER_DB, `${SPANNER_DB}2`); + function googleScope(deployments: Deployment[], extendsList?: string[]): Record { const lines = deployments.map( @@ -1170,7 +1177,7 @@ ${extendsList ? ` extends: [${extendsList.join(', ')}]\n` : ''}\ fields: - { name: key, expression: OrderId } `, - 'commerce.profile.prod.yaml': PROD_PROFILE, + 'commerce.profile.prod.yaml': prodProfile, }; } @@ -1178,7 +1185,7 @@ ${extendsList ? ` extends: [${extendsList.join(', ')}]\n` : ''}\ function vanillaScope(deployments: Deployment[], extendsList?: string[]): Record { const model = JSON.stringify({ - profiles: [yaml.parse(PROD_PROFILE)], + profiles: [yaml.parse(prodProfile)], deployments, }); const dataset = extendsList ? @@ -1239,14 +1246,14 @@ ${dataset} fields: test('a deployment for a declared profile, under it: push takes no --profile, the runtime uses it', async () => { const {google, vanilla} = await bothFlavors( - [prod(`${SPANNER_DB}/graphs/commerce`)], 'prod'); + [prod(`${SPANNER_DB}2/graphs/commerce`)], 'prod'); expect(codes(google)) .toEqual({push: 1, report: 0, printStore: 0, skills: 0}); expect(google.push.out).toContain(msg.flag.profile); - expect(google.printStore.out).toBe('acme-ops/prod/commerce'); + expect(google.printStore.out).toBe('acme-ops/prod/commerce2'); expect(codes(vanilla)).toEqual(codes(google)); expect(vanilla.printStore.out.split('\n').pop()) - .toBe('acme-ops/prod/commerce'); + .toBe('acme-ops/prod/commerce2'); }); test('a deployment for an undeclared profile: every runtime command refuses', diff --git a/toolbox/mdcode/tests/tool/two_phase/deploy.test.ts b/toolbox/mdcode/tests/tool/two_phase/deploy.test.ts index a0c708aa..fa2caaef 100644 --- a/toolbox/mdcode/tests/tool/two_phase/deploy.test.ts +++ b/toolbox/mdcode/tests/tool/two_phase/deploy.test.ts @@ -5,7 +5,7 @@ import {expect} from 'bun:test'; -import {ALLOYDB, at, boundAt, boundTo, bqGraph, bqTable, Case, CUSTOMER, deploymentNames, err, has, kcResult, modelFile, msg, PREPARED, profileFile, PushRun, REAL, refused, scenarios, Scope, SNOWFLAKE_METRIC, SPANNER_CUSTOMER, spannerGraph, TRANSPILE_WARNING, when} from './harness'; +import {ALLOYDB, at, boundAt, boundTo, bqGraph, bqTable, Case, CUSTOMER, deploymentNames, err, EXPRESSION_KEY, has, kcResult, modelFile, msg, PREPARED, profileFile, PushRun, REAL, refused, scenarios, Scope, SNOWFLAKE_METRIC, SPANNER_CUSTOMER, spannerGraph, TRANSPILE_WARNING, when} from './harness'; const header = (d: string) => `\n-- Deployment '${d}' --`; @@ -175,6 +175,11 @@ const WITH_SCORE = { ...CUSTOMER, fields: [...CUSTOMER.fields, {name: 'score', expression: 'score'}], }; +// The warning about the SQL profile `p` writes for field score. +const unreadScore = (p: string) => + `Warning: [commerce.profile.${p}.yaml] profile '${p}': kcmd could not ` + + "read the SQL of field 'Customer.score', so the fields it reads were not " + + 'checked'; // Binds field score to `expression`, written in `dialect`. const scoreSql = (name: string, dialect: string, expression: string) => ({ entities: { @@ -382,12 +387,13 @@ scenarios('how push deploys each deployment in order, and what it prints', [ }, code: 1, errors: [has("action 'refund'")], - warnings: [], + warnings: [has("unbound 'default' profile")], nothingWritten: true, then: [{ flags: {deployment: ['g1']}, code: 0, - warnings: ["Note: profile 'prod' leaves 1 field(s) unbound."], + warnings: [has("unbound 'default' profile"), + "Note: profile 'prod' leaves 1 field(s) unbound."], }], }, { @@ -412,7 +418,8 @@ scenarios('how push deploys each deployment in order, and what it prints', [ }, }, { - name: 'each deployment prints its own warning about the SQL its profile sets', + name: 'each profile prints its own warning about the SQL it sets, also ' + + 'when --deployment stops at another profile', deployments: [ {name: 'a', target: bqGraph('ga'), profile: 'pa'}, {name: 'b', target: bqGraph('gb'), profile: 'pb'}, @@ -424,11 +431,17 @@ scenarios('how push deploys each deployment in order, and what it prints', [ }, flags: {validateOnly: true}, warnings: [ + ...['pa', 'pb'].map(unreadScore), has("Warning: [commerce] note: no 'BIGQUERY' dialect"), - ...['a', 'b'].map( - d => "Warning: field 'Customer.score' in model 'commerce' " + - `(commerce, deployment '${d}'): kcmd could not read its SQL, ` + - 'so the fields it reads were not checked.'), ], + then: [{ + profiles: { + pa: scoreSql('pa', 'ANSI_SQL', '((pa'), + pb: {entities: {Customer: EXPRESSION_KEY}}, + }, + flags: {deployment: ['a']}, + code: 1, + warnings: [unreadScore('pa')], + }], }, ], {deployments: ['g1', 'g2'], code: 0}); diff --git a/toolbox/mdcode/tests/tool/two_phase/report.test.ts b/toolbox/mdcode/tests/tool/two_phase/report.test.ts index 4799d8ea..b456043f 100644 --- a/toolbox/mdcode/tests/tool/two_phase/report.test.ts +++ b/toolbox/mdcode/tests/tool/two_phase/report.test.ts @@ -1,4 +1,4 @@ -// Tests of what `kcmd profiles` prints for a scope that a plain push pushes in +// Tests of what `kcmd profiles` prints for a scope whose plain push runs in // two phases (src/tool/commands.ts, profiles). Each case runs // `kcmd push --validate-only` and then the report. @@ -151,9 +151,27 @@ scenarios('kcmd profiles reports what push rejects', [ ['a malformed overview sidecar', {v2Aspects: true, files: { 'commerce.overview.md': '---\ncontentType: markdown\n---\nOrders.\n', }}, "not 'markdown'", {'': [pushed()], 'no profiles': []}], + ['a model file that does not load beside a misnamed profile file', {files: { + 'commerce.yaml': modelFile(MODEL) + .replace(' entities:\n', ' bogus_key: 1\n entities:\n'), + 'commerce.profile.prod.yaml': 'name: staging\n', + }}, 'bogus_key', {'': [pushed()]}], + ['a vanilla model file that does not load beside a GOOGLE block that ' + + 'does not load', {files: { + 'commerce.yaml': modelFile({google: {deployments: [graph('prod')]}}) + .replace(' datasets:\n', " - vendor_name: GOOGLE\n" + + " data: '[]'\n datasets:\n") + .replace(' primary_key:', ' bogus_key: 1\n primary_key:'), + }}, 'bogus_key', {'': [pushed()]}], ['a repeated metrics_exclude name the model lacks', {profiles: {prod: {...PROD, metrics_exclude: ['nope', 'nope']}}}, "metric 'nope' in 'metrics_exclude'", {prod: [pushed()]}], + ['a metric a Spanner deployment\'s profile leaves available', { + deployments: [{name: 'sp', target: spannerGraph('g'), profile: 'prod'}, + graph('qa')], + metrics: [{name: 'n', expression: 'COUNT(customer.c_id)'}], + profiles: {prod: PROD}, + }, "leaves metric(s) 'n' available", {prod: [pushed()]}], ] as Array<[string, Scope, string, ReportStep['rejects']]>).map( ([label, scope, part, rejects]): Case => ({ name: `prints ${label} once`, diff --git a/toolbox/mdcode/tests/tool/two_phase/tables.test.ts b/toolbox/mdcode/tests/tool/two_phase/tables.test.ts new file mode 100644 index 00000000..3f83c8d1 --- /dev/null +++ b/toolbox/mdcode/tests/tool/two_phase/tables.test.ts @@ -0,0 +1,118 @@ +// Tests that push refuses a deployment whose tables are outside the database +// it deploys to, and that it probes each BigQuery table in the location of +// the graph's dataset (src/tool/commands.ts, checkBeforeWrites and +// liveErrors). + +import {expect} from 'bun:test'; + +import {PushOptions} from '../../../src/tool/commands'; + +import {ALLOYDB, boundAt, bqTable, Case, CUSTOMER, err, has, pushed, REAL, Scope, scenarios, Services, SPANNER, SPANNER_CUSTOMER, spannerGraph} from './harness'; + +const OTHER_ALLOYDB = 'alloydb.googleapis.com/projects/test-project/' + + 'locations/us-central1/clusters/c/databases/other/schemas/public/tables/' + + 'customer'; +const UNSELECTED = "[commerce] deployment 'sp' targets Spanner database " + + "'test-project/i/db', but profile 'x' binds entity 'Customer' to " + + `'${SPANNER}2/tables/Customer', which is not a table in that database.`; + +const fails = (name: string, scope: Scope, error: string, + flags: PushOptions = {}): Case => ({ + name, ...scope, flags, code: 1, + errors: [err(error)], called: {preflight: 0}, nothingWritten: true, +}); + +// The graph's dataset d is in us-central1. BigQuery finds a table in dataset +// eu only in europe-west1, and a probe that names no location runs where its +// table is. +const TWO_LOCATIONS: Services = { + getDataset: {status: 200, result: {location: 'us-central1'}}, + sources: REAL.sources, + query: (_project: string, sql: string, location?: string) => { + const where = sql.includes('.eu.') ? 'europe-west1' : 'us-central1'; + return location && location !== where ? + {status: 404, message: `Not found in location ${location}`} : + {status: 200, result: {jobReference: {location: where}}}; + }, +}; + +scenarios('each deployment\'s tables are in the database it deploys to', [ + fails( + 'an AlloyDB deployment fails for a table in another database of its cluster, and nothing is written', { + deployments: [{name: 'd1', target: ALLOYDB, profile: 'x'}], + profiles: {x: boundAt(OTHER_ALLOYDB)}, + }, + "[commerce] deployment 'd1' targets AlloyDB database " + + "'test-project/us-central1/c/db', but profile 'x' binds entity " + + `'Customer' to '${OTHER_ALLOYDB}', which is not a table in that ` + + 'database.'), + fails( + 'a deployment that uses the model file\'s bindings names only the entity outside its database', + {entities: [CUSTOMER, { + ...CUSTOMER, name: 'Orders', source: `${SPANNER}/tables/Orders`, + }]}, + "[commerce] deployment 'g' targets a BigQuery graph, but the model " + + `file binds entity 'Orders' to '${SPANNER}/tables/Orders', which ` + + 'is not a BigQuery or BigLake table.'), + { + ...fails( + 'a deployment the push does not select fails the push the same way, ' + + 'also under --no-deployment', { + deployments: + ['g', {name: 'sp', target: spannerGraph('g'), profile: 'x'}], + profiles: {x: boundAt(`${SPANNER}2/tables/Customer`)}, + }, + UNSELECTED, {deployment: ['g']}), + then: [{flags: {deployment: false}, code: 1, errors: [err(UNSELECTED)], + nothingWritten: true}], + report: {code: 1, rejects: {x: [pushed()]}}, + }, + { + name: 'a profile source in no accepted form gets the profile check\'s error first', + deployments: [{name: 'sp', target: spannerGraph('g'), profile: 'x'}], + profiles: {x: boundAt(`${SPANNER}/table/Customer`)}, + code: 1, errors: [has('is not a valid table reference')], + }, + { + name: 'a model with an abstract entity, which has no source, passes and deploys to Spanner', + deployments: [{name: 'sp', target: spannerGraph('g')}], + entities: [ + {name: 'Party', abstract: true, fields: [{name: 'key'}]}, + {...CUSTOMER, extends: ['Party'], source: SPANNER_CUSTOMER}, + ], + code: 0, + events: ['preflight', 'apply', 'deploy sp'], + }, + { + name: 'a BigQuery table outside the graph dataset\'s location fails, naming its location, and passes with a probe that names no location when the dataset\'s location cannot be read; a table in the dataset\'s location passes', + entities: [{...CUSTOMER, source: bqTable('customer', 'eu')}], + services: TWO_LOCATIONS, + code: 1, + errors: [err( + "[commerce] deployment 'g' targets a BigQuery graph in dataset " + + "'test-project.d', which is in location 'us-central1', but entity " + + "'Customer' reads table 'test-project.eu.customer', which is in " + + "location 'europe-west1'. A property graph can use only tables in " + + "its dataset's location.")], + nothingWritten: true, + check: r => expect(r.fake.query.mock.calls.map(c => c[2] ?? null)) + .toEqual(['us-central1', null]), + then: [{ + services: {getDataset: {status: 403, message: 'Access Denied'}}, + code: 0, + check: r => expect(r.fake.query.mock.calls.map(c => c[2] ?? null)) + .toEqual([null, null]), + }, { + entities: [CUSTOMER], + code: 0, + events: ['preflight', 'apply', 'deploy g'], + check: r => expect(r.fake.query.mock.calls + .filter(c => String(c[1]).startsWith('SELECT 1')) + .map(c => c.slice(1, 4))) + .toEqual([[ + 'SELECT 1 FROM `test-project.d.customer` WHERE FALSE', + 'us-central1', true, + ]]), + }], + }, +], {deployments: ['g']}); diff --git a/toolbox/mdcode/tests/tool/two_phase/two_phase_path.test.ts b/toolbox/mdcode/tests/tool/two_phase/two_phase_path.test.ts index 9282e5c7..7847151d 100644 --- a/toolbox/mdcode/tests/tool/two_phase/two_phase_path.test.ts +++ b/toolbox/mdcode/tests/tool/two_phase/two_phase_path.test.ts @@ -162,7 +162,7 @@ scenarios('the two-phase push path', [ ], }, { - name: 'a Spanner deployment that names no profile is refused when the model has metrics and deploys when it has none, and one under a profile whose metrics_exclude is "*" deploys', + name: 'a Spanner deployment that names no profile is refused when the model has metrics, after the table check, and deploys when it has none, and one under a profile whose metrics_exclude is "*" deploys', deployments: [{name: 'sp', target: spannerGraph('g')}], entities: [{...CUSTOMER, source: SPANNER_CUSTOMER}], metrics: [{ @@ -175,6 +175,11 @@ scenarios('the two-phase push path', [ called: {preflight: 0}, nothingWritten: true, then: [{ + entities: [CUSTOMER, {...CUSTOMER, name: 'Orders'}], + code: 1, + errors: ['Customer', 'Orders'].map( + e => has(`but the model file binds entity '${e}'`)), + }, { metrics: undefined, code: 0, events: ['preflight', 'apply', 'deploy sp'],