From 38144b1620b5d370daa55d7f39ddedfe1a966fdb Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Wed, 5 Aug 2026 16:50:42 -0700 Subject: [PATCH 1/5] refactor(ci): reconstruct PPL grammar compatibility check Signed-off-by: Hanyu Wei --- .../ppl-lint-grammar-compatibility.yml | 389 ++++++++++++++ docs/dev/index.md | 3 +- docs/dev/ppl-lint-grammar-compatibility-ci.md | 166 ++++++ scripts/ppl-lint-rule-validation.sh | 493 ++++++++++++++++++ scripts/ppl-lint/README.md | 213 ++++++++ .../__tests__/validate-osd-grammar.test.mjs | 453 ++++++++++++++++ .../__tests__/workflow-pairing.test.mjs | 395 ++++++++++++++ scripts/ppl-lint/grammar-cases.json | 155 ++++++ scripts/ppl-lint/validate-osd-grammar.mjs | 463 ++++++++++++++++ 9 files changed, 2729 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ppl-lint-grammar-compatibility.yml create mode 100644 docs/dev/ppl-lint-grammar-compatibility-ci.md create mode 100755 scripts/ppl-lint-rule-validation.sh create mode 100644 scripts/ppl-lint/README.md create mode 100644 scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs create mode 100644 scripts/ppl-lint/__tests__/workflow-pairing.test.mjs create mode 100644 scripts/ppl-lint/grammar-cases.json create mode 100644 scripts/ppl-lint/validate-osd-grammar.mjs diff --git a/.github/workflows/ppl-lint-grammar-compatibility.yml b/.github/workflows/ppl-lint-grammar-compatibility.yml new file mode 100644 index 00000000000..401c7edfe45 --- /dev/null +++ b/.github/workflows/ppl-lint-grammar-compatibility.yml @@ -0,0 +1,389 @@ +name: "[Linter] PPL grammar compatibility" + +on: + pull_request: + branches: + - main + - '[0-9]+.[0-9]+' + paths: + - build.gradle + - settings.gradle + - ppl/build.gradle + - ppl/src/main/antlr/OpenSearchPPLLexer.g4 + - ppl/src/main/antlr/OpenSearchPPLParser.g4 + - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/GrammarBundle.java + - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleBuilder.java + - plugin/build.gradle + - plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java + - plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLGrammarAction.java + - scripts/ppl-lint/** + - scripts/ppl-lint-rule-validation.sh + - .github/workflows/ppl-lint-grammar-compatibility.yml + - docs/dev/ppl-lint-grammar-compatibility-ci*.md + push: + branches: + - main + - '[0-9]+.[0-9]+' + paths: + - build.gradle + - settings.gradle + - ppl/build.gradle + - ppl/src/main/antlr/OpenSearchPPLLexer.g4 + - ppl/src/main/antlr/OpenSearchPPLParser.g4 + - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/GrammarBundle.java + - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleBuilder.java + - plugin/build.gradle + - plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java + - plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLGrammarAction.java + - scripts/ppl-lint/** + - scripts/ppl-lint-rule-validation.sh + - .github/workflows/ppl-lint-grammar-compatibility.yml + - docs/dev/ppl-lint-grammar-compatibility-ci*.md + workflow_dispatch: + inputs: + osd_repo: + description: Optional OpenSearch Dashboards repository override. + required: false + type: string + osd_ref: + description: Optional OpenSearch Dashboards ref override. + required: false + type: string + allow_release_line_mismatch: + description: Development-only bypass for exact release-line validation. + required: false + default: false + type: boolean + +permissions: + contents: read + +concurrency: + group: ppl-lint-grammar-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + validate: + name: PPL grammar compatibility + runs-on: ubuntu-latest + timeout-minutes: 60 + permissions: + contents: read + + steps: + - name: Checkout SQL + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false + + - name: Resolve SQL target and OSD pair + id: target + env: + EVENT_NAME: ${{ github.event_name }} + TARGET_BRANCH: ${{ github.base_ref || github.ref_name }} + SQL_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + REQUESTED_OSD_REPO: ${{ inputs.osd_repo }} + REQUESTED_OSD_REF: ${{ inputs.osd_ref }} + REQUESTED_BYPASS: ${{ inputs.allow_release_line_mismatch }} + run: | + set -euo pipefail + sql_raw=$(sed -nE 's/.*opensearch_version = System\.getProperty\("opensearch\.version", "([^"]+)"\).*/\1/p' build.gradle) + [[ "$sql_raw" =~ ^([0-9]+)\.([0-9]+)\.([0-9]+)([-+][0-9A-Za-z.-]+)?$ ]] || + { echo "::error::Invalid SQL product version: $sql_raw"; exit 1; } + sql_version="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}.${BASH_REMATCH[3]}" + sql_line="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}" + bypass=$([[ "$EVENT_NAME" == workflow_dispatch && "$REQUESTED_BYPASS" == true ]] && echo true || echo false) + case "$TARGET_BRANCH" in + main) canonical_ref=main ;; + *) + [[ "$TARGET_BRANCH" =~ ^[0-9]+\.[0-9]+$ ]] || + { echo "::error::Target branch must be main or an exact X.Y release branch"; exit 1; } + [[ "$TARGET_BRANCH" == "$sql_line" || "$bypass" == true ]] || + { echo "::error::SQL $sql_raw does not match target branch $TARGET_BRANCH"; exit 1; } + canonical_ref="$TARGET_BRANCH" + ;; + esac + canonical_repo=opensearch-project/OpenSearch-Dashboards + osd_repo="$canonical_repo" + osd_ref="$canonical_ref" + if [[ "$EVENT_NAME" == workflow_dispatch ]]; then + osd_repo="${REQUESTED_OSD_REPO:-$osd_repo}" + osd_ref="${REQUESTED_OSD_REF:-$osd_ref}" + elif [[ -n "$REQUESTED_OSD_REPO$REQUESTED_OSD_REF" || "$REQUESTED_BYPASS" == true ]]; then + echo "::error::OSD overrides are allowed only for workflow_dispatch" + exit 1 + fi + [[ "$osd_repo$osd_ref" != *$'\n'* && "$osd_repo$osd_ref" != *$'\r'* ]] || + { echo "::error::OSD overrides must be single-line values"; exit 1; } + sql_sha=$(git rev-parse HEAD) + override=$([[ "$osd_repo" != "$canonical_repo" || "$osd_ref" != "$canonical_ref" ]] && echo true || echo false) + { + echo "target_branch=$TARGET_BRANCH" + echo "sql_version_raw=$sql_raw" + echo "sql_version=$sql_version" + echo "sql_release_line=$sql_line" + echo "sql_sha=$sql_sha" + echo "sql_head_sha=$SQL_HEAD_SHA" + echo "osd_repo=$osd_repo" + echo "osd_ref=$osd_ref" + echo "osd_override=$override" + echo "release_line_bypass=$bypass" + } >> "$GITHUB_OUTPUT" + + - name: Checkout paired OpenSearch Dashboards + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + repository: ${{ steps.target.outputs.osd_repo }} + ref: ${{ steps.target.outputs.osd_ref }} + path: .ci/OpenSearch-Dashboards + persist-credentials: false + + - name: Record immutable OSD revision + id: osd + run: | + set -euo pipefail + sha=$(git -C .ci/OpenSearch-Dashboards rev-parse HEAD) + printf '%s\n' "$sha" > osd-revision.txt + echo "sha=$sha" >> "$GITHUB_OUTPUT" + echo "OpenSearch Dashboards revision: $sha" + + - name: Set up Node from OSD .nvmrc + uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 + with: + node-version-file: .ci/OpenSearch-Dashboards/.nvmrc + + - name: Pin OSD Yarn and validate checked-out versions + id: versions + working-directory: .ci/OpenSearch-Dashboards + env: + TARGET_BRANCH: ${{ steps.target.outputs.target_branch }} + SQL_VERSION_RAW: ${{ steps.target.outputs.sql_version_raw }} + SQL_VERSION: ${{ steps.target.outputs.sql_version }} + SQL_RELEASE_LINE: ${{ steps.target.outputs.sql_release_line }} + SQL_SHA: ${{ steps.target.outputs.sql_sha }} + SQL_HEAD_SHA: ${{ steps.target.outputs.sql_head_sha }} + OSD_REPOSITORY: ${{ steps.target.outputs.osd_repo }} + OSD_REF: ${{ steps.target.outputs.osd_ref }} + OSD_SHA: ${{ steps.osd.outputs.sha }} + OSD_OVERRIDE: ${{ steps.target.outputs.osd_override }} + RELEASE_LINE_BYPASS: ${{ steps.target.outputs.release_line_bypass }} + run: | + set -euo pipefail + yarn_version=$(node -e "process.stdout.write(require('./package.json').engines.yarn.match(/[0-9]+\.[0-9]+\.[0-9]+/)[0])") + npm install --global "yarn@$yarn_version" + osd_raw=$(yarn --silent pkg-version) + [[ "$osd_raw" =~ ^([0-9]+)\.([0-9]+)\.([0-9]+)([-+][0-9A-Za-z.-]+)?$ ]] || + { echo "::error::Invalid OSD product version: $osd_raw"; exit 1; } + osd_version="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}.${BASH_REMATCH[3]}" + osd_line="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}" + if [[ "$TARGET_BRANCH" != main && "$RELEASE_LINE_BYPASS" != true ]]; then + [[ "$SQL_RELEASE_LINE" == "$TARGET_BRANCH" && "$osd_line" == "$TARGET_BRANCH" ]] || + { echo "::error::Expected SQL and OSD versions on release line $TARGET_BRANCH"; exit 1; } + fi + jq -n \ + --arg target "$TARGET_BRANCH" \ + --arg sqlRaw "$SQL_VERSION_RAW" --arg sqlVersion "$SQL_VERSION" \ + --arg sqlLine "$SQL_RELEASE_LINE" --arg sqlSha "$SQL_SHA" --arg headSha "$SQL_HEAD_SHA" \ + --arg osdRepo "$OSD_REPOSITORY" --arg osdRef "$OSD_REF" --arg osdSha "$OSD_SHA" \ + --arg osdRaw "$osd_raw" --arg osdVersion "$osd_version" --arg osdLine "$osd_line" \ + --arg override "$OSD_OVERRIDE" --arg bypass "$RELEASE_LINE_BYPASS" \ + '{ + schemaVersion: 1, + sql: { + sha: $sqlSha, headSha: $headSha, targetBranch: $target, + versionRaw: $sqlRaw, version: $sqlVersion, releaseLine: $sqlLine + }, + osd: { + repository: $osdRepo, ref: $osdRef, sha: $osdSha, + versionRaw: $osdRaw, version: $osdVersion, releaseLine: $osdLine, + override: ($override == "true") + }, + releaseLineValidationBypassed: ($bypass == "true") + }' > "$GITHUB_WORKSPACE/resolved-target.json" + echo "osd_version=$osd_version" >> "$GITHUB_OUTPUT" + echo "osd_release_line=$osd_line" >> "$GITHUB_OUTPUT" + + - name: Detect headless grammar capability + id: capability + run: | + set -euo pipefail + module=.ci/OpenSearch-Dashboards/src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint + if [[ -f "${module}.ts" || -f "${module}.js" ]]; then + echo "available=true" >> "$GITHUB_OUTPUT" + else + echo "available=false" >> "$GITHUB_OUTPUT" + fi + + - name: Record unsupported paired branch with adapter + if: ${{ steps.capability.outputs.available == 'false' }} + run: | + set -euo pipefail + node "$GITHUB_WORKSPACE/scripts/ppl-lint/validate-osd-grammar.mjs" \ + --grammar "$GITHUB_WORKSPACE/ppl-grammar-bundle.json" \ + --cases "$GITHUB_WORKSPACE/scripts/ppl-lint/grammar-cases.json" \ + --target "$GITHUB_WORKSPACE/resolved-target.json" \ + --osd-root "$GITHUB_WORKSPACE/.ci/OpenSearch-Dashboards" \ + --osd-sha "${{ steps.osd.outputs.sha }}" \ + --report "$GITHUB_WORKSPACE/ppl-lint-grammar-compatibility-report.json" \ + --summary "$GITHUB_STEP_SUMMARY" + + - name: Set up JDK 21 + if: ${{ steps.capability.outputs.available == 'true' }} + uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4 + with: + distribution: temurin + java-version: 21 + cache: gradle + + - name: Capture candidate runtime grammar + if: ${{ steps.capability.outputs.available == 'true' }} + timeout-minutes: 25 + run: | + set -euo pipefail + ./gradlew :opensearch-sql-plugin:run --no-daemon > ppl-grammar-cluster.log 2>&1 & + gradle_pid=$! + cleanup() { + kill "$gradle_pid" 2>/dev/null || true + wait "$gradle_pid" 2>/dev/null || true + } + trap cleanup EXIT + ready=false + for attempt in $(seq 1 120); do + if curl --silent --fail http://127.0.0.1:9200/_cluster/health > /dev/null; then + ready=true + break + fi + kill -0 "$gradle_pid" 2>/dev/null || + { echo "::error::Gradle run task stopped before cluster startup"; exit 1; } + echo "Waiting for candidate cluster (${attempt}/120)" + sleep 5 + done + [[ "$ready" == true ]] || { echo "::error::Candidate cluster did not become ready"; exit 1; } + code=$(curl --silent --show-error --output ppl-grammar-bundle.json --write-out '%{http_code}' \ + http://127.0.0.1:9200/_plugins/_ppl/_grammar) + [[ "$code" == 200 ]] || { echo "::error::Grammar endpoint returned HTTP $code"; exit 1; } + jq -e ' + type == "object" + and (.bundleVersion | type == "string" and length > 0) + and (.antlrVersion | type == "string" and length > 0) + and (.grammarHash | type == "string" and test("^sha256:[0-9a-f]{64}$")) + and (.lexerSerializedATN | type == "array" and length > 0) + and (.lexerRuleNames | type == "array" and length > 0) + and (.channelNames | type == "array" and length > 0) + and (.modeNames | type == "array" and length > 0) + and (.parserSerializedATN | type == "array" and length > 0) + and (.parserRuleNames | type == "array" and length > 0) + and (.startRuleIndex | type == "number") + and (.literalNames | type == "array" and length > 0 and any(.[]; . == null)) + and (.symbolicNames | type == "array" and length > 0 and any(.[]; . == null)) + and (.tokenDictionary | type == "object" and length > 0) + and (.ignoredTokens | type == "array") + and (.rulesToVisit | type == "array" and length > 0) + ' ppl-grammar-bundle.json > /dev/null + + - name: Cache OSD Yarn dependencies + if: ${{ steps.capability.outputs.available == 'true' }} + uses: actions/cache@0c907a75c2c80ebcb7f088228285e798b750cf8f # v4 + with: + path: ~/.cache/yarn + key: ${{ runner.os }}-osd-yarn-${{ hashFiles('.ci/OpenSearch-Dashboards/yarn.lock') }} + restore-keys: ${{ runner.os }}-osd-yarn- + + - name: Bootstrap OpenSearch Dashboards + if: ${{ steps.capability.outputs.available == 'true' }} + working-directory: .ci/OpenSearch-Dashboards + run: yarn osd bootstrap --prefer-offline + + - name: Validate OSD linter against candidate grammar + id: compatibility + if: ${{ steps.capability.outputs.available == 'true' }} + working-directory: .ci/OpenSearch-Dashboards + run: | + set +e + node -r ./src/setup_node_env \ + "$GITHUB_WORKSPACE/scripts/ppl-lint/validate-osd-grammar.mjs" \ + --grammar "$GITHUB_WORKSPACE/ppl-grammar-bundle.json" \ + --cases "$GITHUB_WORKSPACE/scripts/ppl-lint/grammar-cases.json" \ + --target "$GITHUB_WORKSPACE/resolved-target.json" \ + --osd-root "$PWD" \ + --osd-sha "${{ steps.osd.outputs.sha }}" \ + --report "$GITHUB_WORKSPACE/ppl-lint-grammar-compatibility-report.json" \ + --summary "$GITHUB_STEP_SUMMARY" + result=$? + echo "exit_code=$result" >> "$GITHUB_OUTPUT" + exit 0 + + - name: Record pre-report failure + if: ${{ always() }} + run: | + if [[ ! -s ppl-lint-grammar-compatibility-report.json ]]; then + error='workflow failed before the compatibility adapter produced a report' + if [[ -s resolved-target.json ]]; then + jq --arg error "$error" '{ + schemaVersion: 1, + status: "error", + error: $error, + sql: .sql, + osd: .osd, + manualOverride: (.osd.override // false), + releaseLineValidationBypassed: (.releaseLineValidationBypassed // false), + rules: {selected: 0, passed: 0, failed: 0}, + caseCounts: {selected: 0, passed: 0, failed: 0}, + cases: [], + failures: [] + }' resolved-target.json > ppl-lint-grammar-compatibility-report.json + else + jq -n --arg error "$error" '{ + schemaVersion: 1, + status: "error", + error: $error, + rules: {selected: 0, passed: 0, failed: 0}, + caseCounts: {selected: 0, passed: 0, failed: 0}, + cases: [], + failures: [] + }' > ppl-lint-grammar-compatibility-report.json + fi + { + echo '## PPL lint grammar compatibility' + echo + echo "- Status: \`error\`" + echo "- Reason: $error" + if [[ -s resolved-target.json ]]; then + jq -r '"- SQL: `\(.sql.targetBranch)` / `\(.sql.version)` / `\(.sql.sha)`"' resolved-target.json + jq -r '"- OSD: `\(.osd.repository) @ \(.osd.ref)` / `\(.osd.version)` / `\(.osd.sha)`"' resolved-target.json + fi + } >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Upload grammar compatibility artifacts + if: ${{ always() }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: ppl-lint-grammar-compatibility + path: | + resolved-target.json + osd-revision.txt + ppl-lint-grammar-compatibility-report.json + ppl-grammar-bundle.json + ppl-grammar-cluster.log + if-no-files-found: warn + + - name: Enforce compatibility result + if: ${{ always() }} + env: + CAPABILITY_AVAILABLE: ${{ steps.capability.outputs.available }} + COMPATIBILITY_EXIT: ${{ steps.compatibility.outputs.exit_code }} + run: | + set -euo pipefail + if [[ "$CAPABILITY_AVAILABLE" == false ]]; then + jq -e '.status == "skipped"' ppl-lint-grammar-compatibility-report.json > /dev/null + exit 0 + fi + [[ -n "$COMPATIBILITY_EXIT" ]] || + { echo "::error::Compatibility validation did not produce an exit code"; exit 1; } + if [[ "$COMPATIBILITY_EXIT" != 0 ]]; then + echo "::error::PPL grammar compatibility validation exited $COMPATIBILITY_EXIT" + exit "$COMPATIBILITY_EXIT" + fi + jq -e '.status == "passed"' ppl-lint-grammar-compatibility-report.json > /dev/null || + { echo "::error::Compatibility validation did not produce a passed report"; exit 1; } diff --git a/docs/dev/index.md b/docs/dev/index.md index fa19a6484c6..620f2ef4912 100644 --- a/docs/dev/index.md +++ b/docs/dev/index.md @@ -40,6 +40,7 @@ + **Piped Processing Language** + [PPL Command Checklist](ppl-commands.md): A checklist of developing a new PPL command + [PPL Functions](ppl-functions.md): Guidance on developing a PPL function + + [PPL Linter Grammar Compatibility CI](ppl-lint-grammar-compatibility-ci.md): Guidance on validating SQL grammar changes against OpenSearch Dashboards PPL linter rules ### Query Processing @@ -73,4 +74,4 @@ + [Comparison Test](testing-comparison-test.md): compares with other databases to ensure functional correctness + **Benchmark** + [Hash Join Benchmark](testing-hash-join-benchmark.md): performance test on hash join implementation -+ **Operation Tools** \ No newline at end of file ++ **Operation Tools** diff --git a/docs/dev/ppl-lint-grammar-compatibility-ci.md b/docs/dev/ppl-lint-grammar-compatibility-ci.md new file mode 100644 index 00000000000..dd04b97bfe7 --- /dev/null +++ b/docs/dev/ppl-lint-grammar-compatibility-ci.md @@ -0,0 +1,166 @@ +# PPL Linter Grammar Compatibility CI + +This check verifies that the runtime PPL grammar built from a SQL revision +remains compatible with the grammar-dependent linter rules in the paired +OpenSearch Dashboards (OSD) revision. + +SQL owns the candidate grammar endpoint, branch pairing, grammar cases, and CI +artifacts. OSD owns the headless lint API, rule catalog, detectors, and +parse-tree behavior. + +The check is deliberately grammar-only. It does not send PPL queries to an +OpenSearch backend, create indices or fixtures, compare historical engines, run +Analytics Engine, or assert diagnostic wording, severity, fixes, hover content, +or UI rendering. OpenSearch runs only long enough to serve +`GET /_plugins/_ppl/_grammar`. + +## Branch pairing + +The SQL event target selects the OSD branch: + +| SQL target | OSD ref | Version check | +| --- | --- | --- | +| `main` | `main` | Record both versions; their release lines may differ | +| Exact `X.Y` | Exact `X.Y` | SQL and OSD must both report `X.Y.z` | + +Pull requests use `github.base_ref`; pushes and manual dispatches use +`github.ref_name`. The workflow tests the checked-out SQL revision and records +the pull request head SHA separately when available. + +SQL's product version is the default `opensearch.version` in the root +`build.gradle`. OSD's product version is read from the checked-out build with +`yarn --silent pkg-version`, after selecting Node from OSD's `.nvmrc`. Patch +versions may differ on an exact release line. + +The workflow does not infer an exact branch for `X.x` branches and never falls +back from a missing OSD `X.Y` branch to OSD `main`. There is no fixed +historical-release lane on `main`: the contract is between the two branches +that ship together, not between current SQL and an unrelated old OSD grammar. + +## Validation path + +The focused workflow is +[ppl-lint-grammar-compatibility.yml](../../.github/workflows/ppl-lint-grammar-compatibility.yml). +For each supported event it: + +1. Resolves the SQL target and paired OSD ref. +2. Checks out OSD and records its immutable SHA before running OSD code. +3. Validates product versions and writes `resolved-target.json`. +4. Probes the + `src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint` module, + accepting its `.ts` or `.js` form. +5. If the module is absent, writes a successful capability-skip report and + stops before starting OpenSearch or bootstrapping OSD. +6. Otherwise starts the existing `:opensearch-sql-plugin:run` task, captures + and validates `ppl-grammar-bundle.json`, then stops the cluster. +7. Bootstraps OSD and runs + [validate-osd-grammar.mjs](../../scripts/ppl-lint/validate-osd-grammar.mjs) + against [grammar-cases.json](../../scripts/ppl-lint/grammar-cases.json). +8. Publishes the report and evidence before enforcing the result. + +Capability is detected from the checkout, not from a product-version constant. +An absent module means: + +```json +{ + "status": "skipped", + "skipReason": "osd-headless-grammar-api-unavailable" +} +``` + +This is exit code `0` and executes zero cases. If the module exists but cannot +load `deserializeBundleOrThrow` and `lintQueryWithBundle`, the supported API is +broken and the run fails structurally. It must not become a skip. + +The adapter deserializes the candidate bundle without a compiled-grammar +fallback. For each case it enables only the named rule, supplies the normalized +SQL version, builds a parse tree, and compares the target rule's diagnostic +count. Every selected rule needs trigger and control coverage, and every case +must name a rule in the paired OSD catalog. Missing cases, rules, parse trees, +syntax-clean trees, or bundles fail rather than pass vacuously. + +## Events + +| Event | Purpose | OSD source | +| --- | --- | --- | +| `pull_request` | Pre-merge candidate validation | Canonical paired branch | +| `push` | Validation of the exact merged revision | Canonical paired branch | +| `workflow_dispatch` | Non-required OSD branch, fork, or SHA evidence | Optional `osd_repo` and `osd_ref` | + +Pull request and push runs use no repository variables to redirect OSD. +Manual overrides remain diagnostic evidence and do not satisfy branch +protection. + +## Reports and artifacts + +The `ppl-lint-grammar-compatibility` CI artifact contains the files that were +available for the run: + +- `resolved-target.json`: SQL and OSD branches, versions, and immutable SHAs; +- `osd-revision.txt`: the recorded OSD SHA; +- `ppl-lint-grammar-compatibility-report.json`: final machine-readable result; +- `ppl-grammar-bundle.json`: candidate bundle when capability is available; and +- `ppl-grammar-cluster.log`: candidate cluster output when it was started. + +Every report has `schemaVersion`, `status`, and rule counts. Reports produced +after target resolution also include SQL and OSD metadata, the manual-override +flag, and the release-line-bypass flag. A validating report additionally +includes `grammarHash`, case counts, normalized case results, and failures with +rule ID, case ID, query, and expected and actual diagnostic counts. A skipped +report includes `skipReason` and zero selected rules. Structural adapter +failures use `status: "error"` and include `error`. +Failures before target resolution still produce an error report, but cannot +include SQL or OSD metadata. The GitHub step summary presents the available +provenance and failed cases. + +The wrapper and adapter use these exit codes: + +| Code | Meaning | +| ---: | --- | +| `0` | All cases passed, or the paired OSD branch lacks the headless API | +| `1` | One or more grammar/linter diagnostic counts did not match | +| `2` | Pairing, input, bundle, case coverage, or advertised-API failure | + +See the [operational quick start](../../scripts/ppl-lint/README.md) for local +commands and exact-SHA reproduction. + +## Security + +- Required runs execute only the canonical paired OSD branch. +- OSD repository/ref overrides and release-line bypasses are + `workflow_dispatch`-only. +- The job has `contents: read`, persists no checkout credentials, and receives + no repository secrets. +- The immutable OSD SHA is recorded before OSD dependency or build code runs. +- Manual fork/ref runs are non-required and receive no privileged credentials. + +## Release branches + +When a new exact `X.Y` branch is cut, carry the workflow, scripts, grammar +cases, and this documentation with the SQL branch. Confirm that the OSD `X.Y` +branch exists and both builds report `X.Y.z`, then run a manual canary. +Capability detection decides whether the branch validates or reports +`skipped`; no version constant or documentation rewrite is required. + +An already-cut branch without the OSD API may continue to report a visible +successful skip. If the API is later backported, the same workflow starts +validating automatically. Version-family branches remain unsupported until +they receive an explicit pairing policy. + +## Triage + +Always begin with the SQL and OSD SHAs in the report. Reproducing against a +newer `main` does not reproduce the completed run. + +| Failure | First owner or action | +| --- | --- | +| SQL or OSD version disagrees with exact `X.Y` | CI owner checks branch selection and branch-cut state | +| Paired OSD branch is missing | CI owner fixes pairing; never substitute `main` | +| Headless module is absent | No product action; verify `skipped` and exact metadata | +| Module exists but imports or exports fail | OSD linter owner treats it as a headless API regression | +| Cluster startup or grammar GET fails | SQL plugin owner inspects `ppl-grammar-cluster.log` | +| Bundle validation or deserialization fails | SQL grammar-bundle owner checks the endpoint schema and generated grammar | +| Trigger stops firing | SQL grammar and OSD rule owners inspect the parse-tree contract | +| Control starts firing | OSD rule owner checks whether matching broadened intentionally | +| Case names a missing OSD rule | Review the OSD change, then update or remove the stale SQL case | +| Post-merge push fails | Fix the merged revision before relying on its compatibility evidence | diff --git a/scripts/ppl-lint-rule-validation.sh b/scripts/ppl-lint-rule-validation.sh new file mode 100755 index 00000000000..b8bcac30327 --- /dev/null +++ b/scripts/ppl-lint-rule-validation.sh @@ -0,0 +1,493 @@ +#!/usr/bin/env bash +# +# Copyright OpenSearch Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Capture the candidate SQL runtime grammar and validate it with the paired +# OpenSearch Dashboards (OSD) headless PPL linter. + +set -euo pipefail + +SQL_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$SQL_ROOT" + +HEADLESS_MODULE="src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint" +VALIDATOR="$SQL_ROOT/scripts/ppl-lint/validate-osd-grammar.mjs" + +TARGET_INPUT="${PPL_LINT_TARGET:-}" +OSD_ROOT_INPUT="${OSD_SOURCE_PATH:-}" +TARGET_BRANCH_INPUT="${TARGET_BRANCH:-${SQL_TARGET_BRANCH:-}}" +OSD_REPOSITORY_INPUT="${OSD_REPOSITORY:-}" +OSD_REF_INPUT="${OSD_REF:-}" + +GRAMMAR_BUNDLE="${PPL_LINT_GRAMMAR_BUNDLE:-$SQL_ROOT/ppl-grammar-bundle.json}" +GRAMMAR_CASES="${PPL_LINT_CASES:-$SQL_ROOT/scripts/ppl-lint/grammar-cases.json}" +REPORT="${PPL_LINT_REPORT:-$SQL_ROOT/ppl-lint-grammar-compatibility-report.json}" +SUMMARY="${PPL_LINT_SUMMARY:-${GITHUB_STEP_SUMMARY:-$SQL_ROOT/ppl-lint-grammar-summary.md}}" +CLUSTER_LOG="${PPL_LINT_CLUSTER_LOG:-$SQL_ROOT/ppl-grammar-cluster.log}" +STARTUP_TIMEOUT="${PPL_LINT_STARTUP_TIMEOUT_SECONDS:-300}" +SKIP_OSD_BOOTSTRAP="${PPL_LINT_SKIP_OSD_BOOTSTRAP:-0}" +RELEASE_LINE_BYPASS=false + +GRADLE_PID="" +CAPTURE_TMP="" + +log() { + printf '[ppl-lint-rule-validation] %s\n' "$*" +} + +die() { + log "ERROR: $*" >&2 + exit 2 +} + +usage() { + cat <<'EOF' +Usage: + scripts/ppl-lint-rule-validation.sh --target FILE --osd-root DIR + scripts/ppl-lint-rule-validation.sh --osd-root DIR [--target-branch BRANCH] + [--osd-repository OWNER/REPO] [--osd-ref REF] + +Options: + --target FILE Existing resolved-target.json from CI. + --osd-root DIR Existing OSD checkout (or OSD_SOURCE_PATH). + --target-branch NAME SQL target branch for local metadata (main or X.Y). + --osd-repository NAME OSD repository recorded in local metadata. + --osd-ref REF OSD ref recorded in local metadata. + --grammar FILE Captured grammar output path. + --cases FILE Grammar cases input path. + --report FILE Validation report output path. + --summary FILE Validation summary output path. + --cluster-log FILE Gradle run log output path. + --startup-timeout SEC Bounded cluster readiness timeout (default: 300). + -h, --help Show this help. + +Environment: + PPL_LINT_SKIP_OSD_BOOTSTRAP=1 + Skip OSD bootstrap on the supported path. The caller + must ensure generated OSD targets match the checkout. +EOF +} + +require_value() { + local option="$1" + local value="${2:-}" + [[ -n "$value" ]] || die "$option requires a value" +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --target) + require_value "$1" "${2:-}" + TARGET_INPUT="$2" + shift 2 + ;; + --osd-root) + require_value "$1" "${2:-}" + OSD_ROOT_INPUT="$2" + shift 2 + ;; + --target-branch) + require_value "$1" "${2:-}" + TARGET_BRANCH_INPUT="$2" + shift 2 + ;; + --osd-repository) + require_value "$1" "${2:-}" + OSD_REPOSITORY_INPUT="$2" + shift 2 + ;; + --osd-ref) + require_value "$1" "${2:-}" + OSD_REF_INPUT="$2" + shift 2 + ;; + --grammar) + require_value "$1" "${2:-}" + GRAMMAR_BUNDLE="$2" + shift 2 + ;; + --cases) + require_value "$1" "${2:-}" + GRAMMAR_CASES="$2" + shift 2 + ;; + --report) + require_value "$1" "${2:-}" + REPORT="$2" + shift 2 + ;; + --summary) + require_value "$1" "${2:-}" + SUMMARY="$2" + shift 2 + ;; + --cluster-log) + require_value "$1" "${2:-}" + CLUSTER_LOG="$2" + shift 2 + ;; + --startup-timeout) + require_value "$1" "${2:-}" + STARTUP_TIMEOUT="$2" + shift 2 + ;; + -h|--help) + usage + exit 0 + ;; + *) + die "unknown argument: $1" + ;; + esac +done + +absolute_path() { + case "$1" in + /*) printf '%s\n' "$1" ;; + *) printf '%s/%s\n' "$SQL_ROOT" "$1" ;; + esac +} + +normalize_version() { + local raw="$1" + local normalized="${raw%%[-+]*}" + if [[ ! "$normalized" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + die "invalid product version: $raw" + fi + if [[ "$raw" != "$normalized" && "$raw" != "$normalized"-* && "$raw" != "$normalized"+* ]]; then + die "invalid product version: $raw" + fi + printf '%s\n' "$normalized" +} + +release_line() { + local version="$1" + printf '%s\n' "${version%.*}" +} + +read_sql_version() { + local version + version="$( + sed -nE \ + 's/.*opensearch_version = System\.getProperty\("opensearch\.version", "([^"]+)"\).*/\1/p' \ + "$SQL_ROOT/build.gradle" + )" + [[ -n "$version" && "$version" != *$'\n'* ]] || + die "could not read one default opensearch.version from build.gradle" + printf '%s\n' "$version" +} + +stop_cluster() { + local pid="$GRADLE_PID" + local count=0 + + [[ -n "$pid" ]] || return 0 + if kill -0 "$pid" 2>/dev/null; then + log "Stopping Gradle development cluster (pid $pid)" + kill "$pid" 2>/dev/null || true + while kill -0 "$pid" 2>/dev/null && [[ "$count" -lt 30 ]]; do + sleep 1 + count=$((count + 1)) + done + if kill -0 "$pid" 2>/dev/null; then + log "Gradle process did not stop in time; terminating it" + kill -KILL "$pid" 2>/dev/null || true + fi + fi + wait "$pid" 2>/dev/null || true + GRADLE_PID="" +} + +cleanup() { + local status=$? + trap - EXIT + stop_cluster + if [[ -n "$CAPTURE_TMP" ]]; then + rm -f "$CAPTURE_TMP" + fi + exit "$status" +} + +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM + +for command in curl git jq node; do + command -v "$command" >/dev/null 2>&1 || die "required command not found: $command" +done +[[ "$STARTUP_TIMEOUT" =~ ^[1-9][0-9]*$ ]] || + die "--startup-timeout must be a positive integer" +[[ "$SKIP_OSD_BOOTSTRAP" == "0" || "$SKIP_OSD_BOOTSTRAP" == "1" ]] || + die "PPL_LINT_SKIP_OSD_BOOTSTRAP must be 0 or 1" + +GRAMMAR_BUNDLE="$(absolute_path "$GRAMMAR_BUNDLE")" +GRAMMAR_CASES="$(absolute_path "$GRAMMAR_CASES")" +REPORT="$(absolute_path "$REPORT")" +SUMMARY="$(absolute_path "$SUMMARY")" +CLUSTER_LOG="$(absolute_path "$CLUSTER_LOG")" + +if [[ -n "$TARGET_INPUT" ]]; then + TARGET="$(absolute_path "$TARGET_INPUT")" + [[ -f "$TARGET" ]] || die "target metadata not found: $TARGET" + jq -e ' + type == "object" and + (.sql | type == "object") and + (.sql.sha | type == "string" and length > 0) and + (.sql.targetBranch | type == "string" and length > 0) and + (.sql.version | type == "string" and length > 0) and + (.osd | type == "object") and + (.osd.repository | type == "string" and length > 0) and + (.osd.ref | type == "string" and length > 0) and + (.osd.sha | type == "string" and length > 0) and + (.osd.version | type == "string" and length > 0) and + ((has("releaseLineValidationBypassed") | not) or + (.releaseLineValidationBypassed | type == "boolean")) + ' "$TARGET" >/dev/null || die "target metadata is missing required SQL/OSD fields: $TARGET" + OSD_REPOSITORY_INPUT="$(jq -r '.osd.repository' "$TARGET")" + OSD_REF_INPUT="$(jq -r '.osd.ref' "$TARGET")" + OSD_CHECKOUT_REF="$(jq -r '.osd.sha' "$TARGET")" + RELEASE_LINE_BYPASS="$(jq -r '.releaseLineValidationBypassed // false' "$TARGET")" +else + TARGET_BRANCH_INPUT="${TARGET_BRANCH_INPUT:-main}" + OSD_REPOSITORY_INPUT="${OSD_REPOSITORY_INPUT:-opensearch-project/OpenSearch-Dashboards}" + OSD_REF_INPUT="${OSD_REF_INPUT:-$TARGET_BRANCH_INPUT}" + OSD_CHECKOUT_REF="$OSD_REF_INPUT" + TARGET="$SQL_ROOT/resolved-target.json" +fi + +if [[ -n "$OSD_ROOT_INPUT" ]]; then + OSD_ROOT="$(cd "$OSD_ROOT_INPUT" 2>/dev/null && pwd)" || + die "OSD checkout not found: $OSD_ROOT_INPUT" +else + OSD_ROOT="$SQL_ROOT/.ci/OpenSearch-Dashboards" + OSD_REPO_URL="${OSD_REPO_URL:-https://github.com/$OSD_REPOSITORY_INPUT.git}" + if [[ ! -d "$OSD_ROOT" ]]; then + log "Creating managed OSD checkout for $OSD_REPOSITORY_INPUT" + mkdir -p "$(dirname "$OSD_ROOT")" + git clone --filter=blob:none --no-checkout --depth 1 "$OSD_REPO_URL" "$OSD_ROOT" + else + git -C "$OSD_ROOT" rev-parse --is-inside-work-tree >/dev/null 2>&1 || + die "managed OSD path is not a Git checkout: $OSD_ROOT" + if git -C "$OSD_ROOT" remote get-url origin >/dev/null 2>&1; then + git -C "$OSD_ROOT" remote set-url origin "$OSD_REPO_URL" + else + git -C "$OSD_ROOT" remote add origin "$OSD_REPO_URL" + fi + fi + log "Resolving managed OSD checkout at $OSD_REPOSITORY_INPUT@$OSD_CHECKOUT_REF" + git -C "$OSD_ROOT" fetch --depth 1 origin "$OSD_CHECKOUT_REF" + git -C "$OSD_ROOT" checkout --detach FETCH_HEAD + OSD_ROOT="$(cd "$OSD_ROOT" && pwd)" +fi + +git -C "$OSD_ROOT" rev-parse --is-inside-work-tree >/dev/null 2>&1 || + die "OSD root is not a Git checkout: $OSD_ROOT" +[[ -f "$OSD_ROOT/package.json" ]] || die "OSD package.json not found under $OSD_ROOT" + +SQL_SHA_ACTUAL="$(git -C "$SQL_ROOT" rev-parse HEAD)" +SQL_VERSION_RAW_ACTUAL="$(read_sql_version)" +SQL_VERSION_ACTUAL="$(normalize_version "$SQL_VERSION_RAW_ACTUAL")" +OSD_SHA_ACTUAL="$(git -C "$OSD_ROOT" rev-parse HEAD)" +OSD_VERSION_RAW_ACTUAL="$(jq -er '.version | select(type == "string" and length > 0)' "$OSD_ROOT/package.json")" || + die "could not read OSD package.json version" +OSD_VERSION_ACTUAL="$(normalize_version "$OSD_VERSION_RAW_ACTUAL")" + +if [[ -z "$TARGET_INPUT" && -n "$OSD_ROOT_INPUT" ]]; then + OSD_REF_SHA="$(git -C "$OSD_ROOT" rev-parse --verify "${OSD_REF_INPUT}^{commit}" 2>/dev/null)" || + die "OSD ref $OSD_REF_INPUT is not available in $OSD_ROOT" + [[ "$OSD_REF_SHA" == "$OSD_SHA_ACTUAL" ]] || + die "OSD ref $OSD_REF_INPUT resolves to $OSD_REF_SHA, not checkout $OSD_SHA_ACTUAL" +fi + +if [[ -z "$TARGET_INPUT" ]]; then + [[ "$TARGET_BRANCH_INPUT" == "main" || "$TARGET_BRANCH_INPUT" =~ ^[0-9]+\.[0-9]+$ ]] || + die "local --target-branch must be main or an exact X.Y release branch" + OSD_OVERRIDE=false + if [[ "$OSD_REPOSITORY_INPUT" != "opensearch-project/OpenSearch-Dashboards" || + "$OSD_REF_INPUT" != "$TARGET_BRANCH_INPUT" ]]; then + OSD_OVERRIDE=true + fi + target_tmp="$(mktemp "$TARGET.tmp.XXXXXX")" + jq -n \ + --arg sqlSha "$SQL_SHA_ACTUAL" \ + --arg sqlHeadSha "${SQL_HEAD_SHA:-}" \ + --arg targetBranch "$TARGET_BRANCH_INPUT" \ + --arg sqlVersionRaw "$SQL_VERSION_RAW_ACTUAL" \ + --arg sqlVersion "$SQL_VERSION_ACTUAL" \ + --arg osdRepository "$OSD_REPOSITORY_INPUT" \ + --arg osdRef "$OSD_REF_INPUT" \ + --arg osdSha "$OSD_SHA_ACTUAL" \ + --arg osdVersion "$OSD_VERSION_ACTUAL" \ + --argjson osdOverride "$OSD_OVERRIDE" \ + '{ + schemaVersion: 1, + sql: { + sha: $sqlSha, + targetBranch: $targetBranch, + versionRaw: $sqlVersionRaw, + version: $sqlVersion + }, + osd: { + repository: $osdRepository, + ref: $osdRef, + sha: $osdSha, + version: $osdVersion, + override: $osdOverride + }, + releaseLineValidationBypassed: false + } + | if $sqlHeadSha == "" then . else .sql.headSha = $sqlHeadSha end' \ + >"$target_tmp" + mv "$target_tmp" "$TARGET" + log "Wrote local target metadata: $TARGET" +fi + +SQL_SHA="$(jq -r '.sql.sha' "$TARGET")" +TARGET_BRANCH="$(jq -r '.sql.targetBranch' "$TARGET")" +SQL_VERSION="$(normalize_version "$(jq -r '.sql.version' "$TARGET")")" +OSD_SHA="$(jq -r '.osd.sha' "$TARGET")" +OSD_VERSION="$(normalize_version "$(jq -r '.osd.version' "$TARGET")")" + +[[ "$SQL_SHA" == "$SQL_SHA_ACTUAL" ]] || + die "target SQL SHA $SQL_SHA does not match checkout $SQL_SHA_ACTUAL" +[[ "$OSD_SHA" == "$OSD_SHA_ACTUAL" ]] || + die "target OSD SHA $OSD_SHA does not match checkout $OSD_SHA_ACTUAL" +[[ "$SQL_VERSION" == "$SQL_VERSION_ACTUAL" ]] || + die "target SQL version $SQL_VERSION does not match build.gradle $SQL_VERSION_ACTUAL" +[[ "$OSD_VERSION" == "$OSD_VERSION_ACTUAL" ]] || + die "target OSD version $OSD_VERSION does not match package.json $OSD_VERSION_ACTUAL" + +if [[ "$TARGET_BRANCH" != "main" ]]; then + [[ "$TARGET_BRANCH" =~ ^[0-9]+\.[0-9]+$ ]] || + die "target branch must be main or an exact X.Y release branch: $TARGET_BRANCH" + if [[ "$RELEASE_LINE_BYPASS" != "true" ]]; then + [[ "$(release_line "$SQL_VERSION")" == "$TARGET_BRANCH" ]] || + die "SQL version $SQL_VERSION does not match target release line $TARGET_BRANCH" + [[ "$(release_line "$OSD_VERSION")" == "$TARGET_BRANCH" ]] || + die "OSD version $OSD_VERSION does not match target release line $TARGET_BRANCH" + fi +fi + +[[ -f "$VALIDATOR" ]] || die "validation adapter not found: $VALIDATOR" +mkdir -p "$(dirname "$GRAMMAR_BUNDLE")" "$(dirname "$REPORT")" \ + "$(dirname "$SUMMARY")" "$(dirname "$CLUSTER_LOG")" + +ADAPTER_ARGS=( + --grammar "$GRAMMAR_BUNDLE" + --cases "$GRAMMAR_CASES" + --target "$TARGET" + --osd-root "$OSD_ROOT" + --osd-sha "$OSD_SHA" + --report "$REPORT" + --summary "$SUMMARY" +) + +headless_module_exists() { + [[ -f "$OSD_ROOT/$HEADLESS_MODULE" || + -f "$OSD_ROOT/$HEADLESS_MODULE.ts" || + -f "$OSD_ROOT/$HEADLESS_MODULE.js" || + -f "$OSD_ROOT/$HEADLESS_MODULE.mjs" ]] +} + +if ! headless_module_exists; then + log "OSD headless grammar API is unavailable; requesting a skipped report" + node "$VALIDATOR" "${ADAPTER_ARGS[@]}" + log "Validation skipped; report: $REPORT" + exit 0 +fi + +[[ -f "$GRAMMAR_CASES" ]] || die "grammar cases not found: $GRAMMAR_CASES" +[[ -x "$SQL_ROOT/gradlew" ]] || die "Gradle wrapper is not executable: $SQL_ROOT/gradlew" + +if curl --fail --silent --max-time 2 "http://127.0.0.1:9200/_cluster/health" >/dev/null 2>&1; then + die "port 9200 already serves an OpenSearch cluster; refusing to capture from an unknown process" +fi + +: >"$CLUSTER_LOG" +log "Starting candidate SQL development cluster" +./gradlew :opensearch-sql-plugin:run >"$CLUSTER_LOG" 2>&1 & +GRADLE_PID=$! + +deadline=$((SECONDS + STARTUP_TIMEOUT)) +while true; do + if ! kill -0 "$GRADLE_PID" 2>/dev/null; then + wait "$GRADLE_PID" 2>/dev/null || gradle_status=$? + GRADLE_PID="" + die "Gradle run exited before cluster readiness (status ${gradle_status:-0}); see $CLUSTER_LOG" + fi + if curl --fail --silent --show-error --connect-timeout 2 --max-time 5 \ + "http://127.0.0.1:9200/_cluster/health" >/dev/null 2>&1; then + break + fi + (( SECONDS < deadline )) || + die "cluster did not become ready within ${STARTUP_TIMEOUT}s; see $CLUSTER_LOG" + sleep 2 +done + +CAPTURE_TMP="$(mktemp "$GRAMMAR_BUNDLE.tmp.XXXXXX")" +log "Capturing GET /_plugins/_ppl/_grammar" +if ! http_status="$( + curl --silent --show-error --connect-timeout 2 --max-time 30 \ + --output "$CAPTURE_TMP" --write-out '%{http_code}' \ + "http://127.0.0.1:9200/_plugins/_ppl/_grammar" +)"; then + die "grammar endpoint request failed; see $CLUSTER_LOG" +fi +if [[ "$http_status" != "200" ]]; then + mv "$CAPTURE_TMP" "$GRAMMAR_BUNDLE" + CAPTURE_TMP="" + die "grammar endpoint returned HTTP $http_status" +fi + +jq -e ' + def nonempty_strings: + type == "array" and length > 0 and all(.[]; type == "string" and length > 0); + def nonempty_integers: + type == "array" and length > 0 and all(.[]; type == "number" and floor == .); + def sparse_names: + type == "array" and length > 0 and + all(.[]; . == null or type == "string") and any(.[]; . == null); + type == "object" and + (.bundleVersion | type == "string" and length > 0) and + (.antlrVersion | type == "string" and length > 0) and + (.grammarHash | type == "string" and test("^sha256:[0-9a-fA-F]{64}$")) and + (.lexerSerializedATN | nonempty_integers) and + (.parserSerializedATN | nonempty_integers) and + (.lexerRuleNames | nonempty_strings) and + (.parserRuleNames | nonempty_strings) and + (.channelNames | nonempty_strings) and + (.modeNames | nonempty_strings) and + (.startRuleIndex | type == "number" and floor == . and . >= 0) and + (.literalNames | sparse_names) and + (.symbolicNames | sparse_names) and + (.tokenDictionary | + type == "object" and length > 0 and + all(.[]; type == "number" and floor == . and . >= 0)) and + (.ignoredTokens | type == "array" and all(.[]; type == "number" and floor == .)) and + (.rulesToVisit | nonempty_integers) +' "$CAPTURE_TMP" >/dev/null || die "grammar endpoint returned a malformed bundle" + +mv "$CAPTURE_TMP" "$GRAMMAR_BUNDLE" +CAPTURE_TMP="" +log "Captured structurally valid grammar bundle: $GRAMMAR_BUNDLE" +stop_cluster + +if [[ "$SKIP_OSD_BOOTSTRAP" == "1" ]]; then + log "Skipping OSD bootstrap because PPL_LINT_SKIP_OSD_BOOTSTRAP=1" +else + command -v yarn >/dev/null 2>&1 || die "required command not found: yarn" + log "Bootstrapping OSD" + (cd "$OSD_ROOT" && yarn osd bootstrap) || die "OSD bootstrap failed" +fi + +[[ -d "$OSD_ROOT/src/setup_node_env" || -f "$OSD_ROOT/src/setup_node_env.js" || + -f "$OSD_ROOT/src/setup_node_env.ts" || -f "$OSD_ROOT/src/setup_node_env" ]] || + die "OSD setup_node_env entry point not found" + +log "Validating candidate grammar with OSD@$OSD_SHA" +( + cd "$OSD_ROOT" + node -r ./src/setup_node_env "$VALIDATOR" "${ADAPTER_ARGS[@]}" +) +log "Validation completed; report: $REPORT" diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md new file mode 100644 index 00000000000..09c4e713605 --- /dev/null +++ b/scripts/ppl-lint/README.md @@ -0,0 +1,213 @@ +# PPL grammar compatibility quick start + +This directory contains the grammar cases and the adapter used by +[PPL linter grammar compatibility CI](../../docs/dev/ppl-lint-grammar-compatibility-ci.md): + +- [grammar-cases.json](grammar-cases.json) contains grammar-only trigger and + control queries with expected diagnostic counts. +- [validate-osd-grammar.mjs](validate-osd-grammar.mjs) loads the paired OSD + headless linter API and writes the compatibility report. +- [ppl-lint-rule-validation.sh](../ppl-lint-rule-validation.sh) resolves local + metadata, captures the candidate SQL grammar, bootstraps OSD, and invokes the + adapter. + +The temporary OpenSearch process serves only +`GET /_plugins/_ppl/_grammar`. This validation creates no indices or fixtures +and sends no backend PPL queries. + +## Prerequisites + +- JDK 21; +- Node from the selected OSD checkout's `.nvmrc`; +- the Yarn version required by that checkout's `package.json`; +- `curl`, `git`, `jq`, and an available local port `9200`; and +- either a local OSD checkout or permission to clone + `opensearch-project/OpenSearch-Dashboards`. + +## Run the CI-equivalent path + +From the SQL repository root: + +```bash +./scripts/ppl-lint-rule-validation.sh +``` + +With no options, the wrapper pairs local SQL with OSD `main` and clones OSD +into `.ci/OpenSearch-Dashboards` when that checkout does not exist. To use a +sibling checkout: + +```bash +./scripts/ppl-lint-rule-validation.sh \ + --osd-root ../OpenSearch-Dashboards \ + --target-branch main +``` + +For an exact release branch, check out the same `X.Y` line in both repositories +and use `--target-branch X.Y --osd-ref X.Y`. The wrapper rejects SQL or OSD +versions outside that release line. On `main`, it records both product versions +without requiring their release lines to match. + +The wrapper checks OSD capability before starting OpenSearch. If the headless +module is absent, it writes a `skipped` report and exits `0`. + +## Exact CI reproduction + +Start with `resolved-target.json` from the +`ppl-lint-grammar-compatibility` artifact. Check out the report's tested SQL SHA +and OSD SHA, not current branch tips: + +```bash +git checkout --detach "$(jq -r '.sql.sha' /path/to/resolved-target.json)" +git -C ../OpenSearch-Dashboards checkout --detach \ + "$(jq -r '.osd.sha' /path/to/resolved-target.json)" + +./scripts/ppl-lint-rule-validation.sh \ + --target /path/to/resolved-target.json \ + --osd-root ../OpenSearch-Dashboards +``` + +Fetch either SHA from its repository first if it is not present locally. The +wrapper verifies both checkout SHAs and both product versions against the +target file before doing any validation. For pull requests, `.sql.sha` is the +tested merge revision; `.sql.headSha` is traceability metadata, not a +substitute. A dispatch artifact with `releaseLineValidationBypassed: true` +retains that development-only bypass during exact reproduction. + +## Local files + +The wrapper defaults to these repository-root paths: + +| Path | Role | +| --- | --- | +| `resolved-target.json` | Generated SQL/OSD metadata when `--target` is omitted | +| `ppl-grammar-bundle.json` | Captured production grammar endpoint response | +| `scripts/ppl-lint/grammar-cases.json` | SQL-owned trigger/control input | +| `ppl-lint-grammar-compatibility-report.json` | Machine-readable result | +| `ppl-lint-grammar-summary.md` | Local human-readable summary | +| `ppl-grammar-cluster.log` | Gradle development-cluster output | + +Override these with `--grammar`, `--cases`, `--report`, `--summary`, and +`--cluster-log`. + +A capability skip does not start the cluster, so no bundle or cluster log is +expected. + +## Primitive debugging + +Inspect the versions using the same sources as CI: + +```bash +sed -nE \ + 's/.*opensearch_version = System\.getProperty\("opensearch\.version", "([^"]+)"\).*/\1/p' \ + build.gradle + +cd ../OpenSearch-Dashboards +nvm use +yarn --silent pkg-version +``` + +Check capability: + +```bash +HEADLESS=../OpenSearch-Dashboards/src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint +test -f "${HEADLESS}.ts" || test -f "${HEADLESS}.js" +``` + +Start the existing SQL development cluster from the SQL root: + +```bash +./gradlew :opensearch-sql-plugin:run +``` + +In another terminal, capture and inspect the endpoint: + +```bash +curl --fail --silent --show-error \ + http://127.0.0.1:9200/_plugins/_ppl/_grammar \ + --output ppl-grammar-bundle.json + +jq -e ' + (.grammarHash | test("^sha256:[0-9a-fA-F]{64}$")) and + (.lexerSerializedATN | length > 0) and + (.parserSerializedATN | length > 0) and + (.lexerRuleNames | length > 0) and + (.parserRuleNames | length > 0) +' ppl-grammar-bundle.json +``` + +Stop the development cluster after capture. The wrapper is preferred for normal +use because its trap stops the process on success and failure. + +To run only the adapter after `resolved-target.json` and the bundle exist, +bootstrap the exact OSD checkout first with `yarn osd bootstrap` unless its +generated targets already match that revision: + +```bash +SQL_ROOT=/absolute/path/to/sql +OSD_ROOT=/absolute/path/to/OpenSearch-Dashboards +cd "$OSD_ROOT" + +node -r ./src/setup_node_env \ + "$SQL_ROOT/scripts/ppl-lint/validate-osd-grammar.mjs" \ + --grammar "$SQL_ROOT/ppl-grammar-bundle.json" \ + --cases "$SQL_ROOT/scripts/ppl-lint/grammar-cases.json" \ + --target "$SQL_ROOT/resolved-target.json" \ + --osd-root "$OSD_ROOT" \ + --osd-sha "$(git rev-parse HEAD)" \ + --report "$SQL_ROOT/ppl-lint-grammar-compatibility-report.json" \ + --summary "$SQL_ROOT/ppl-lint-grammar-summary.md" +``` + +## Report results + +Minimal passed result: + +```json +{ + "schemaVersion": 1, + "status": "passed", + "sql": {"sha": "", "targetBranch": "main", "version": "X.Y.Z"}, + "osd": {"ref": "main", "sha": "", "version": "X.Y.Z"}, + "manualOverride": false, + "grammarHash": "sha256:", + "rules": {"selected": 9, "passed": 9, "failed": 0}, + "caseCounts": {"selected": 18, "passed": 18, "failed": 0}, + "failures": [] +} +``` + +Minimal skipped result: + +```json +{ + "schemaVersion": 1, + "status": "skipped", + "skipReason": "osd-headless-grammar-api-unavailable", + "sql": {"sha": "", "targetBranch": "X.Y", "version": "X.Y.Z"}, + "osd": {"ref": "X.Y", "sha": "", "version": "X.Y.Z"}, + "rules": {"selected": 0, "passed": 0, "failed": 0}, + "caseCounts": {"selected": 0, "passed": 0, "failed": 0} +} +``` + +Exit codes: + +| Code | Meaning | +| ---: | --- | +| `0` | Passed, or skipped because the paired OSD API is absent | +| `1` | A diagnostic count mismatch or per-case execution failure | +| `2` | Structural input, pairing, bundle, coverage, or advertised-API failure | + +## Common outcomes + +| Outcome | Action | +| --- | --- | +| Exact `X.Y` version mismatch | Verify both checkouts and branch-cut state; do not use OSD `main` | +| `osd-headless-grammar-api-unavailable` | No product fix; confirm exact metadata and successful skip | +| Module exists but exports fail to load | Treat as an OSD supported-path API regression | +| Cluster startup or grammar GET fails | Inspect `ppl-grammar-cluster.log` | +| Bundle is malformed or cannot deserialize | Check the SQL grammar endpoint schema and generated grammar | +| Rule lacks trigger/control coverage | Add the missing grammar case before interpreting detector results | +| Case names a missing OSD rule | Review the OSD catalog change, then update the stale SQL case | +| Trigger count drops | SQL grammar and OSD rule owners inspect parser node names and tree shape | +| Control count rises | OSD rule owner checks whether matching broadened intentionally | diff --git a/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs b/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs new file mode 100644 index 00000000000..52b08f469c5 --- /dev/null +++ b/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs @@ -0,0 +1,453 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const SCRIPT = fileURLToPath( + new URL('../validate-osd-grammar.mjs', import.meta.url) +); +const HASH = `sha256:${'a'.repeat(64)}`; +const HEADLESS = path.join( + 'src', + 'plugins', + 'data', + 'public', + 'antlr', + 'opensearch_ppl', + 'headless_ppl_lint.js' +); +const CATALOG = path.join( + 'packages', + 'osd-monaco', + 'src', + 'ppl', + 'lint', + 'catalog.js' +); + +function writeJson(file, value) { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify(value)); +} + +function makeTarget({ + branch = '3.8', + sqlVersion = '3.8.0', + osdVersion = '3.8.1', + osdRef = branch, + releaseLineValidationBypassed = false, +} = {}) { + return { + sql: { + sha: 'sql-tested-sha', + headSha: 'sql-head-sha', + targetBranch: branch, + versionRaw: `${sqlVersion}-SNAPSHOT`, + version: sqlVersion, + }, + osd: { + repository: 'opensearch-project/OpenSearch-Dashboards', + ref: osdRef, + version: osdVersion, + }, + releaseLineValidationBypassed, + }; +} + +function defaultCases(ruleIds = ['rule-a']) { + return { + schemaVersion: 1, + cases: ruleIds.flatMap((ruleId) => [ + { + id: `${ruleId}-trigger`, + ruleId, + kind: 'trigger', + query: `source=accounts | ${ruleId} trigger`, + expectedCount: 1, + context: { isCalcite: true }, + }, + { + id: `${ruleId}-control`, + ruleId, + kind: 'control', + query: `source=accounts | ${ruleId} control`, + expectedCount: 0, + context: { isCalcite: true }, + }, + ]), + }; +} + +function headlessSource({ + missingExport = false, + includeBuildTree = true, + directTree = false, +} = {}) { + return ` +exports.deserializeBundleOrThrow = (bundle) => ({ + grammarHash: bundle.grammarHash +}); +${includeBuildTree ? ` +exports.buildRuntimeTree = (query) => + query.includes('no-tree') + ? undefined + : query.includes('syntax-error') + ? { tree: { children: [{ constructor: { name: 'ErrorNode' } }] } } + : ${directTree ? '{}' : '{ tree: {} }'};` : ''} +${missingExport ? '' : ` +exports.lintQueryWithBundle = (query, grammar, context) => { + if (query.includes('throws')) throw new Error('detector crashed'); + const target = Object.entries(context.overrides) + .find(([, override]) => override.enabled)?.[0]; + if (query.includes('wrong-rule')) { + return { diagnostics: [{ ruleId: 'some-other-rule' }] }; + } + return { + diagnostics: query.includes('trigger') ? [{ ruleId: target }] : [] + }; +};`} +`; +} + +function makeFixture( + t, + { + target = makeTarget(), + ruleIds = ['rule-a'], + cases = defaultCases(ruleIds), + api = 'valid', + } = {} +) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-osd-grammar-')); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const osdRoot = path.join(root, 'osd'); + const targetPath = path.join(root, 'resolved-target.json'); + const grammarPath = path.join(root, 'grammar.json'); + const casesPath = path.join(root, 'cases.json'); + const reportPath = path.join(root, 'report.json'); + const summaryPath = path.join(root, 'summary.md'); + fs.mkdirSync(osdRoot, { recursive: true }); + writeJson(targetPath, target); + writeJson(grammarPath, { grammarHash: HASH }); + writeJson(casesPath, cases); + + if (api !== 'absent') { + const headlessPath = path.join(osdRoot, HEADLESS); + fs.mkdirSync(path.dirname(headlessPath), { recursive: true }); + fs.writeFileSync( + headlessPath, + headlessSource({ + missingExport: api === 'missing-export', + includeBuildTree: api !== 'no-build-tree', + directTree: api === 'direct-tree', + }) + ); + const catalogPath = path.join(osdRoot, CATALOG); + fs.mkdirSync(path.dirname(catalogPath), { recursive: true }); + fs.writeFileSync( + catalogPath, + `exports.getBundledCatalog = () => ${JSON.stringify( + ruleIds.map((id) => ({ id })) + )};` + ); + } + + return { + root, + osdRoot, + targetPath, + grammarPath, + casesPath, + reportPath, + summaryPath, + }; +} + +function invoke(fixture, extraArgs = []) { + return spawnSync( + process.execPath, + [ + SCRIPT, + '--grammar', + fixture.grammarPath, + '--cases', + fixture.casesPath, + '--target', + fixture.targetPath, + '--osd-root', + fixture.osdRoot, + '--osd-sha', + 'osd-immutable-sha', + '--report', + fixture.reportPath, + '--summary', + fixture.summaryPath, + ...extraArgs, + ], + { encoding: 'utf8' } + ); +} + +function readReport(fixture) { + return JSON.parse(fs.readFileSync(fixture.reportPath, 'utf8')); +} + +test('main accepts different SQL and OSD product lines and records exact metadata', (t) => { + const fixture = makeFixture(t, { + target: makeTarget({ + branch: 'main', + sqlVersion: '3.9.0', + osdVersion: '3.8.2', + osdRef: 'main', + }), + }); + + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.status, 'passed'); + assert.equal(report.sql.version, '3.9.0'); + assert.equal(report.osd.version, '3.8.2'); + assert.equal(report.osd.sha, 'osd-immutable-sha'); + assert.equal(report.manualOverride, false); + assert.equal(report.grammarHash, HASH); + assert.match(fs.readFileSync(fixture.summaryPath, 'utf8'), /Status: \*\*passed\*\*/); +}); + +test('matching exact release versions accept a RuntimeParseOutcome tree', (t) => { + const fixture = makeFixture(t); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.cases[0].parseTreeCheck, 'verified'); + assert.deepEqual(report.rules, { + selected: 1, + passed: 1, + failed: 0, + }); +}); + +test('matching exact release versions accept a direct ParserRuleContext', (t) => { + const fixture = makeFixture(t, { api: 'direct-tree' }); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.status, 'passed'); + assert.ok(report.cases.every((entry) => entry.parseTreeCheck === 'verified')); +}); + +test('exact release branch version mismatch is structural and writes a report', (t) => { + const fixture = makeFixture(t, { + target: makeTarget({ branch: '3.8', sqlVersion: '3.9.0' }), + }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /requires SQL and OSD 3\.8\.x versions/); + const report = readReport(fixture); + assert.equal(report.status, 'error'); + assert.equal(report.releaseLineValidationBypassed, false); + assert.match(report.error, /SQL 3\.9\.0/); +}); + +test('explicit release-line bypass permits mismatched product lines', (t) => { + const fixture = makeFixture(t, { + target: makeTarget({ + branch: '3.8', + sqlVersion: '3.9.0', + osdVersion: '3.7.2', + releaseLineValidationBypassed: true, + }), + }); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.status, 'passed'); + assert.equal(report.releaseLineValidationBypassed, true); + assert.equal(report.sql.version, '3.9.0'); + assert.equal(report.osd.version, '3.7.2'); + assert.match( + fs.readFileSync(fixture.summaryPath, 'utf8'), + /Release-line validation bypassed: `true`/ + ); +}); + +test('manual OSD override is explicit in the report and summary', (t) => { + const target = makeTarget({ branch: 'main', osdRef: 'candidate' }); + target.osd.override = true; + const fixture = makeFixture(t, { target }); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.manualOverride, true); + assert.match(fs.readFileSync(fixture.summaryPath, 'utf8'), /Manual OSD override: `true`/); +}); + +test('absent headless API skips before grammar and cases are read', (t) => { + const fixture = makeFixture(t, { api: 'absent' }); + fs.rmSync(fixture.grammarPath); + fs.rmSync(fixture.casesPath); + + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.status, 'skipped'); + assert.equal( + report.skipReason, + 'osd-headless-grammar-api-unavailable' + ); + assert.equal(report.caseCounts.selected, 0); + assert.deepEqual(report.cases, []); +}); + +test('an advertised API with a missing export fails structurally', (t) => { + const fixture = makeFixture(t, { api: 'missing-export' }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /must export deserializeBundleOrThrow and lintQueryWithBundle/); + assert.equal(readReport(fixture).status, 'error'); +}); + +test('a malformed bundle fails structurally when capability is present', (t) => { + const fixture = makeFixture(t); + fs.writeFileSync(fixture.grammarPath, '{not json'); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /Could not .*parse grammar bundle/); + assert.equal(readReport(fixture).status, 'error'); +}); + +test('diagnostic count mismatch returns one and preserves every normalized case', (t) => { + const cases = defaultCases(); + cases.cases[0].query = 'source=accounts | wrong-rule trigger'; + const fixture = makeFixture(t, { cases }); + const result = invoke(fixture); + + assert.equal(result.status, 1); + const report = readReport(fixture); + assert.equal(report.status, 'failed'); + assert.equal(report.cases.length, 2); + assert.equal(report.cases[0].status, 'failed'); + assert.equal(report.cases[0].actualCount, 0); + assert.equal(report.cases[1].status, 'passed'); + assert.deepEqual(report.failures[0], { + ruleId: 'rule-a', + caseId: 'rule-a-trigger', + query: 'source=accounts | wrong-rule trigger', + expectedCount: 1, + actualCount: 0, + }); +}); + +test('one case exception does not hide later cases and the failure report exists', (t) => { + const cases = defaultCases(['rule-a', 'rule-b']); + cases.cases[0].query = 'source=accounts | throws'; + const fixture = makeFixture(t, { + ruleIds: ['rule-a', 'rule-b'], + cases, + }); + const result = invoke(fixture); + + assert.equal(result.status, 1); + assert.match(result.stdout, /PASSED rule-b\/rule-b-trigger/); + const report = readReport(fixture); + assert.equal(report.cases.length, 4); + assert.equal(report.cases[0].error, 'detector crashed'); + assert.equal(report.cases[2].status, 'passed'); + assert.deepEqual(report.rules, { + selected: 2, + passed: 1, + failed: 1, + }); +}); + +test('a missing parse tree is a case failure and later cases still execute', (t) => { + const cases = defaultCases(); + cases.cases[0].query = 'source=accounts | no-tree'; + const fixture = makeFixture(t, { cases }); + const result = invoke(fixture); + + assert.equal(result.status, 1); + const report = readReport(fixture); + assert.match(report.cases[0].error, /no parse tree/); + assert.equal(report.cases[1].status, 'passed'); +}); + +test('a recovered syntax error cannot pass as a zero-diagnostic control', (t) => { + const cases = defaultCases(); + cases.cases[1].query = 'source=accounts | syntax-error'; + const fixture = makeFixture(t, { cases }); + const result = invoke(fixture); + + assert.equal(result.status, 1); + const report = readReport(fixture); + assert.equal(report.cases[0].status, 'passed'); + assert.equal(report.cases[1].status, 'failed'); + assert.match(report.cases[1].error, /recovered from a syntax error/); +}); + +test('controls fail explicitly when buildRuntimeTree is not exported', (t) => { + const fixture = makeFixture(t, { api: 'no-build-tree' }); + const result = invoke(fixture); + + assert.equal(result.status, 1); + const report = readReport(fixture); + assert.equal(report.cases[0].status, 'passed'); + assert.equal( + report.cases[0].parseTreeCheck, + 'inferred-from-target-diagnostic' + ); + assert.equal(report.cases[1].status, 'failed'); + assert.equal(report.cases[1].parseTreeCheck, 'unavailable'); + assert.match(report.cases[1].error, /control parse tree cannot be verified/); +}); + +test('anti-vacuous case validation requires trigger and control coverage per rule', (t) => { + const fixture = makeFixture(t, { + cases: { + schemaVersion: 1, + cases: [ + { + id: 'only-trigger', + ruleId: 'rule-a', + kind: 'trigger', + query: 'source=accounts | rule-a trigger', + expectedCount: 1, + }, + ], + }, + }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /must have trigger and control cases/); + assert.equal(readReport(fixture).status, 'error'); +}); + +test('a case naming a missing OSD rule fails structurally', (t) => { + const fixture = makeFixture(t, { + ruleIds: ['different-rule'], + cases: defaultCases(['rule-a']), + }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /names missing OSD rule "rule-a"/); + assert.equal(readReport(fixture).status, 'error'); +}); diff --git a/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs b/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs new file mode 100644 index 00000000000..ab43aa0d919 --- /dev/null +++ b/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs @@ -0,0 +1,395 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const ROOT = fileURLToPath(new URL('../../../', import.meta.url)); +const WORKFLOW = path.join( + ROOT, + '.github', + 'workflows', + 'ppl-lint-grammar-compatibility.yml' +); +const WRAPPER = path.join(ROOT, 'scripts', 'ppl-lint-rule-validation.sh'); +const SOURCE = fs.readFileSync(WORKFLOW, 'utf8'); + +function stepScript(name) { + const lines = SOURCE.split('\n'); + const step = lines.indexOf(` - name: ${name}`); + assert.notEqual(step, -1, `missing workflow step ${name}`); + const next = lines.findIndex( + (line, index) => index > step && line.startsWith(' - name: ') + ); + const end = next === -1 ? lines.length : next; + const run = lines.findIndex( + (line, index) => index > step && index < end && line === ' run: |' + ); + assert.notEqual(run, -1, `step ${name} has no shell block`); + return lines + .slice(run + 1, end) + .map((line) => line.replace(/^ {10}/, '')) + .join('\n') + .trimEnd(); +} + +function temporaryDirectory(t) { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-workflow-pairing-')); + t.after(() => fs.rmSync(directory, { recursive: true, force: true })); + return directory; +} + +function initializeSqlCheckout(t, version) { + const directory = temporaryDirectory(t); + fs.writeFileSync( + path.join(directory, 'build.gradle'), + `opensearch_version = System.getProperty("opensearch.version", "${version}")\n` + ); + assert.equal(spawnSync('git', ['init', '--quiet'], { cwd: directory }).status, 0); + assert.equal(spawnSync('git', ['add', 'build.gradle'], { cwd: directory }).status, 0); + const commit = spawnSync( + 'git', + [ + '-c', + 'user.name=Workflow Test', + '-c', + 'user.email=workflow-test@example.com', + 'commit', + '--quiet', + '-m', + 'fixture', + ], + { cwd: directory, encoding: 'utf8' } + ); + assert.equal(commit.status, 0, commit.stderr); + return directory; +} + +function initializeOsdCheckout(t, version = '3.7.0') { + const directory = temporaryDirectory(t); + fs.writeFileSync( + path.join(directory, 'package.json'), + `${JSON.stringify({ version })}\n` + ); + assert.equal(spawnSync('git', ['init', '--quiet'], { cwd: directory }).status, 0); + assert.equal(spawnSync('git', ['add', 'package.json'], { cwd: directory }).status, 0); + const commit = spawnSync( + 'git', + [ + '-c', + 'user.name=Workflow Test', + '-c', + 'user.email=workflow-test@example.com', + 'commit', + '--quiet', + '-m', + 'fixture', + ], + { cwd: directory, encoding: 'utf8' } + ); + assert.equal(commit.status, 0, commit.stderr); + return directory; +} + +function gitHead(directory) { + const result = spawnSync('git', ['rev-parse', 'HEAD'], { + cwd: directory, + encoding: 'utf8', + }); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); +} + +function readOutputs(file) { + return Object.fromEntries( + fs + .readFileSync(file, 'utf8') + .trim() + .split('\n') + .map((line) => { + const separator = line.indexOf('='); + return [line.slice(0, separator), line.slice(separator + 1)]; + }) + ); +} + +function resolveTarget( + t, + { + version = '4.2.1-SNAPSHOT', + event = 'pull_request', + target = 'main', + osdRepo = '', + osdRef = '', + bypass = '', + } = {} +) { + const directory = initializeSqlCheckout(t, version); + const output = path.join(directory, 'github-output'); + const result = spawnSync('/bin/bash', ['-c', stepScript('Resolve SQL target and OSD pair')], { + cwd: directory, + encoding: 'utf8', + env: { + ...process.env, + EVENT_NAME: event, + TARGET_BRANCH: target, + SQL_HEAD_SHA: 'pull-request-head', + REQUESTED_OSD_REPO: osdRepo, + REQUESTED_OSD_REF: osdRef, + REQUESTED_BYPASS: bypass, + GITHUB_OUTPUT: output, + }, + }); + return { + result, + outputs: fs.existsSync(output) ? readOutputs(output) : {}, + }; +} + +function probeCapability(t, extension) { + const directory = temporaryDirectory(t); + const output = path.join(directory, 'github-output'); + if (extension) { + const module = path.join( + directory, + '.ci', + 'OpenSearch-Dashboards', + 'src', + 'plugins', + 'data', + 'public', + 'antlr', + 'opensearch_ppl', + `headless_ppl_lint.${extension}` + ); + fs.mkdirSync(path.dirname(module), { recursive: true }); + fs.writeFileSync(module, ''); + } + const result = spawnSync('/bin/bash', ['-c', stepScript('Detect headless grammar capability')], { + cwd: directory, + encoding: 'utf8', + env: { ...process.env, GITHUB_OUTPUT: output }, + }); + assert.equal(result.status, 0, result.stderr); + return readOutputs(output).available; +} + +test('workflow has focused triggers, read-only permissions, and no fixed product version', () => { + assert.match(SOURCE, /^ pull_request:$/m); + assert.match(SOURCE, /^ push:$/m); + assert.match(SOURCE, /^ workflow_dispatch:$/m); + assert.equal(SOURCE.match(/^ - '\[0-9\]\+\.\[0-9\]\+'$/gm)?.length, 2); + assert.equal(SOURCE.match(/^ - main$/gm)?.length, 2); + assert.match(SOURCE, /^permissions:\n contents: read$/m); + assert.doesNotMatch(SOURCE, /^\s+schedule:$/m); + assert.doesNotMatch(SOURCE, /\b(?:secrets|vars)\./); + assert.doesNotMatch(SOURCE, /compiled-version|latestEligibleGa|release-tags/); + assert.equal( + SOURCE.match(/plugin\/src\/main\/java\/org\/opensearch\/sql\/plugin\/SQLPlugin\.java/g) + ?.length, + 2 + ); + + const fixedProductVersions = [ + ...SOURCE.matchAll(/(?:^|[^0-9.])([0-9]+\.[0-9]+\.[0-9]+)(?![0-9.])/gm), + ].map((match) => match[1]); + assert.deepEqual(fixedProductVersions, []); +}); + +test('skip path uses the adapter before bootstrap and supports TypeScript or JavaScript modules', (t) => { + const capability = SOURCE.indexOf(' - name: Detect headless grammar capability'); + const skip = SOURCE.indexOf(' - name: Record unsupported paired branch with adapter'); + const java = SOURCE.indexOf(' - name: Set up JDK 21'); + const capture = SOURCE.indexOf(' - name: Capture candidate runtime grammar'); + const bootstrap = SOURCE.indexOf(' - name: Bootstrap OpenSearch Dashboards'); + assert.ok(capability < skip); + assert.ok(skip < java); + assert.ok(skip < capture); + assert.ok(skip < bootstrap); + + const skipScript = stepScript('Record unsupported paired branch with adapter'); + assert.match( + skipScript, + /^node "\$GITHUB_WORKSPACE\/scripts\/ppl-lint\/validate-osd-grammar\.mjs"/m + ); + assert.doesNotMatch(skipScript, /node -r /); + assert.match(skipScript, /--grammar "\$GITHUB_WORKSPACE\/ppl-grammar-bundle\.json"/); + assert.match(skipScript, /--summary "\$GITHUB_STEP_SUMMARY"/); + + assert.equal(probeCapability(t, 'ts'), 'true'); + assert.equal(probeCapability(t, 'js'), 'true'); + assert.equal(probeCapability(t), 'false'); +}); + +test('pre-adapter failure fallback writes the structural report contract without target metadata', (t) => { + const fallback = stepScript('Record pre-report failure'); + assert.match(fallback, /ppl-lint-grammar-compatibility-report\.json/); + assert.match(fallback, /status: "error"/); + assert.match(fallback, /error: \$error/); + assert.match(fallback, /manualOverride: \(\.osd\.override \/\/ false\)/); + assert.match(fallback, /caseCounts: \{selected: 0, passed: 0, failed: 0\}/); + assert.match(fallback, /cases: \[\]/); + assert.match(fallback, /failures: \[\]/); + + const fallbackIndex = SOURCE.indexOf(' - name: Record pre-report failure'); + const uploadIndex = SOURCE.indexOf(' - name: Upload grammar compatibility artifacts'); + const enforceIndex = SOURCE.indexOf(' - name: Enforce compatibility result'); + assert.ok(fallbackIndex < uploadIndex); + assert.ok(uploadIndex < enforceIndex); + + const directory = temporaryDirectory(t); + const summary = path.join(directory, 'summary.md'); + const result = spawnSync('/bin/bash', ['-c', fallback], { + cwd: directory, + encoding: 'utf8', + env: { ...process.env, GITHUB_STEP_SUMMARY: summary }, + }); + assert.equal(result.status, 0, result.stderr); + const report = JSON.parse( + fs.readFileSync( + path.join(directory, 'ppl-lint-grammar-compatibility-report.json'), + 'utf8' + ) + ); + assert.equal(report.status, 'error'); + assert.match(report.error, /before the compatibility adapter/); + assert.deepEqual(report.rules, { selected: 0, passed: 0, failed: 0 }); + assert.equal('sql' in report, false); +}); + +test('extracted resolver pairs main and exact release targets', (t) => { + const main = resolveTarget(t); + assert.equal(main.result.status, 0, main.result.stderr); + assert.equal(main.outputs.target_branch, 'main'); + assert.equal(main.outputs.sql_version, '4.2.1'); + assert.equal(main.outputs.osd_repo, 'opensearch-project/OpenSearch-Dashboards'); + assert.equal(main.outputs.osd_ref, 'main'); + + const release = resolveTarget(t, { target: '4.2' }); + assert.equal(release.result.status, 0, release.result.stderr); + assert.equal(release.outputs.sql_release_line, '4.2'); + assert.equal(release.outputs.osd_ref, '4.2'); +}); + +test('extracted resolver rejects mismatches and limits overrides to dispatch', (t) => { + const mismatch = resolveTarget(t, { target: '4.1' }); + assert.notEqual(mismatch.result.status, 0); + assert.match(mismatch.result.stdout, /does not match target branch 4\.1/); + + const pushOverride = resolveTarget(t, { + event: 'push', + osdRepo: 'example/OpenSearch-Dashboards', + }); + assert.notEqual(pushOverride.result.status, 0); + assert.match(pushOverride.result.stdout, /allowed only for workflow_dispatch/); + + const dispatch = resolveTarget(t, { + event: 'workflow_dispatch', + target: '4.1', + osdRepo: 'example/OpenSearch-Dashboards', + osdRef: 'candidate', + bypass: 'true', + }); + assert.equal(dispatch.result.status, 0, dispatch.result.stderr); + assert.equal(dispatch.outputs.osd_repo, 'example/OpenSearch-Dashboards'); + assert.equal(dispatch.outputs.osd_ref, 'candidate'); + assert.equal(dispatch.outputs.osd_override, 'true'); + assert.equal(dispatch.outputs.release_line_bypass, 'true'); + + const featureBranch = resolveTarget(t, { target: 'feature/test' }); + assert.notEqual(featureBranch.result.status, 0); + assert.match(featureBranch.result.stdout, /must be main or an exact X\.Y release branch/); + + const malformedVersion = resolveTarget(t, { version: '4.2', target: 'main' }); + assert.notEqual(malformedVersion.result.status, 0); + assert.match(malformedVersion.result.stdout, /Invalid SQL product version/); +}); + +test('wrapper reproduces a dispatch release-line bypass on exact checkouts', (t) => { + const directory = temporaryDirectory(t); + const osdRoot = initializeOsdCheckout(t); + const target = path.join(directory, 'target.json'); + const report = path.join(directory, 'report.json'); + const summary = path.join(directory, 'summary.md'); + const sqlVersionRaw = fs + .readFileSync(path.join(ROOT, 'build.gradle'), 'utf8') + .match(/opensearch_version = System\.getProperty\("opensearch\.version", "([^"]+)"\)/)[1]; + const sqlVersion = sqlVersionRaw.replace(/[-+].*$/, ''); + fs.writeFileSync( + target, + JSON.stringify({ + sql: { + sha: gitHead(ROOT), + targetBranch: '9.9', + versionRaw: sqlVersionRaw, + version: sqlVersion, + }, + osd: { + repository: 'local/OpenSearch-Dashboards', + ref: '9.9', + sha: gitHead(osdRoot), + version: '3.7.0', + }, + releaseLineValidationBypassed: true, + }) + ); + + const result = spawnSync( + WRAPPER, + ['--target', target, '--osd-root', osdRoot, '--report', report, '--summary', summary], + { cwd: ROOT, encoding: 'utf8' } + ); + assert.equal(result.status, 0, result.stderr); + const output = JSON.parse(fs.readFileSync(report, 'utf8')); + assert.equal(output.status, 'skipped'); + assert.equal(output.releaseLineValidationBypassed, true); +}); + +test('wrapper refuses to label an unrelated local OSD checkout as the paired ref', (t) => { + const directory = temporaryDirectory(t); + const osdRoot = initializeOsdCheckout(t); + assert.equal( + spawnSync('git', ['checkout', '--quiet', '-b', 'feature'], { cwd: osdRoot }).status, + 0 + ); + const diverge = spawnSync( + 'git', + [ + '-c', + 'user.name=Workflow Test', + '-c', + 'user.email=workflow-test@example.com', + 'commit', + '--quiet', + '--allow-empty', + '-m', + 'diverge', + ], + { cwd: osdRoot, encoding: 'utf8' } + ); + assert.equal(diverge.status, 0, diverge.stderr); + const result = spawnSync( + WRAPPER, + [ + '--osd-root', + osdRoot, + '--target-branch', + 'main', + '--report', + path.join(directory, 'report.json'), + '--summary', + path.join(directory, 'summary.md'), + ], + { cwd: ROOT, encoding: 'utf8' } + ); + + assert.equal(result.status, 2); + assert.match(result.stderr, /OSD ref main resolves to .* not checkout/); +}); diff --git a/scripts/ppl-lint/grammar-cases.json b/scripts/ppl-lint/grammar-cases.json new file mode 100644 index 00000000000..7fb9d234c1c --- /dev/null +++ b/scripts/ppl-lint/grammar-cases.json @@ -0,0 +1,155 @@ +{ + "schemaVersion": 1, + "cases": [ + { + "id": "head-without-sort", + "ruleId": "head-without-sort", + "kind": "trigger", + "query": "source=accounts | head 5", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "head-with-sort-control", + "ruleId": "head-without-sort", + "kind": "control", + "query": "source=accounts | sort age | head 5", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "division-by-zero", + "ruleId": "division-by-zero", + "kind": "trigger", + "query": "source=accounts | eval ratio = balance / 0 | fields ratio", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "division-by-nonzero-control", + "ruleId": "division-by-zero", + "kind": "control", + "query": "source=accounts | eval ratio = balance / 2 | fields ratio", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "eventstats-rank", + "ruleId": "unsupported-window-function-in-eventstats", + "kind": "trigger", + "query": "source=accounts | eventstats rank() as rank_value", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "eventstats-avg-control", + "ruleId": "unsupported-window-function-in-eventstats", + "kind": "control", + "query": "source=accounts | eventstats avg(age) as avg_age", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "multisearch-single-subsearch", + "ruleId": "multisearch-min-subsearch", + "kind": "trigger", + "query": "multisearch [ search source=accounts ]", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "multisearch-two-subsearches-control", + "ruleId": "multisearch-min-subsearch", + "kind": "control", + "query": "multisearch [ search source=accounts ] [ search source=accounts ]", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "right-join-disabled", + "ruleId": "disabled-join-type", + "kind": "trigger", + "query": "source=accounts | join type=right left=l right=r on l.account_number=r.account_number accounts", + "expectedCount": 1, + "context": { + "isCalcite": true, + "settings": { "allJoinTypesAllowed": false } + } + }, + { + "id": "inner-join-control", + "ruleId": "disabled-join-type", + "kind": "control", + "query": "source=accounts | join left=l right=r on l.account_number=r.account_number accounts", + "expectedCount": 0, + "context": { + "isCalcite": true, + "settings": { "allJoinTypesAllowed": false } + } + }, + { + "id": "dedup-consecutive-true", + "ruleId": "dedup-consecutive-unsupported", + "kind": "trigger", + "query": "source=accounts | dedup firstname consecutive=true", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "dedup-plain-control", + "ruleId": "dedup-consecutive-unsupported", + "kind": "control", + "query": "source=accounts | dedup firstname", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "union-single-dataset", + "ruleId": "union-min-datasets", + "kind": "trigger", + "query": "union [ source=accounts ]", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "union-two-datasets-control", + "ruleId": "union-min-datasets", + "kind": "control", + "query": "union [ source=accounts ] [ source=accounts ]", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "replace-wildcard-count-mismatch", + "ruleId": "replace-wildcard-asymmetry", + "kind": "trigger", + "query": "source=accounts | replace \"*_a\" with \"b_*_*\" in firstname", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "replace-symmetric-control", + "ruleId": "replace-wildcard-asymmetry", + "kind": "control", + "query": "source=accounts | replace \"*_a\" with \"b_*\" in firstname", + "expectedCount": 0, + "context": { "isCalcite": true } + }, + { + "id": "rex-capture-name-underscore", + "ruleId": "invalid-capture-group-name", + "kind": "trigger", + "query": "source=accounts | rex field=email \"(?[^@]+)@(?.+)\"", + "expectedCount": 1, + "context": { "isCalcite": true } + }, + { + "id": "rex-capture-name-alphanumeric-control", + "ruleId": "invalid-capture-group-name", + "kind": "control", + "query": "source=accounts | rex field=email \"(?[^@]+)@(?.+)\"", + "expectedCount": 0, + "context": { "isCalcite": true } + } + ] +} diff --git a/scripts/ppl-lint/validate-osd-grammar.mjs b/scripts/ppl-lint/validate-osd-grammar.mjs new file mode 100644 index 00000000000..815f171e620 --- /dev/null +++ b/scripts/ppl-lint/validate-osd-grammar.mjs @@ -0,0 +1,463 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { createRequire } from 'node:module'; +import { fileURLToPath } from 'node:url'; + +const HEADLESS = 'src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint'; +const CATALOGS = [ + 'packages/osd-monaco/ppl-lint', + 'packages/osd-monaco/src/ppl/lint/catalog', +]; +const SKIP_REASON = 'osd-headless-grammar-api-unavailable'; +const OPTIONS = ['grammar', 'cases', 'target', 'osd-root', 'osd-sha', 'report', 'summary']; + +class StructuralError extends Error {} + +function fail(message) { + throw new StructuralError(message); +} + +function parseArgs(argv) { + if (argv.length === 1 && argv[0] === '--help') { + return { help: true }; + } + const args = {}; + for (let i = 0; i < argv.length; i += 2) { + const flag = argv[i]; + const value = argv[i + 1]; + const key = flag?.startsWith('--') ? flag.slice(2) : ''; + if (!OPTIONS.includes(key) || !value || value.startsWith('--')) { + fail(`Invalid CLI argument near ${JSON.stringify(flag)}.`); + } + if (args[key]) fail(`Duplicate CLI argument ${flag}.`); + args[key] = value; + } + const missing = OPTIONS.filter((key) => !args[key]); + if (missing.length) fail(`Missing CLI argument(s): ${missing.map((key) => `--${key}`).join(', ')}.`); + return args; +} + +function readJson(file, label) { + try { + return JSON.parse(fs.readFileSync(file, 'utf8')); + } catch (error) { + fail(`Could not read or parse ${label} ${file}: ${error.message}`); + } +} + +function object(value, label) { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + fail(`${label} must be an object.`); + } + return value; +} + +function string(value, label) { + if (typeof value !== 'string' || !value.trim()) fail(`${label} must be a non-empty string.`); + return value; +} + +function version(value, label) { + const match = /^(\d+)\.(\d+)\.(\d+)(?:[-+][0-9A-Za-z][0-9A-Za-z.-]*)?$/.exec( + string(value, label) + ); + if (!match) fail(`${label} must be a complete product version.`); + return `${Number(match[1])}.${Number(match[2])}.${Number(match[3])}`; +} + +function loadTarget(file, osdSha) { + const raw = object(readJson(file, 'target'), 'target'); + const sql = object(raw.sql, 'target.sql'); + const osd = object(raw.osd, 'target.osd'); + if ( + raw.releaseLineValidationBypassed !== undefined && + typeof raw.releaseLineValidationBypassed !== 'boolean' + ) { + fail('target.releaseLineValidationBypassed must be a boolean.'); + } + const sqlVersion = version(sql.version, 'target.sql.version'); + if (sql.versionRaw && version(sql.versionRaw, 'target.sql.versionRaw') !== sqlVersion) { + fail(`target.sql.versionRaw does not normalize to ${sqlVersion}.`); + } + if (osd.sha && osd.sha !== osdSha) fail('target.osd.sha does not match --osd-sha.'); + + return { + sql: { + sha: string(sql.sha, 'target.sql.sha'), + ...(sql.headSha ? { headSha: string(sql.headSha, 'target.sql.headSha') } : {}), + targetBranch: string(sql.targetBranch, 'target.sql.targetBranch'), + ...(sql.versionRaw ? { versionRaw: sql.versionRaw } : {}), + version: sqlVersion, + }, + osd: { + repository: string(osd.repository, 'target.osd.repository'), + ref: string(osd.ref, 'target.osd.ref'), + sha: string(osdSha, '--osd-sha'), + version: version(osd.version, 'target.osd.version'), + }, + manualOverride: raw.manualOverride === true || osd.override === true, + releaseLineValidationBypassed: raw.releaseLineValidationBypassed === true, + }; +} + +function validatePairing(target) { + const branch = target.sql.targetBranch; + const release = /^\d+\.\d+$/.test(branch); + if (branch !== 'main' && !release) fail(`Unsupported SQL target branch ${JSON.stringify(branch)}.`); + if (!target.manualOverride && target.osd.ref !== branch) { + fail(`OSD ref ${JSON.stringify(target.osd.ref)} must match SQL target branch ${JSON.stringify(branch)}.`); + } + if (release && !target.releaseLineValidationBypassed) { + const sqlLine = target.sql.version.split('.').slice(0, 2).join('.'); + const osdLine = target.osd.version.split('.').slice(0, 2).join('.'); + if (sqlLine !== branch || osdLine !== branch) { + fail( + `Exact release target ${branch} requires SQL and OSD ${branch}.x versions; ` + + `got SQL ${target.sql.version} and OSD ${target.osd.version}.` + ); + } + } +} + +function modulePath(root, name) { + const base = path.join(root, name); + return [base, `${base}.js`, `${base}.ts`].find((candidate) => fs.existsSync(candidate)); +} + +function exportsOf(module) { + return module?.default && typeof module.default === 'object' + ? { ...module.default, ...module } + : module; +} + +function loadApi(osdRoot) { + const requireFromOsd = createRequire(path.join(osdRoot, 'noop.js')); + let headless; + try { + headless = exportsOf(requireFromOsd(path.join(osdRoot, HEADLESS))); + } catch (error) { + fail(`OSD advertises ${HEADLESS}, but import failed: ${error.message}`); + } + if ( + typeof headless?.deserializeBundleOrThrow !== 'function' || + typeof headless?.lintQueryWithBundle !== 'function' + ) { + fail(`${HEADLESS} must export deserializeBundleOrThrow and lintQueryWithBundle.`); + } + + let catalogModule; + const errors = []; + for (const name of CATALOGS) { + if (!modulePath(osdRoot, name)) continue; + try { + const candidate = exportsOf(requireFromOsd(path.join(osdRoot, name))); + if (typeof candidate?.getBundledCatalog === 'function') { + catalogModule = candidate; + break; + } + errors.push(`${name} has no getBundledCatalog export`); + } catch (error) { + errors.push(`${name}: ${error.message}`); + } + } + if (!catalogModule) fail(`Could not load OSD catalog${errors.length ? `: ${errors.join('; ')}` : '.'}`); + + let catalog; + try { + catalog = catalogModule.getBundledCatalog(); + } catch (error) { + fail(`getBundledCatalog failed: ${error.message}`); + } + if (!Array.isArray(catalog) || !catalog.length) fail('OSD catalog must be a non-empty array.'); + const catalogIds = catalog.map((entry, index) => + string(object(entry, `catalog[${index}]`).id, `catalog[${index}].id`) + ); + if (new Set(catalogIds).size !== catalogIds.length) fail('OSD catalog has duplicate rule IDs.'); + + return { + deserialize: headless.deserializeBundleOrThrow, + lint: headless.lintQueryWithBundle, + buildTree: typeof headless.buildRuntimeTree === 'function' ? headless.buildRuntimeTree : undefined, + catalogIds, + }; +} + +function loadGrammar(file, deserialize) { + const bundle = object(readJson(file, 'grammar bundle'), 'grammar bundle'); + if (typeof bundle.grammarHash !== 'string' || !/^sha256:[0-9a-f]{64}$/i.test(bundle.grammarHash)) { + fail('grammarHash must be "sha256:" followed by 64 hexadecimal characters.'); + } + let grammar; + try { + grammar = deserialize(bundle); + } catch (error) { + fail(`Could not deserialize candidate grammar bundle: ${error.message}`); + } + if (!grammar) fail('deserializeBundleOrThrow returned no grammar.'); + if (grammar.grammarHash && grammar.grammarHash !== bundle.grammarHash) { + fail('Deserialized grammar hash does not match the bundle.'); + } + return { bundle, grammar }; +} + +function loadCases(file, catalogIds) { + const document = readJson(file, 'grammar cases'); + const rawCases = Array.isArray(document) ? document : object(document, 'case document').cases; + if (!Array.isArray(rawCases) || !rawCases.length) fail('At least one grammar case is required.'); + const knownRules = new Set(catalogIds); + const ids = new Set(); + const cases = rawCases.map((rawCase, index) => { + const candidate = object(rawCase, `case[${index}]`); + const id = string(candidate.id, `case[${index}].id`); + const ruleId = string(candidate.ruleId, `case ${id}.ruleId`); + if (ids.has(id)) fail(`Duplicate case ID ${JSON.stringify(id)}.`); + if (!knownRules.has(ruleId)) fail(`Case ${JSON.stringify(id)} names missing OSD rule ${JSON.stringify(ruleId)}.`); + ids.add(id); + if (!['trigger', 'control'].includes(candidate.kind)) fail(`Case ${id} has invalid kind.`); + if (!Number.isInteger(candidate.expectedCount) || candidate.expectedCount < 0) { + fail(`Case ${id} expectedCount must be a non-negative integer.`); + } + if (candidate.kind === 'trigger' && candidate.expectedCount === 0) { + fail(`Trigger case ${id} must expect a diagnostic.`); + } + if (candidate.kind === 'control' && candidate.expectedCount !== 0) { + fail(`Control case ${id} must expect zero diagnostics.`); + } + const context = candidate.context === undefined ? {} : object(candidate.context, `case ${id}.context`); + if (['overrides', 'dataSourceVersion', 'knownVersion'].some((key) => key in context)) { + fail(`Case ${id} context sets an adapter-owned field.`); + } + return { + id, + ruleId, + kind: candidate.kind, + query: string(candidate.query, `case ${id}.query`), + expectedCount: candidate.expectedCount, + context, + }; + }); + + for (const ruleId of new Set(cases.map((entry) => entry.ruleId))) { + const kinds = new Set(cases.filter((entry) => entry.ruleId === ruleId).map((entry) => entry.kind)); + if (!kinds.has('trigger') || !kinds.has('control')) { + fail(`Selected rule ${JSON.stringify(ruleId)} must have trigger and control cases.`); + } + } + return cases; +} + +function contextFor(grammarCase, target, catalogIds) { + const context = { ...grammarCase.context }; + if (Array.isArray(context.fields)) context.fields = new Set(context.fields); + if (context.typeMap && !Array.isArray(context.typeMap)) { + context.typeMap = new Map(Object.entries(context.typeMap)); + } + if (Array.isArray(context.disabledObjectFields)) { + context.disabledObjectFields = new Set(context.disabledObjectFields); + } + return { + ...context, + isCalcite: context.isCalcite !== false, + dataSourceVersion: target.sql.version, + knownVersion: target.sql.version, + overrides: Object.fromEntries(catalogIds.map((id) => [id, { enabled: id === grammarCase.ruleId }])), + }; +} + +function hasParseTree(result) { + if (!result) return false; + return typeof result === 'object' && 'tree' in result ? Boolean(result.tree) : true; +} + +function parseTreeOf(result) { + return typeof result === 'object' && result && 'tree' in result ? result.tree : result; +} + +function hasErrorNode(tree) { + const pending = [tree]; + while (pending.length) { + const node = pending.pop(); + if (!node || typeof node !== 'object') continue; + if (node.constructor?.name === 'ErrorNode') return true; + if (Array.isArray(node.children)) pending.push(...node.children); + } + return false; +} + +function executeCases(api, grammar, cases, target) { + return cases.map((grammarCase) => { + const result = { + caseId: grammarCase.id, + ruleId: grammarCase.ruleId, + kind: grammarCase.kind, + query: grammarCase.query, + expectedCount: grammarCase.expectedCount, + }; + try { + if (api.buildTree) { + const parse = api.buildTree(grammarCase.query, grammar); + if (!hasParseTree(parse)) throw new Error('candidate parser produced no parse tree'); + if (hasErrorNode(parseTreeOf(parse))) { + throw new Error('candidate parser recovered from a syntax error'); + } + } + const lint = api.lint(grammarCase.query, grammar, contextFor(grammarCase, target, api.catalogIds)); + if (!Array.isArray(lint?.diagnostics)) throw new Error('lintQueryWithBundle returned no diagnostics array'); + result.actualCount = lint.diagnostics.filter((entry) => entry?.ruleId === grammarCase.ruleId).length; + result.parseTreeCheck = api.buildTree + ? 'verified' + : grammarCase.kind === 'trigger' && result.actualCount > 0 + ? 'inferred-from-target-diagnostic' + : 'unavailable'; + const countMatches = result.actualCount === result.expectedCount; + const treeVerified = result.parseTreeCheck !== 'unavailable'; + result.status = countMatches && treeVerified ? 'passed' : 'failed'; + if (!treeVerified) { + result.error = 'buildRuntimeTree is not exported; control parse tree cannot be verified'; + } + } catch (error) { + result.actualCount = null; + result.parseTreeCheck = api.buildTree ? 'failed' : 'unavailable'; + result.status = 'failed'; + result.error = error instanceof Error ? error.message : String(error); + } + console[result.status === 'passed' ? 'log' : 'error']( + `[ppl-lint-grammar] ${result.status.toUpperCase()} ${result.ruleId}/${result.caseId}: ` + + `expected ${result.expectedCount}, got ${result.actualCount ?? 'error'}` + ); + return result; + }); +} + +function counts(results) { + const passed = results.filter((entry) => entry.status === 'passed').length; + return { selected: results.length, passed, failed: results.length - passed }; +} + +function makeReport(target, grammarHash, cases) { + const ruleResults = [...new Set(cases.map((entry) => entry.ruleId))].map((ruleId) => ({ + status: cases.filter((entry) => entry.ruleId === ruleId).every((entry) => entry.status === 'passed') + ? 'passed' + : 'failed', + })); + const failures = cases.filter((entry) => entry.status === 'failed').map((entry) => ({ + ruleId: entry.ruleId, + caseId: entry.caseId, + query: entry.query, + expectedCount: entry.expectedCount, + actualCount: entry.actualCount, + ...(entry.error ? { error: entry.error } : {}), + })); + return { + schemaVersion: 1, + status: failures.length ? 'failed' : 'passed', + sql: target.sql, + osd: target.osd, + manualOverride: target.manualOverride, + releaseLineValidationBypassed: target.releaseLineValidationBypassed, + grammarHash, + rules: counts(ruleResults), + caseCounts: counts(cases), + cases, + failures, + }; +} + +function emptyReport(status, target, extra = {}) { + return { + schemaVersion: 1, + status, + ...extra, + ...(target + ? { + sql: target.sql, + osd: target.osd, + manualOverride: target.manualOverride, + releaseLineValidationBypassed: target.releaseLineValidationBypassed, + } + : {}), + rules: { selected: 0, passed: 0, failed: 0 }, + caseCounts: { selected: 0, passed: 0, failed: 0 }, + cases: [], + failures: [], + }; +} + +function summary(report) { + const lines = [`## PPL grammar compatibility`, '', `- Status: **${report.status}**`]; + if (report.skipReason) lines.push(`- Skip reason: \`${report.skipReason}\``); + if (report.sql) lines.push(`- SQL: \`${report.sql.targetBranch}\` / \`${report.sql.version}\` / \`${report.sql.sha}\``); + if (report.osd) lines.push(`- OSD: \`${report.osd.ref}\` / \`${report.osd.version}\` / \`${report.osd.sha}\``); + if (typeof report.manualOverride === 'boolean') { + lines.push(`- Manual OSD override: \`${report.manualOverride}\``); + } + if (typeof report.releaseLineValidationBypassed === 'boolean') { + lines.push(`- Release-line validation bypassed: \`${report.releaseLineValidationBypassed}\``); + } + if (report.grammarHash) lines.push(`- Grammar: \`${report.grammarHash}\``); + lines.push(`- Rules: ${report.rules.selected} selected, ${report.rules.passed} passed, ${report.rules.failed} failed`); + if (report.error) lines.push('', `Structural error: ${report.error.replaceAll('\n', ' ')}`); + if (report.failures.length) { + lines.push('', '| Rule | Case | Expected | Actual | Error |', '| --- | --- | ---: | ---: | --- |'); + for (const failure of report.failures) { + lines.push( + `| ${failure.ruleId} | ${failure.caseId} | ${failure.expectedCount} | ` + + `${failure.actualCount ?? 'n/a'} | ${(failure.error || '').replaceAll('|', '\\|')} |` + ); + } + } + return `${lines.join('\n')}\n`; +} + +function writeOutputs(args, report) { + fs.mkdirSync(path.dirname(path.resolve(args.report)), { recursive: true }); + fs.writeFileSync(args.report, `${JSON.stringify(report, null, 2)}\n`); + fs.mkdirSync(path.dirname(path.resolve(args.summary)), { recursive: true }); + fs.appendFileSync(args.summary, summary(report)); +} + +export function run(argv = process.argv.slice(2)) { + let args; + let target; + try { + args = parseArgs(argv); + if (args.help) { + console.log('See --grammar, --cases, --target, --osd-root, --osd-sha, --report, and --summary.'); + return 0; + } + target = loadTarget(args.target, args['osd-sha']); + validatePairing(target); + const osdRoot = path.resolve(args['osd-root']); + if (!fs.existsSync(osdRoot) || !fs.statSync(osdRoot).isDirectory()) fail(`Invalid OSD root ${osdRoot}.`); + if (!modulePath(osdRoot, HEADLESS)) { + writeOutputs(args, emptyReport('skipped', target, { skipReason: SKIP_REASON })); + return 0; + } + const api = loadApi(osdRoot); + const { bundle, grammar } = loadGrammar(args.grammar, api.deserialize); + const cases = loadCases(args.cases, api.catalogIds); + const report = makeReport(target, bundle.grammarHash, executeCases(api, grammar, cases, target)); + writeOutputs(args, report); + return report.status === 'passed' ? 0 : 1; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(`[ppl-lint-grammar] ERROR: ${message}`); + if (args?.report && args?.summary) { + try { + writeOutputs(args, emptyReport('error', target, { error: message })); + } catch (writeError) { + console.error(`[ppl-lint-grammar] ERROR: could not write artifacts: ${writeError.message}`); + } + } + return 2; + } +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = run(); +} From 6298d4bf3c82d02b48f24f2b0d5f7eaa85314545 Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Thu, 6 Aug 2026 12:36:12 -0700 Subject: [PATCH 2/5] fix(ci): harden PPL grammar compatibility validation Signed-off-by: Hanyu Wei --- .../ppl-lint-grammar-compatibility.yml | 167 ++---- docs/dev/ppl-lint-grammar-compatibility-ci.md | 179 +++---- ppl/build.gradle | 17 + .../PPLGrammarBundleExporter.java | 119 +++++ .../PPLGrammarBundleExporterTest.java | 162 ++++++ scripts/ppl-lint-rule-validation.sh | 220 ++------ scripts/ppl-lint/README.md | 180 +++---- .../__tests__/validate-osd-grammar.test.mjs | 489 +++++++++++------- .../__tests__/workflow-pairing.test.mjs | 144 +++--- scripts/ppl-lint/grammar-cases.json | 188 ++++++- scripts/ppl-lint/validate-osd-grammar.mjs | 327 ++++++++---- 11 files changed, 1363 insertions(+), 829 deletions(-) create mode 100644 ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporter.java create mode 100644 ppl/src/test/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporterTest.java diff --git a/.github/workflows/ppl-lint-grammar-compatibility.yml b/.github/workflows/ppl-lint-grammar-compatibility.yml index 401c7edfe45..eb4c86e767a 100644 --- a/.github/workflows/ppl-lint-grammar-compatibility.yml +++ b/.github/workflows/ppl-lint-grammar-compatibility.yml @@ -7,53 +7,36 @@ on: - '[0-9]+.[0-9]+' paths: - build.gradle - - settings.gradle - ppl/build.gradle - ppl/src/main/antlr/OpenSearchPPLLexer.g4 - ppl/src/main/antlr/OpenSearchPPLParser.g4 - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/GrammarBundle.java - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleBuilder.java - - plugin/build.gradle - - plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java - - plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLGrammarAction.java + - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporter.java - scripts/ppl-lint/** - scripts/ppl-lint-rule-validation.sh - .github/workflows/ppl-lint-grammar-compatibility.yml - - docs/dev/ppl-lint-grammar-compatibility-ci*.md push: branches: - main - '[0-9]+.[0-9]+' paths: - build.gradle - - settings.gradle - ppl/build.gradle - ppl/src/main/antlr/OpenSearchPPLLexer.g4 - ppl/src/main/antlr/OpenSearchPPLParser.g4 - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/GrammarBundle.java - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleBuilder.java - - plugin/build.gradle - - plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java - - plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLGrammarAction.java + - ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporter.java - scripts/ppl-lint/** - scripts/ppl-lint-rule-validation.sh - .github/workflows/ppl-lint-grammar-compatibility.yml - - docs/dev/ppl-lint-grammar-compatibility-ci*.md workflow_dispatch: inputs: - osd_repo: - description: Optional OpenSearch Dashboards repository override. - required: false - type: string osd_ref: - description: Optional OpenSearch Dashboards ref override. + description: Optional ref in opensearch-project/OpenSearch-Dashboards. required: false type: string - allow_release_line_mismatch: - description: Development-only bypass for exact release-line validation. - required: false - default: false - type: boolean permissions: contents: read @@ -82,9 +65,7 @@ jobs: EVENT_NAME: ${{ github.event_name }} TARGET_BRANCH: ${{ github.base_ref || github.ref_name }} SQL_HEAD_SHA: ${{ github.event.pull_request.head.sha }} - REQUESTED_OSD_REPO: ${{ inputs.osd_repo }} REQUESTED_OSD_REF: ${{ inputs.osd_ref }} - REQUESTED_BYPASS: ${{ inputs.allow_release_line_mismatch }} run: | set -euo pipefail sql_raw=$(sed -nE 's/.*opensearch_version = System\.getProperty\("opensearch\.version", "([^"]+)"\).*/\1/p' build.gradle) @@ -92,13 +73,12 @@ jobs: { echo "::error::Invalid SQL product version: $sql_raw"; exit 1; } sql_version="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}.${BASH_REMATCH[3]}" sql_line="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}" - bypass=$([[ "$EVENT_NAME" == workflow_dispatch && "$REQUESTED_BYPASS" == true ]] && echo true || echo false) case "$TARGET_BRANCH" in main) canonical_ref=main ;; *) [[ "$TARGET_BRANCH" =~ ^[0-9]+\.[0-9]+$ ]] || { echo "::error::Target branch must be main or an exact X.Y release branch"; exit 1; } - [[ "$TARGET_BRANCH" == "$sql_line" || "$bypass" == true ]] || + [[ "$TARGET_BRANCH" == "$sql_line" ]] || { echo "::error::SQL $sql_raw does not match target branch $TARGET_BRANCH"; exit 1; } canonical_ref="$TARGET_BRANCH" ;; @@ -107,16 +87,15 @@ jobs: osd_repo="$canonical_repo" osd_ref="$canonical_ref" if [[ "$EVENT_NAME" == workflow_dispatch ]]; then - osd_repo="${REQUESTED_OSD_REPO:-$osd_repo}" osd_ref="${REQUESTED_OSD_REF:-$osd_ref}" - elif [[ -n "$REQUESTED_OSD_REPO$REQUESTED_OSD_REF" || "$REQUESTED_BYPASS" == true ]]; then - echo "::error::OSD overrides are allowed only for workflow_dispatch" + elif [[ -n "$REQUESTED_OSD_REF" ]]; then + echo "::error::OSD ref overrides are allowed only for workflow_dispatch" exit 1 fi - [[ "$osd_repo$osd_ref" != *$'\n'* && "$osd_repo$osd_ref" != *$'\r'* ]] || - { echo "::error::OSD overrides must be single-line values"; exit 1; } + [[ "$osd_ref" != *$'\n'* && "$osd_ref" != *$'\r'* ]] || + { echo "::error::OSD ref override must be a single-line value"; exit 1; } sql_sha=$(git rev-parse HEAD) - override=$([[ "$osd_repo" != "$canonical_repo" || "$osd_ref" != "$canonical_ref" ]] && echo true || echo false) + override=$([[ "$osd_ref" != "$canonical_ref" ]] && echo true || echo false) { echo "target_branch=$TARGET_BRANCH" echo "sql_version_raw=$sql_raw" @@ -127,7 +106,6 @@ jobs: echo "osd_repo=$osd_repo" echo "osd_ref=$osd_ref" echo "osd_override=$override" - echo "release_line_bypass=$bypass" } >> "$GITHUB_OUTPUT" - name: Checkout paired OpenSearch Dashboards @@ -166,7 +144,6 @@ jobs: OSD_REF: ${{ steps.target.outputs.osd_ref }} OSD_SHA: ${{ steps.osd.outputs.sha }} OSD_OVERRIDE: ${{ steps.target.outputs.osd_override }} - RELEASE_LINE_BYPASS: ${{ steps.target.outputs.release_line_bypass }} run: | set -euo pipefail yarn_version=$(node -e "process.stdout.write(require('./package.json').engines.yarn.match(/[0-9]+\.[0-9]+\.[0-9]+/)[0])") @@ -176,7 +153,7 @@ jobs: { echo "::error::Invalid OSD product version: $osd_raw"; exit 1; } osd_version="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}.${BASH_REMATCH[3]}" osd_line="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}" - if [[ "$TARGET_BRANCH" != main && "$RELEASE_LINE_BYPASS" != true ]]; then + if [[ "$TARGET_BRANCH" != main ]]; then [[ "$SQL_RELEASE_LINE" == "$TARGET_BRANCH" && "$osd_line" == "$TARGET_BRANCH" ]] || { echo "::error::Expected SQL and OSD versions on release line $TARGET_BRANCH"; exit 1; } fi @@ -186,7 +163,7 @@ jobs: --arg sqlLine "$SQL_RELEASE_LINE" --arg sqlSha "$SQL_SHA" --arg headSha "$SQL_HEAD_SHA" \ --arg osdRepo "$OSD_REPOSITORY" --arg osdRef "$OSD_REF" --arg osdSha "$OSD_SHA" \ --arg osdRaw "$osd_raw" --arg osdVersion "$osd_version" --arg osdLine "$osd_line" \ - --arg override "$OSD_OVERRIDE" --arg bypass "$RELEASE_LINE_BYPASS" \ + --arg override "$OSD_OVERRIDE" \ '{ schemaVersion: 1, sql: { @@ -197,92 +174,25 @@ jobs: repository: $osdRepo, ref: $osdRef, sha: $osdSha, versionRaw: $osdRaw, version: $osdVersion, releaseLine: $osdLine, override: ($override == "true") - }, - releaseLineValidationBypassed: ($bypass == "true") + } }' > "$GITHUB_WORKSPACE/resolved-target.json" echo "osd_version=$osd_version" >> "$GITHUB_OUTPUT" echo "osd_release_line=$osd_line" >> "$GITHUB_OUTPUT" - - name: Detect headless grammar capability - id: capability - run: | - set -euo pipefail - module=.ci/OpenSearch-Dashboards/src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint - if [[ -f "${module}.ts" || -f "${module}.js" ]]; then - echo "available=true" >> "$GITHUB_OUTPUT" - else - echo "available=false" >> "$GITHUB_OUTPUT" - fi - - - name: Record unsupported paired branch with adapter - if: ${{ steps.capability.outputs.available == 'false' }} - run: | - set -euo pipefail - node "$GITHUB_WORKSPACE/scripts/ppl-lint/validate-osd-grammar.mjs" \ - --grammar "$GITHUB_WORKSPACE/ppl-grammar-bundle.json" \ - --cases "$GITHUB_WORKSPACE/scripts/ppl-lint/grammar-cases.json" \ - --target "$GITHUB_WORKSPACE/resolved-target.json" \ - --osd-root "$GITHUB_WORKSPACE/.ci/OpenSearch-Dashboards" \ - --osd-sha "${{ steps.osd.outputs.sha }}" \ - --report "$GITHUB_WORKSPACE/ppl-lint-grammar-compatibility-report.json" \ - --summary "$GITHUB_STEP_SUMMARY" - - name: Set up JDK 21 - if: ${{ steps.capability.outputs.available == 'true' }} uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4 with: distribution: temurin java-version: 21 cache: gradle - - name: Capture candidate runtime grammar - if: ${{ steps.capability.outputs.available == 'true' }} - timeout-minutes: 25 + - name: Export candidate runtime grammar run: | set -euo pipefail - ./gradlew :opensearch-sql-plugin:run --no-daemon > ppl-grammar-cluster.log 2>&1 & - gradle_pid=$! - cleanup() { - kill "$gradle_pid" 2>/dev/null || true - wait "$gradle_pid" 2>/dev/null || true - } - trap cleanup EXIT - ready=false - for attempt in $(seq 1 120); do - if curl --silent --fail http://127.0.0.1:9200/_cluster/health > /dev/null; then - ready=true - break - fi - kill -0 "$gradle_pid" 2>/dev/null || - { echo "::error::Gradle run task stopped before cluster startup"; exit 1; } - echo "Waiting for candidate cluster (${attempt}/120)" - sleep 5 - done - [[ "$ready" == true ]] || { echo "::error::Candidate cluster did not become ready"; exit 1; } - code=$(curl --silent --show-error --output ppl-grammar-bundle.json --write-out '%{http_code}' \ - http://127.0.0.1:9200/_plugins/_ppl/_grammar) - [[ "$code" == 200 ]] || { echo "::error::Grammar endpoint returned HTTP $code"; exit 1; } - jq -e ' - type == "object" - and (.bundleVersion | type == "string" and length > 0) - and (.antlrVersion | type == "string" and length > 0) - and (.grammarHash | type == "string" and test("^sha256:[0-9a-f]{64}$")) - and (.lexerSerializedATN | type == "array" and length > 0) - and (.lexerRuleNames | type == "array" and length > 0) - and (.channelNames | type == "array" and length > 0) - and (.modeNames | type == "array" and length > 0) - and (.parserSerializedATN | type == "array" and length > 0) - and (.parserRuleNames | type == "array" and length > 0) - and (.startRuleIndex | type == "number") - and (.literalNames | type == "array" and length > 0 and any(.[]; . == null)) - and (.symbolicNames | type == "array" and length > 0 and any(.[]; . == null)) - and (.tokenDictionary | type == "object" and length > 0) - and (.ignoredTokens | type == "array") - and (.rulesToVisit | type == "array" and length > 0) - ' ppl-grammar-bundle.json > /dev/null + ./gradlew :ppl:exportPplGrammarBundle --no-daemon \ + "-PpplGrammarBundleOutput=$GITHUB_WORKSPACE/ppl-grammar-bundle.json" - name: Cache OSD Yarn dependencies - if: ${{ steps.capability.outputs.available == 'true' }} uses: actions/cache@0c907a75c2c80ebcb7f088228285e798b750cf8f # v4 with: path: ~/.cache/yarn @@ -290,13 +200,11 @@ jobs: restore-keys: ${{ runner.os }}-osd-yarn- - name: Bootstrap OpenSearch Dashboards - if: ${{ steps.capability.outputs.available == 'true' }} working-directory: .ci/OpenSearch-Dashboards run: yarn osd bootstrap --prefer-offline - name: Validate OSD linter against candidate grammar id: compatibility - if: ${{ steps.capability.outputs.available == 'true' }} working-directory: .ci/OpenSearch-Dashboards run: | set +e @@ -320,24 +228,48 @@ jobs: error='workflow failed before the compatibility adapter produced a report' if [[ -s resolved-target.json ]]; then jq --arg error "$error" '{ - schemaVersion: 1, + schemaVersion: 2, status: "error", error: $error, sql: .sql, osd: .osd, manualOverride: (.osd.override // false), - releaseLineValidationBypassed: (.releaseLineValidationBypassed // false), - rules: {selected: 0, passed: 0, failed: 0}, + releaseLineValidationBypassed: false, + coverage: { + catalogRuleIds: [], requiredRuleIds: [], coveredRuleIds: [], + excludedRuleIds: [], excludedRules: [], + missingRuleIds: [], unexpectedRuleIds: [], + counts: { + catalog: 0, required: 0, covered: 0, + excluded: 0, missing: 0, unexpected: 0 + } + }, + rules: { + catalog: 0, required: 0, excluded: 0, + selected: 0, passed: 0, failed: 0 + }, caseCounts: {selected: 0, passed: 0, failed: 0}, cases: [], failures: [] }' resolved-target.json > ppl-lint-grammar-compatibility-report.json else jq -n --arg error "$error" '{ - schemaVersion: 1, + schemaVersion: 2, status: "error", error: $error, - rules: {selected: 0, passed: 0, failed: 0}, + coverage: { + catalogRuleIds: [], requiredRuleIds: [], coveredRuleIds: [], + excludedRuleIds: [], excludedRules: [], + missingRuleIds: [], unexpectedRuleIds: [], + counts: { + catalog: 0, required: 0, covered: 0, + excluded: 0, missing: 0, unexpected: 0 + } + }, + rules: { + catalog: 0, required: 0, excluded: 0, + selected: 0, passed: 0, failed: 0 + }, caseCounts: {selected: 0, passed: 0, failed: 0}, cases: [], failures: [] @@ -365,25 +297,20 @@ jobs: osd-revision.txt ppl-lint-grammar-compatibility-report.json ppl-grammar-bundle.json - ppl-grammar-cluster.log if-no-files-found: warn - name: Enforce compatibility result if: ${{ always() }} env: - CAPABILITY_AVAILABLE: ${{ steps.capability.outputs.available }} COMPATIBILITY_EXIT: ${{ steps.compatibility.outputs.exit_code }} run: | set -euo pipefail - if [[ "$CAPABILITY_AVAILABLE" == false ]]; then - jq -e '.status == "skipped"' ppl-lint-grammar-compatibility-report.json > /dev/null - exit 0 - fi [[ -n "$COMPATIBILITY_EXIT" ]] || { echo "::error::Compatibility validation did not produce an exit code"; exit 1; } if [[ "$COMPATIBILITY_EXIT" != 0 ]]; then echo "::error::PPL grammar compatibility validation exited $COMPATIBILITY_EXIT" exit "$COMPATIBILITY_EXIT" fi - jq -e '.status == "passed"' ppl-lint-grammar-compatibility-report.json > /dev/null || - { echo "::error::Compatibility validation did not produce a passed report"; exit 1; } + jq -e '.status == "passed" or .status == "skipped"' \ + ppl-lint-grammar-compatibility-report.json > /dev/null || + { echo "::error::Compatibility validation did not produce a passed or skipped report"; exit 1; } diff --git a/docs/dev/ppl-lint-grammar-compatibility-ci.md b/docs/dev/ppl-lint-grammar-compatibility-ci.md index dd04b97bfe7..42ef404dfc7 100644 --- a/docs/dev/ppl-lint-grammar-compatibility-ci.md +++ b/docs/dev/ppl-lint-grammar-compatibility-ci.md @@ -1,18 +1,17 @@ # PPL Linter Grammar Compatibility CI This check verifies that the runtime PPL grammar built from a SQL revision -remains compatible with the grammar-dependent linter rules in the paired -OpenSearch Dashboards (OSD) revision. +preserves the observable diagnostics of every applicable rule in the paired +OpenSearch Dashboards (OSD) PPL lint catalog. -SQL owns the candidate grammar endpoint, branch pairing, grammar cases, and CI -artifacts. OSD owns the headless lint API, rule catalog, detectors, and +SQL owns the candidate grammar exporter, branch pairing, cases, exclusions, and +CI artifacts. OSD owns the headless lint API, rule catalog, detectors, and parse-tree behavior. -The check is deliberately grammar-only. It does not send PPL queries to an -OpenSearch backend, create indices or fixtures, compare historical engines, run -Analytics Engine, or assert diagnostic wording, severity, fixes, hover content, -or UI rendering. OpenSearch runs only long enough to serve -`GET /_plugins/_ppl/_grammar`. +The check is deliberately grammar-only. It does not start OpenSearch, call the +plugin REST endpoint, execute PPL, create indices, require a syntax-clean parse +tree, compare compiled fallback grammars, or assert diagnostic wording, +severity, fixes, hover content, or UI rendering. ## Branch pairing @@ -28,56 +27,58 @@ Pull requests use `github.base_ref`; pushes and manual dispatches use the pull request head SHA separately when available. SQL's product version is the default `opensearch.version` in the root -`build.gradle`. OSD's product version is read from the checked-out build with -`yarn --silent pkg-version`, after selecting Node from OSD's `.nvmrc`. Patch -versions may differ on an exact release line. +`build.gradle`. OSD's product version comes from `yarn --silent pkg-version` +after selecting Node from OSD's `.nvmrc`. Patch versions may differ on an exact +release line. -The workflow does not infer an exact branch for `X.x` branches and never falls -back from a missing OSD `X.Y` branch to OSD `main`. There is no fixed -historical-release lane on `main`: the contract is between the two branches -that ship together, not between current SQL and an unrelated old OSD grammar. +The workflow does not infer an exact branch for version-family branches and +never falls back from a missing OSD `X.Y` branch to OSD `main`. There is no +release-line bypass. ## Validation path The focused workflow is [ppl-lint-grammar-compatibility.yml](../../.github/workflows/ppl-lint-grammar-compatibility.yml). -For each supported event it: +It: 1. Resolves the SQL target and paired OSD ref. -2. Checks out OSD and records its immutable SHA before running OSD code. +2. Checks out canonical `opensearch-project/OpenSearch-Dashboards` and records + its immutable SHA before executing OSD code. 3. Validates product versions and writes `resolved-target.json`. -4. Probes the - `src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint` module, - accepting its `.ts` or `.js` form. -5. If the module is absent, writes a successful capability-skip report and - stops before starting OpenSearch or bootstrapping OSD. -6. Otherwise starts the existing `:opensearch-sql-plugin:run` task, captures - and validates `ppl-grammar-bundle.json`, then stops the cluster. -7. Bootstraps OSD and runs +4. Runs `:ppl:exportPplGrammarBundle` to serialize + `PPLGrammarBundleBuilder.getBundle()` directly. +5. Bootstraps the exact OSD checkout. +6. Invokes [validate-osd-grammar.mjs](../../scripts/ppl-lint/validate-osd-grammar.mjs) - against [grammar-cases.json](../../scripts/ppl-lint/grammar-cases.json). -8. Publishes the report and evidence before enforcing the result. + once under OSD's Node environment. +7. Publishes the report and evidence before enforcing the adapter exit code. -Capability is detected from the checkout, not from a product-version constant. -An absent module means: +The workflow and shell wrapper do not probe for OSD module files. The adapter's +resolver and loader are the sole capability authority: -```json -{ - "status": "skipped", - "skipReason": "osd-headless-grammar-api-unavailable" -} -``` +- a genuinely absent headless entry point produces `status: "skipped"` with + `skipReason: "osd-headless-grammar-api-unavailable"`; +- a resolved entry point that cannot import, has a missing transitive + dependency, or lacks required exports produces a structural error. + +This prevents valid API layouts from becoming false-green skips. + +The adapter deserializes the candidate bundle without invoking OSD's compiled +grammar fallback. For each case it enables only the named rule and compares the +target rule's exact diagnostic count. Recovered parse trees are valid; only +observable linter diagnostics determine case behavior. -This is exit code `0` and executes zero cases. If the module exists but cannot -load `deserializeBundleOrThrow` and `lintQueryWithBundle`, the supported API is -broken and the run fails structurally. It must not become a skip. +Case schema version 2 classifies the complete paired catalog. It requires: + +```text +coveredRuleIds == catalogRuleIds - excludedRuleIds +``` -The adapter deserializes the candidate bundle without a compiled-grammar -fallback. For each case it enables only the named rule, supplies the normalized -SQL version, builds a parse tree, and compares the target rule's diagnostic -count. Every selected rule needs trigger and control coverage, and every case -must name a rule in the paired OSD catalog. Missing cases, rules, parse trees, -syntax-clean trees, or bundles fail rather than pass vacuously. +Every covered rule needs trigger and control cases. Every exclusion needs a +non-empty reason. Missing rules, unknown cases, stale exclusions, overlaps, and +duplicate IDs fail structurally. The two explain-backed rules are excluded +because a grammar-only run has no backend explain plan; the other 16 catalog +rules are covered. ## Events @@ -85,41 +86,48 @@ syntax-clean trees, or bundles fail rather than pass vacuously. | --- | --- | --- | | `pull_request` | Pre-merge candidate validation | Canonical paired branch | | `push` | Validation of the exact merged revision | Canonical paired branch | -| `workflow_dispatch` | Non-required OSD branch, fork, or SHA evidence | Optional `osd_repo` and `osd_ref` | +| `workflow_dispatch` | Non-required canary against another canonical ref | Optional `osd_ref` in the canonical repository | -Pull request and push runs use no repository variables to redirect OSD. -Manual overrides remain diagnostic evidence and do not satisfy branch -protection. +Required pull request and push runs cannot redirect OSD. Manual dispatch may +select another ref only from +`opensearch-project/OpenSearch-Dashboards`; it cannot select another repository +or bypass release-line version checks. + +Product path triggers are limited to the PPL grammar sources, grammar bundle +builder/exporter, and their build inputs. Tooling paths cover the adapter, +cases, wrapper, tests, and workflow itself. Plugin startup and REST action files +are not inputs to this check. ## Reports and artifacts -The `ppl-lint-grammar-compatibility` CI artifact contains the files that were -available for the run: +The `ppl-lint-grammar-compatibility` artifact contains the files available for +the run: - `resolved-target.json`: SQL and OSD branches, versions, and immutable SHAs; - `osd-revision.txt`: the recorded OSD SHA; -- `ppl-lint-grammar-compatibility-report.json`: final machine-readable result; -- `ppl-grammar-bundle.json`: candidate bundle when capability is available; and -- `ppl-grammar-cluster.log`: candidate cluster output when it was started. - -Every report has `schemaVersion`, `status`, and rule counts. Reports produced -after target resolution also include SQL and OSD metadata, the manual-override -flag, and the release-line-bypass flag. A validating report additionally -includes `grammarHash`, case counts, normalized case results, and failures with -rule ID, case ID, query, and expected and actual diagnostic counts. A skipped -report includes `skipReason` and zero selected rules. Structural adapter -failures use `status: "error"` and include `error`. -Failures before target resolution still produce an error report, but cannot -include SQL or OSD metadata. The GitHub step summary presents the available -provenance and failed cases. +- `ppl-grammar-bundle.json`: direct exporter output; and +- `ppl-lint-grammar-compatibility-report.json`: machine-readable result. + +Report schema version 2 includes status, provenance, grammar hash, case results, +failures, and catalog classification: + +- catalog, required, covered, excluded, missing, and unexpected rule sets; +- reasons for every excluded rule; +- selected/passed/failed rule and case counts; and +- expected and actual target-rule diagnostic counts. + +A failure before the adapter writes a report produces a schema-version-2 error +report. The workflow always creates that fallback when needed, uploads all +available evidence, and only then enforces the result. This preserves evidence +for exporter, bootstrap, and adapter startup failures. The wrapper and adapter use these exit codes: | Code | Meaning | | ---: | --- | -| `0` | All cases passed, or the paired OSD branch lacks the headless API | -| `1` | One or more grammar/linter diagnostic counts did not match | -| `2` | Pairing, input, bundle, case coverage, or advertised-API failure | +| `0` | All cases passed, or the paired OSD branch genuinely lacks the API | +| `1` | One or more target-rule diagnostic counts did not match | +| `2` | Pairing, input, bundle, catalog coverage, or advertised-API failure | See the [operational quick start](../../scripts/ppl-lint/README.md) for local commands and exact-SHA reproduction. @@ -127,40 +135,37 @@ commands and exact-SHA reproduction. ## Security - Required runs execute only the canonical paired OSD branch. -- OSD repository/ref overrides and release-line bypasses are - `workflow_dispatch`-only. +- Manual dispatch can override only the canonical repository's ref. +- Exact release-line validation cannot be disabled. - The job has `contents: read`, persists no checkout credentials, and receives no repository secrets. -- The immutable OSD SHA is recorded before OSD dependency or build code runs. -- Manual fork/ref runs are non-required and receive no privileged credentials. +- The immutable OSD SHA is recorded before dependency or build code runs. ## Release branches -When a new exact `X.Y` branch is cut, carry the workflow, scripts, grammar -cases, and this documentation with the SQL branch. Confirm that the OSD `X.Y` -branch exists and both builds report `X.Y.z`, then run a manual canary. -Capability detection decides whether the branch validates or reports -`skipped`; no version constant or documentation rewrite is required. +When a new exact `X.Y` branch is cut, carry the workflow, scripts, cases, and +this documentation with the SQL branch. Confirm that the OSD `X.Y` branch +exists and both builds report `X.Y.z`, then run a manual canary. -An already-cut branch without the OSD API may continue to report a visible -successful skip. If the API is later backported, the same workflow starts -validating automatically. Version-family branches remain unsupported until -they receive an explicit pairing policy. +An older paired OSD branch without the headless API may continue to produce a +visible successful skip. If the API is backported, the same adapter begins +validating automatically. No workflow or wrapper capability table needs +updating. ## Triage Always begin with the SQL and OSD SHAs in the report. Reproducing against a -newer `main` does not reproduce the completed run. +newer branch tip does not reproduce the completed run. | Failure | First owner or action | | --- | --- | | SQL or OSD version disagrees with exact `X.Y` | CI owner checks branch selection and branch-cut state | | Paired OSD branch is missing | CI owner fixes pairing; never substitute `main` | -| Headless module is absent | No product action; verify `skipped` and exact metadata | -| Module exists but imports or exports fail | OSD linter owner treats it as a headless API regression | -| Cluster startup or grammar GET fails | SQL plugin owner inspects `ppl-grammar-cluster.log` | -| Bundle validation or deserialization fails | SQL grammar-bundle owner checks the endpoint schema and generated grammar | +| Headless API is genuinely absent | Verify the visible skip and exact old OSD SHA | +| Headless module import or exports fail | OSD linter owner treats it as an API regression | +| Direct grammar export fails | SQL grammar owner inspects the `:ppl` build | +| Bundle validation or deserialization fails | SQL grammar-bundle owner checks exporter schema and generated grammar | +| Catalog classification is incomplete | Add cases or a justified exclusion; do not remove failing coverage | | Trigger stops firing | SQL grammar and OSD rule owners inspect the parse-tree contract | | Control starts firing | OSD rule owner checks whether matching broadened intentionally | -| Case names a missing OSD rule | Review the OSD change, then update or remove the stale SQL case | | Post-merge push fails | Fix the merged revision before relying on its compatibility evidence | diff --git a/ppl/build.gradle b/ppl/build.gradle index e883c891fd3..ec04e4a8493 100644 --- a/ppl/build.gradle +++ b/ppl/build.gradle @@ -123,3 +123,20 @@ jacocoTestCoverageVerification { } } check.dependsOn jacocoTestCoverageVerification + +tasks.register('exportPplGrammarBundle', JavaExec) { + group = 'build' + description = 'Exports the generated PPL grammar bundle as JSON.' + dependsOn classes + classpath = sourceSets.main.runtimeClasspath + mainClass.set('org.opensearch.sql.ppl.autocomplete.PPLGrammarBundleExporter') + + def outputProperty = providers.gradleProperty('pplGrammarBundleOutput') + doFirst { + if (!outputProperty.isPresent() || outputProperty.get().trim().isEmpty()) { + throw new GradleException( + 'Missing required property: -PpplGrammarBundleOutput=') + } + setArgs([outputProperty.get()]) + } +} diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporter.java b/ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporter.java new file mode 100644 index 00000000000..ee2febbec87 --- /dev/null +++ b/ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporter.java @@ -0,0 +1,119 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ppl.autocomplete; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Map; +import org.json.JSONStringer; + +/** Exports the generated PPL grammar bundle without starting an OpenSearch cluster. */ +public final class PPLGrammarBundleExporter { + + private PPLGrammarBundleExporter() {} + + /** + * Writes the PPL grammar bundle to the output path supplied as the sole argument. + * + * @param args exactly one output file path + */ + public static void main(String[] args) throws IOException { + if (args.length != 1 || args[0].trim().isEmpty()) { + throw new IllegalArgumentException( + "Expected exactly one non-empty output path argument for the PPL grammar bundle"); + } + + writeBundle(Path.of(args[0]), PPLGrammarBundleBuilder.getBundle()); + } + + static void writeBundle(Path outputPath, GrammarBundle bundle) throws IOException { + Path output = outputPath.toAbsolutePath().normalize(); + Path parent = output.getParent(); + if (output.getFileName() == null || parent == null) { + throw new IllegalArgumentException("Output path must identify a file: " + outputPath); + } + + Files.createDirectories(parent); + Path temporary = + Files.createTempFile(parent, "." + output.getFileName().toString() + ".", ".tmp"); + boolean moved = false; + try { + Files.writeString( + temporary, + serializeBundle(bundle), + StandardCharsets.UTF_8, + StandardOpenOption.TRUNCATE_EXISTING); + moveIntoPlace(temporary, output); + moved = true; + } finally { + if (!moved) { + Files.deleteIfExists(temporary); + } + } + } + + static String serializeBundle(GrammarBundle bundle) { + JSONStringer json = new JSONStringer(); + json.object(); + + json.key("bundleVersion").value(bundle.getBundleVersion()); + json.key("antlrVersion").value(bundle.getAntlrVersion()); + json.key("grammarHash").value(bundle.getGrammarHash()); + json.key("startRuleIndex").value(bundle.getStartRuleIndex()); + + writeIntArray(json, "lexerSerializedATN", bundle.getLexerSerializedATN()); + writeStringArray(json, "lexerRuleNames", bundle.getLexerRuleNames()); + writeStringArray(json, "channelNames", bundle.getChannelNames()); + writeStringArray(json, "modeNames", bundle.getModeNames()); + + writeIntArray(json, "parserSerializedATN", bundle.getParserSerializedATN()); + writeStringArray(json, "parserRuleNames", bundle.getParserRuleNames()); + + writeStringArray(json, "literalNames", bundle.getLiteralNames()); + writeStringArray(json, "symbolicNames", bundle.getSymbolicNames()); + + json.key("tokenDictionary").object(); + for (Map.Entry entry : bundle.getTokenDictionary().entrySet()) { + json.key(entry.getKey()).value(entry.getValue()); + } + json.endObject(); + writeIntArray(json, "ignoredTokens", bundle.getIgnoredTokens()); + writeIntArray(json, "rulesToVisit", bundle.getRulesToVisit()); + + json.endObject(); + return json.toString(); + } + + private static void writeIntArray(JSONStringer json, String fieldName, int[] values) { + json.key(fieldName).array(); + for (int value : values) { + json.value(value); + } + json.endArray(); + } + + private static void writeStringArray(JSONStringer json, String fieldName, String[] values) { + json.key(fieldName).array(); + for (String value : values) { + json.value(value); + } + json.endArray(); + } + + private static void moveIntoPlace(Path temporary, Path output) throws IOException { + try { + Files.move( + temporary, output, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temporary, output, StandardCopyOption.REPLACE_EXISTING); + } + } +} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporterTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporterTest.java new file mode 100644 index 00000000000..402d4f1e6e0 --- /dev/null +++ b/ppl/src/test/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleExporterTest.java @@ -0,0 +1,162 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ppl.autocomplete; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Stream; +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class PPLGrammarBundleExporterTest { + + private static final Set EXPECTED_FIELDS = + new HashSet<>( + Arrays.asList( + "bundleVersion", + "antlrVersion", + "grammarHash", + "startRuleIndex", + "lexerSerializedATN", + "lexerRuleNames", + "channelNames", + "modeNames", + "parserSerializedATN", + "parserRuleNames", + "literalNames", + "symbolicNames", + "tokenDictionary", + "ignoredTokens", + "rulesToVisit")); + + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void testSerializeBundleUsesRestSchema() { + GrammarBundle bundle = PPLGrammarBundleBuilder.getBundle(); + JSONObject json = new JSONObject(PPLGrammarBundleExporter.serializeBundle(bundle)); + + assertEquals(15, json.length()); + assertEquals(EXPECTED_FIELDS, json.keySet()); + assertEquals(bundle.getBundleVersion(), json.getString("bundleVersion")); + assertEquals(bundle.getAntlrVersion(), json.getString("antlrVersion")); + assertEquals(bundle.getGrammarHash(), json.getString("grammarHash")); + assertEquals(bundle.getStartRuleIndex(), json.getInt("startRuleIndex")); + assertEquals( + bundle.getLexerSerializedATN().length, json.getJSONArray("lexerSerializedATN").length()); + assertEquals(bundle.getLexerRuleNames().length, json.getJSONArray("lexerRuleNames").length()); + assertEquals(bundle.getChannelNames().length, json.getJSONArray("channelNames").length()); + assertEquals(bundle.getModeNames().length, json.getJSONArray("modeNames").length()); + assertEquals( + bundle.getParserSerializedATN().length, json.getJSONArray("parserSerializedATN").length()); + assertEquals(bundle.getParserRuleNames().length, json.getJSONArray("parserRuleNames").length()); + assertEquals(bundle.getLiteralNames().length, json.getJSONArray("literalNames").length()); + assertEquals(bundle.getSymbolicNames().length, json.getJSONArray("symbolicNames").length()); + assertEquals( + bundle.getTokenDictionary().size(), json.getJSONObject("tokenDictionary").length()); + assertEquals(bundle.getIgnoredTokens().length, json.getJSONArray("ignoredTokens").length()); + assertEquals(bundle.getRulesToVisit().length, json.getJSONArray("rulesToVisit").length()); + } + + @Test + public void testSerializeBundlePreservesSparseVocabularyNulls() { + GrammarBundle bundle = PPLGrammarBundleBuilder.getBundle(); + JSONObject json = new JSONObject(PPLGrammarBundleExporter.serializeBundle(bundle)); + + assertSparseArrayEquals(bundle.getLiteralNames(), json.getJSONArray("literalNames")); + assertSparseArrayEquals(bundle.getSymbolicNames(), json.getJSONArray("symbolicNames")); + } + + @Test + public void testSerializeBundleIsDeterministic() { + GrammarBundle bundle = PPLGrammarBundleBuilder.getBundle(); + + assertEquals( + PPLGrammarBundleExporter.serializeBundle(bundle), + PPLGrammarBundleExporter.serializeBundle(bundle)); + } + + @Test + public void testWriteBundleCreatesParentsAndReplacesOutput() throws IOException { + GrammarBundle bundle = PPLGrammarBundleBuilder.getBundle(); + Path root = temporaryFolder.getRoot().toPath(); + Path output = root.resolve("nested/grammar/ppl.json"); + + PPLGrammarBundleExporter.writeBundle(output, bundle); + byte[] first = Files.readAllBytes(output); + Files.writeString(output, "stale", StandardCharsets.UTF_8); + PPLGrammarBundleExporter.writeBundle(output, bundle); + + assertArrayEquals(first, Files.readAllBytes(output)); + assertEquals( + bundle.getGrammarHash(), + new JSONObject(Files.readString(output, StandardCharsets.UTF_8)).getString("grammarHash")); + try (Stream siblings = Files.list(output.getParent())) { + assertFalse( + siblings.anyMatch( + path -> + path.getFileName().toString().startsWith(".ppl.json.") + && path.getFileName().toString().endsWith(".tmp"))); + } + } + + @Test + public void testMainRejectsInvalidArguments() throws IOException { + assertInvalidArguments(new String[0]); + assertInvalidArguments(new String[] {""}); + assertInvalidArguments(new String[] {"first.json", "second.json"}); + } + + @Test + public void testWriteBundleFailsWhenParentIsAFile() throws IOException { + Path parent = temporaryFolder.newFile("not-a-directory").toPath(); + Path output = parent.resolve("ppl.json"); + + try { + PPLGrammarBundleExporter.writeBundle(output, PPLGrammarBundleBuilder.getBundle()); + fail("Expected write to fail when the output parent is a file"); + } catch (IOException expected) { + assertFalse(Files.exists(output)); + } + } + + private static void assertSparseArrayEquals(String[] expected, JSONArray actual) { + assertEquals(expected.length, actual.length()); + boolean foundNull = false; + for (int i = 0; i < expected.length; i++) { + if (expected[i] == null) { + foundNull = true; + assertTrue("Expected null vocabulary entry at index " + i, actual.isNull(i)); + } else { + assertEquals(expected[i], actual.getString(i)); + } + } + assertTrue("Expected at least one sparse vocabulary entry", foundNull); + } + + private static void assertInvalidArguments(String[] args) throws IOException { + try { + PPLGrammarBundleExporter.main(args); + fail("Expected invalid arguments to fail"); + } catch (IllegalArgumentException expected) { + assertTrue(expected.getMessage().contains("output path")); + } + } +} diff --git a/scripts/ppl-lint-rule-validation.sh b/scripts/ppl-lint-rule-validation.sh index b8bcac30327..59e7a82fbbc 100755 --- a/scripts/ppl-lint-rule-validation.sh +++ b/scripts/ppl-lint-rule-validation.sh @@ -3,7 +3,7 @@ # Copyright OpenSearch Contributors # SPDX-License-Identifier: Apache-2.0 # -# Capture the candidate SQL runtime grammar and validate it with the paired +# Export the candidate SQL runtime grammar and validate it with the paired # OpenSearch Dashboards (OSD) headless PPL linter. set -euo pipefail @@ -11,7 +11,7 @@ set -euo pipefail SQL_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" cd "$SQL_ROOT" -HEADLESS_MODULE="src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint" +CANONICAL_OSD_REPOSITORY="opensearch-project/OpenSearch-Dashboards" VALIDATOR="$SQL_ROOT/scripts/ppl-lint/validate-osd-grammar.mjs" TARGET_INPUT="${PPL_LINT_TARGET:-}" @@ -24,13 +24,7 @@ GRAMMAR_BUNDLE="${PPL_LINT_GRAMMAR_BUNDLE:-$SQL_ROOT/ppl-grammar-bundle.json}" GRAMMAR_CASES="${PPL_LINT_CASES:-$SQL_ROOT/scripts/ppl-lint/grammar-cases.json}" REPORT="${PPL_LINT_REPORT:-$SQL_ROOT/ppl-lint-grammar-compatibility-report.json}" SUMMARY="${PPL_LINT_SUMMARY:-${GITHUB_STEP_SUMMARY:-$SQL_ROOT/ppl-lint-grammar-summary.md}}" -CLUSTER_LOG="${PPL_LINT_CLUSTER_LOG:-$SQL_ROOT/ppl-grammar-cluster.log}" -STARTUP_TIMEOUT="${PPL_LINT_STARTUP_TIMEOUT_SECONDS:-300}" SKIP_OSD_BOOTSTRAP="${PPL_LINT_SKIP_OSD_BOOTSTRAP:-0}" -RELEASE_LINE_BYPASS=false - -GRADLE_PID="" -CAPTURE_TMP="" log() { printf '[ppl-lint-rule-validation] %s\n' "$*" @@ -54,18 +48,16 @@ Options: --target-branch NAME SQL target branch for local metadata (main or X.Y). --osd-repository NAME OSD repository recorded in local metadata. --osd-ref REF OSD ref recorded in local metadata. - --grammar FILE Captured grammar output path. + --grammar FILE Exported grammar bundle output path. --cases FILE Grammar cases input path. --report FILE Validation report output path. --summary FILE Validation summary output path. - --cluster-log FILE Gradle run log output path. - --startup-timeout SEC Bounded cluster readiness timeout (default: 300). -h, --help Show this help. Environment: PPL_LINT_SKIP_OSD_BOOTSTRAP=1 - Skip OSD bootstrap on the supported path. The caller - must ensure generated OSD targets match the checkout. + Skip OSD bootstrap. The caller must ensure generated + OSD targets match the exact checkout. EOF } @@ -122,16 +114,6 @@ while [[ $# -gt 0 ]]; do SUMMARY="$2" shift 2 ;; - --cluster-log) - require_value "$1" "${2:-}" - CLUSTER_LOG="$2" - shift 2 - ;; - --startup-timeout) - require_value "$1" "${2:-}" - STARTUP_TIMEOUT="$2" - shift 2 - ;; -h|--help) usage exit 0 @@ -178,46 +160,9 @@ read_sql_version() { printf '%s\n' "$version" } -stop_cluster() { - local pid="$GRADLE_PID" - local count=0 - - [[ -n "$pid" ]] || return 0 - if kill -0 "$pid" 2>/dev/null; then - log "Stopping Gradle development cluster (pid $pid)" - kill "$pid" 2>/dev/null || true - while kill -0 "$pid" 2>/dev/null && [[ "$count" -lt 30 ]]; do - sleep 1 - count=$((count + 1)) - done - if kill -0 "$pid" 2>/dev/null; then - log "Gradle process did not stop in time; terminating it" - kill -KILL "$pid" 2>/dev/null || true - fi - fi - wait "$pid" 2>/dev/null || true - GRADLE_PID="" -} - -cleanup() { - local status=$? - trap - EXIT - stop_cluster - if [[ -n "$CAPTURE_TMP" ]]; then - rm -f "$CAPTURE_TMP" - fi - exit "$status" -} - -trap cleanup EXIT -trap 'exit 130' INT -trap 'exit 143' TERM - -for command in curl git jq node; do +for command in git jq node; do command -v "$command" >/dev/null 2>&1 || die "required command not found: $command" done -[[ "$STARTUP_TIMEOUT" =~ ^[1-9][0-9]*$ ]] || - die "--startup-timeout must be a positive integer" [[ "$SKIP_OSD_BOOTSTRAP" == "0" || "$SKIP_OSD_BOOTSTRAP" == "1" ]] || die "PPL_LINT_SKIP_OSD_BOOTSTRAP must be 0 or 1" @@ -225,7 +170,6 @@ GRAMMAR_BUNDLE="$(absolute_path "$GRAMMAR_BUNDLE")" GRAMMAR_CASES="$(absolute_path "$GRAMMAR_CASES")" REPORT="$(absolute_path "$REPORT")" SUMMARY="$(absolute_path "$SUMMARY")" -CLUSTER_LOG="$(absolute_path "$CLUSTER_LOG")" if [[ -n "$TARGET_INPUT" ]]; then TARGET="$(absolute_path "$TARGET_INPUT")" @@ -242,15 +186,14 @@ if [[ -n "$TARGET_INPUT" ]]; then (.osd.sha | type == "string" and length > 0) and (.osd.version | type == "string" and length > 0) and ((has("releaseLineValidationBypassed") | not) or - (.releaseLineValidationBypassed | type == "boolean")) + .releaseLineValidationBypassed == false) ' "$TARGET" >/dev/null || die "target metadata is missing required SQL/OSD fields: $TARGET" OSD_REPOSITORY_INPUT="$(jq -r '.osd.repository' "$TARGET")" OSD_REF_INPUT="$(jq -r '.osd.ref' "$TARGET")" OSD_CHECKOUT_REF="$(jq -r '.osd.sha' "$TARGET")" - RELEASE_LINE_BYPASS="$(jq -r '.releaseLineValidationBypassed // false' "$TARGET")" else TARGET_BRANCH_INPUT="${TARGET_BRANCH_INPUT:-main}" - OSD_REPOSITORY_INPUT="${OSD_REPOSITORY_INPUT:-opensearch-project/OpenSearch-Dashboards}" + OSD_REPOSITORY_INPUT="${OSD_REPOSITORY_INPUT:-$CANONICAL_OSD_REPOSITORY}" OSD_REF_INPUT="${OSD_REF_INPUT:-$TARGET_BRANCH_INPUT}" OSD_CHECKOUT_REF="$OSD_REF_INPUT" TARGET="$SQL_ROOT/resolved-target.json" @@ -303,8 +246,14 @@ fi if [[ -z "$TARGET_INPUT" ]]; then [[ "$TARGET_BRANCH_INPUT" == "main" || "$TARGET_BRANCH_INPUT" =~ ^[0-9]+\.[0-9]+$ ]] || die "local --target-branch must be main or an exact X.Y release branch" + if [[ "$TARGET_BRANCH_INPUT" != "main" ]]; then + [[ "$(release_line "$SQL_VERSION_ACTUAL")" == "$TARGET_BRANCH_INPUT" ]] || + die "SQL version $SQL_VERSION_ACTUAL does not match target release line $TARGET_BRANCH_INPUT" + [[ "$(release_line "$OSD_VERSION_ACTUAL")" == "$TARGET_BRANCH_INPUT" ]] || + die "OSD version $OSD_VERSION_ACTUAL does not match target release line $TARGET_BRANCH_INPUT" + fi OSD_OVERRIDE=false - if [[ "$OSD_REPOSITORY_INPUT" != "opensearch-project/OpenSearch-Dashboards" || + if [[ "$OSD_REPOSITORY_INPUT" != "$CANONICAL_OSD_REPOSITORY" || "$OSD_REF_INPUT" != "$TARGET_BRANCH_INPUT" ]]; then OSD_OVERRIDE=true fi @@ -318,6 +267,7 @@ if [[ -z "$TARGET_INPUT" ]]; then --arg osdRepository "$OSD_REPOSITORY_INPUT" \ --arg osdRef "$OSD_REF_INPUT" \ --arg osdSha "$OSD_SHA_ACTUAL" \ + --arg osdVersionRaw "$OSD_VERSION_RAW_ACTUAL" \ --arg osdVersion "$OSD_VERSION_ACTUAL" \ --argjson osdOverride "$OSD_OVERRIDE" \ '{ @@ -332,10 +282,10 @@ if [[ -z "$TARGET_INPUT" ]]; then repository: $osdRepository, ref: $osdRef, sha: $osdSha, + versionRaw: $osdVersionRaw, version: $osdVersion, override: $osdOverride - }, - releaseLineValidationBypassed: false + } } | if $sqlHeadSha == "" then . else .sql.headSha = $sqlHeadSha end' \ >"$target_tmp" @@ -347,6 +297,8 @@ SQL_SHA="$(jq -r '.sql.sha' "$TARGET")" TARGET_BRANCH="$(jq -r '.sql.targetBranch' "$TARGET")" SQL_VERSION="$(normalize_version "$(jq -r '.sql.version' "$TARGET")")" OSD_SHA="$(jq -r '.osd.sha' "$TARGET")" +OSD_REF="$(jq -r '.osd.ref' "$TARGET")" +OSD_OVERRIDE="$(jq -r '.osd.override // .manualOverride // false' "$TARGET")" OSD_VERSION="$(normalize_version "$(jq -r '.osd.version' "$TARGET")")" [[ "$SQL_SHA" == "$SQL_SHA_ACTUAL" ]] || @@ -357,121 +309,27 @@ OSD_VERSION="$(normalize_version "$(jq -r '.osd.version' "$TARGET")")" die "target SQL version $SQL_VERSION does not match build.gradle $SQL_VERSION_ACTUAL" [[ "$OSD_VERSION" == "$OSD_VERSION_ACTUAL" ]] || die "target OSD version $OSD_VERSION does not match package.json $OSD_VERSION_ACTUAL" - +[[ "$TARGET_BRANCH" == "main" || "$TARGET_BRANCH" =~ ^[0-9]+\.[0-9]+$ ]] || + die "target branch must be main or an exact X.Y release branch: $TARGET_BRANCH" +if [[ "$OSD_OVERRIDE" != "true" && "$OSD_REF" != "$TARGET_BRANCH" ]]; then + die "target OSD ref $OSD_REF does not match target branch $TARGET_BRANCH" +fi if [[ "$TARGET_BRANCH" != "main" ]]; then - [[ "$TARGET_BRANCH" =~ ^[0-9]+\.[0-9]+$ ]] || - die "target branch must be main or an exact X.Y release branch: $TARGET_BRANCH" - if [[ "$RELEASE_LINE_BYPASS" != "true" ]]; then - [[ "$(release_line "$SQL_VERSION")" == "$TARGET_BRANCH" ]] || - die "SQL version $SQL_VERSION does not match target release line $TARGET_BRANCH" - [[ "$(release_line "$OSD_VERSION")" == "$TARGET_BRANCH" ]] || - die "OSD version $OSD_VERSION does not match target release line $TARGET_BRANCH" - fi + [[ "$(release_line "$SQL_VERSION")" == "$TARGET_BRANCH" ]] || + die "SQL version $SQL_VERSION does not match target release line $TARGET_BRANCH" + [[ "$(release_line "$OSD_VERSION")" == "$TARGET_BRANCH" ]] || + die "OSD version $OSD_VERSION does not match target release line $TARGET_BRANCH" fi [[ -f "$VALIDATOR" ]] || die "validation adapter not found: $VALIDATOR" -mkdir -p "$(dirname "$GRAMMAR_BUNDLE")" "$(dirname "$REPORT")" \ - "$(dirname "$SUMMARY")" "$(dirname "$CLUSTER_LOG")" - -ADAPTER_ARGS=( - --grammar "$GRAMMAR_BUNDLE" - --cases "$GRAMMAR_CASES" - --target "$TARGET" - --osd-root "$OSD_ROOT" - --osd-sha "$OSD_SHA" - --report "$REPORT" - --summary "$SUMMARY" -) - -headless_module_exists() { - [[ -f "$OSD_ROOT/$HEADLESS_MODULE" || - -f "$OSD_ROOT/$HEADLESS_MODULE.ts" || - -f "$OSD_ROOT/$HEADLESS_MODULE.js" || - -f "$OSD_ROOT/$HEADLESS_MODULE.mjs" ]] -} - -if ! headless_module_exists; then - log "OSD headless grammar API is unavailable; requesting a skipped report" - node "$VALIDATOR" "${ADAPTER_ARGS[@]}" - log "Validation skipped; report: $REPORT" - exit 0 -fi - [[ -f "$GRAMMAR_CASES" ]] || die "grammar cases not found: $GRAMMAR_CASES" [[ -x "$SQL_ROOT/gradlew" ]] || die "Gradle wrapper is not executable: $SQL_ROOT/gradlew" +mkdir -p "$(dirname "$GRAMMAR_BUNDLE")" "$(dirname "$REPORT")" "$(dirname "$SUMMARY")" -if curl --fail --silent --max-time 2 "http://127.0.0.1:9200/_cluster/health" >/dev/null 2>&1; then - die "port 9200 already serves an OpenSearch cluster; refusing to capture from an unknown process" -fi - -: >"$CLUSTER_LOG" -log "Starting candidate SQL development cluster" -./gradlew :opensearch-sql-plugin:run >"$CLUSTER_LOG" 2>&1 & -GRADLE_PID=$! - -deadline=$((SECONDS + STARTUP_TIMEOUT)) -while true; do - if ! kill -0 "$GRADLE_PID" 2>/dev/null; then - wait "$GRADLE_PID" 2>/dev/null || gradle_status=$? - GRADLE_PID="" - die "Gradle run exited before cluster readiness (status ${gradle_status:-0}); see $CLUSTER_LOG" - fi - if curl --fail --silent --show-error --connect-timeout 2 --max-time 5 \ - "http://127.0.0.1:9200/_cluster/health" >/dev/null 2>&1; then - break - fi - (( SECONDS < deadline )) || - die "cluster did not become ready within ${STARTUP_TIMEOUT}s; see $CLUSTER_LOG" - sleep 2 -done - -CAPTURE_TMP="$(mktemp "$GRAMMAR_BUNDLE.tmp.XXXXXX")" -log "Capturing GET /_plugins/_ppl/_grammar" -if ! http_status="$( - curl --silent --show-error --connect-timeout 2 --max-time 30 \ - --output "$CAPTURE_TMP" --write-out '%{http_code}' \ - "http://127.0.0.1:9200/_plugins/_ppl/_grammar" -)"; then - die "grammar endpoint request failed; see $CLUSTER_LOG" -fi -if [[ "$http_status" != "200" ]]; then - mv "$CAPTURE_TMP" "$GRAMMAR_BUNDLE" - CAPTURE_TMP="" - die "grammar endpoint returned HTTP $http_status" -fi - -jq -e ' - def nonempty_strings: - type == "array" and length > 0 and all(.[]; type == "string" and length > 0); - def nonempty_integers: - type == "array" and length > 0 and all(.[]; type == "number" and floor == .); - def sparse_names: - type == "array" and length > 0 and - all(.[]; . == null or type == "string") and any(.[]; . == null); - type == "object" and - (.bundleVersion | type == "string" and length > 0) and - (.antlrVersion | type == "string" and length > 0) and - (.grammarHash | type == "string" and test("^sha256:[0-9a-fA-F]{64}$")) and - (.lexerSerializedATN | nonempty_integers) and - (.parserSerializedATN | nonempty_integers) and - (.lexerRuleNames | nonempty_strings) and - (.parserRuleNames | nonempty_strings) and - (.channelNames | nonempty_strings) and - (.modeNames | nonempty_strings) and - (.startRuleIndex | type == "number" and floor == . and . >= 0) and - (.literalNames | sparse_names) and - (.symbolicNames | sparse_names) and - (.tokenDictionary | - type == "object" and length > 0 and - all(.[]; type == "number" and floor == . and . >= 0)) and - (.ignoredTokens | type == "array" and all(.[]; type == "number" and floor == .)) and - (.rulesToVisit | nonempty_integers) -' "$CAPTURE_TMP" >/dev/null || die "grammar endpoint returned a malformed bundle" - -mv "$CAPTURE_TMP" "$GRAMMAR_BUNDLE" -CAPTURE_TMP="" -log "Captured structurally valid grammar bundle: $GRAMMAR_BUNDLE" -stop_cluster +log "Exporting candidate grammar bundle" +./gradlew :ppl:exportPplGrammarBundle --no-daemon \ + "-PpplGrammarBundleOutput=$GRAMMAR_BUNDLE" +[[ -s "$GRAMMAR_BUNDLE" ]] || die "grammar exporter did not write $GRAMMAR_BUNDLE" if [[ "$SKIP_OSD_BOOTSTRAP" == "1" ]]; then log "Skipping OSD bootstrap because PPL_LINT_SKIP_OSD_BOOTSTRAP=1" @@ -485,6 +343,16 @@ fi -f "$OSD_ROOT/src/setup_node_env.ts" || -f "$OSD_ROOT/src/setup_node_env" ]] || die "OSD setup_node_env entry point not found" +ADAPTER_ARGS=( + --grammar "$GRAMMAR_BUNDLE" + --cases "$GRAMMAR_CASES" + --target "$TARGET" + --osd-root "$OSD_ROOT" + --osd-sha "$OSD_SHA" + --report "$REPORT" + --summary "$SUMMARY" +) + log "Validating candidate grammar with OSD@$OSD_SHA" ( cd "$OSD_ROOT" diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md index 09c4e713605..93e73550fb3 100644 --- a/scripts/ppl-lint/README.md +++ b/scripts/ppl-lint/README.md @@ -1,26 +1,26 @@ # PPL grammar compatibility quick start -This directory contains the grammar cases and the adapter used by +This directory contains the SQL-owned inputs and adapter used by [PPL linter grammar compatibility CI](../../docs/dev/ppl-lint-grammar-compatibility-ci.md): -- [grammar-cases.json](grammar-cases.json) contains grammar-only trigger and - control queries with expected diagnostic counts. -- [validate-osd-grammar.mjs](validate-osd-grammar.mjs) loads the paired OSD - headless linter API and writes the compatibility report. -- [ppl-lint-rule-validation.sh](../ppl-lint-rule-validation.sh) resolves local - metadata, captures the candidate SQL grammar, bootstraps OSD, and invokes the - adapter. +- [grammar-cases.json](grammar-cases.json) contains trigger/control cases and + justified catalog exclusions. +- [validate-osd-grammar.mjs](validate-osd-grammar.mjs) resolves the paired OSD + headless API, validates catalog coverage, and writes report schema version 2. +- [ppl-lint-rule-validation.sh](../ppl-lint-rule-validation.sh) verifies the + selected checkouts, exports the candidate grammar, bootstraps OSD, and invokes + the adapter once. -The temporary OpenSearch process serves only -`GET /_plugins/_ppl/_grammar`. This validation creates no indices or fixtures -and sends no backend PPL queries. +The grammar is a build artifact of the generated ANTLR classes. The wrapper +exports it through `:ppl:exportPplGrammarBundle`; it does not start OpenSearch, +open a port, call the plugin REST endpoint, create indices, or execute PPL. ## Prerequisites - JDK 21; - Node from the selected OSD checkout's `.nvmrc`; - the Yarn version required by that checkout's `package.json`; -- `curl`, `git`, `jq`, and an available local port `9200`; and +- `git` and `jq`; and - either a local OSD checkout or permission to clone `opensearch-project/OpenSearch-Dashboards`. @@ -32,9 +32,8 @@ From the SQL repository root: ./scripts/ppl-lint-rule-validation.sh ``` -With no options, the wrapper pairs local SQL with OSD `main` and clones OSD -into `.ci/OpenSearch-Dashboards` when that checkout does not exist. To use a -sibling checkout: +With no options, the wrapper pairs local SQL with OSD `main` and manages a +checkout in `.ci/OpenSearch-Dashboards`. To use a sibling checkout: ```bash ./scripts/ppl-lint-rule-validation.sh \ @@ -43,18 +42,22 @@ sibling checkout: ``` For an exact release branch, check out the same `X.Y` line in both repositories -and use `--target-branch X.Y --osd-ref X.Y`. The wrapper rejects SQL or OSD -versions outside that release line. On `main`, it records both product versions -without requiring their release lines to match. +and use `--target-branch X.Y --osd-ref X.Y`. The wrapper requires both products +to report `X.Y.z`. On `main`, it records both product versions without requiring +their release lines to match. -The wrapper checks OSD capability before starting OpenSearch. If the headless -module is absent, it writes a `skipped` report and exits `0`. +The adapter is the only authority on OSD headless API availability. The wrapper +does not inspect module filenames or extensions. A truly absent legacy API +produces a visible `skipped` report; an entry point that resolves but fails to +load or lacks required exports is a structural failure. + +Set `PPL_LINT_SKIP_OSD_BOOTSTRAP=1` only when the selected checkout has already +been bootstrapped and its generated targets are current. ## Exact CI reproduction -Start with `resolved-target.json` from the -`ppl-lint-grammar-compatibility` artifact. Check out the report's tested SQL SHA -and OSD SHA, not current branch tips: +Download `resolved-target.json` from the +`ppl-lint-grammar-compatibility` artifact, then check out the exact tested SHAs: ```bash git checkout --detach "$(jq -r '.sql.sha' /path/to/resolved-target.json)" @@ -66,12 +69,10 @@ git -C ../OpenSearch-Dashboards checkout --detach \ --osd-root ../OpenSearch-Dashboards ``` -Fetch either SHA from its repository first if it is not present locally. The -wrapper verifies both checkout SHAs and both product versions against the -target file before doing any validation. For pull requests, `.sql.sha` is the -tested merge revision; `.sql.headSha` is traceability metadata, not a -substitute. A dispatch artifact with `releaseLineValidationBypassed: true` -retains that development-only bypass during exact reproduction. +Fetch either SHA first if it is not present locally. The wrapper verifies both +checkout SHAs and both product versions before exporting or validating. For +pull requests, `.sql.sha` is the tested merge revision; `.sql.headSha` is +traceability metadata and is not a substitute. ## Local files @@ -80,72 +81,36 @@ The wrapper defaults to these repository-root paths: | Path | Role | | --- | --- | | `resolved-target.json` | Generated SQL/OSD metadata when `--target` is omitted | -| `ppl-grammar-bundle.json` | Captured production grammar endpoint response | -| `scripts/ppl-lint/grammar-cases.json` | SQL-owned trigger/control input | +| `ppl-grammar-bundle.json` | Direct grammar exporter output | +| `scripts/ppl-lint/grammar-cases.json` | Cases and explicit exclusions | | `ppl-lint-grammar-compatibility-report.json` | Machine-readable result | -| `ppl-lint-grammar-summary.md` | Local human-readable summary | -| `ppl-grammar-cluster.log` | Gradle development-cluster output | - -Override these with `--grammar`, `--cases`, `--report`, `--summary`, and -`--cluster-log`. +| `ppl-lint-grammar-summary.md` | Human-readable summary | -A capability skip does not start the cluster, so no bundle or cluster log is -expected. +Override output/input paths with `--grammar`, `--cases`, `--report`, and +`--summary`. ## Primitive debugging -Inspect the versions using the same sources as CI: - -```bash -sed -nE \ - 's/.*opensearch_version = System\.getProperty\("opensearch\.version", "([^"]+)"\).*/\1/p' \ - build.gradle - -cd ../OpenSearch-Dashboards -nvm use -yarn --silent pkg-version -``` - -Check capability: - -```bash -HEADLESS=../OpenSearch-Dashboards/src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint -test -f "${HEADLESS}.ts" || test -f "${HEADLESS}.js" -``` - -Start the existing SQL development cluster from the SQL root: +Export the candidate bundle directly: ```bash -./gradlew :opensearch-sql-plugin:run -``` - -In another terminal, capture and inspect the endpoint: - -```bash -curl --fail --silent --show-error \ - http://127.0.0.1:9200/_plugins/_ppl/_grammar \ - --output ppl-grammar-bundle.json +./gradlew :ppl:exportPplGrammarBundle --no-daemon \ + -PpplGrammarBundleOutput="$PWD/ppl-grammar-bundle.json" jq -e ' (.grammarHash | test("^sha256:[0-9a-fA-F]{64}$")) and (.lexerSerializedATN | length > 0) and - (.parserSerializedATN | length > 0) and - (.lexerRuleNames | length > 0) and - (.parserRuleNames | length > 0) + (.parserSerializedATN | length > 0) ' ppl-grammar-bundle.json ``` -Stop the development cluster after capture. The wrapper is preferred for normal -use because its trap stops the process on success and failure. - -To run only the adapter after `resolved-target.json` and the bundle exist, -bootstrap the exact OSD checkout first with `yarn osd bootstrap` unless its -generated targets already match that revision: +To invoke only the adapter, bootstrap the exact OSD checkout first: ```bash SQL_ROOT=/absolute/path/to/sql OSD_ROOT=/absolute/path/to/OpenSearch-Dashboards cd "$OSD_ROOT" +yarn osd bootstrap node -r ./src/setup_node_env \ "$SQL_ROOT/scripts/ppl-lint/validate-osd-grammar.mjs" \ @@ -160,54 +125,55 @@ node -r ./src/setup_node_env \ ## Report results -Minimal passed result: +Schema version 2 records catalog classification as well as behavior: ```json { - "schemaVersion": 1, + "schemaVersion": 2, "status": "passed", "sql": {"sha": "", "targetBranch": "main", "version": "X.Y.Z"}, "osd": {"ref": "main", "sha": "", "version": "X.Y.Z"}, - "manualOverride": false, "grammarHash": "sha256:", - "rules": {"selected": 9, "passed": 9, "failed": 0}, - "caseCounts": {"selected": 18, "passed": 18, "failed": 0}, + "coverage": { + "catalogRuleIds": ["..."], + "requiredRuleIds": ["..."], + "coveredRuleIds": ["..."], + "excludedRuleIds": ["..."], + "excludedRules": [{"ruleId": "...", "reason": "..."}], + "missingRuleIds": [], + "unexpectedRuleIds": [], + "counts": { + "catalog": 18, "required": 16, "covered": 16, + "excluded": 2, "missing": 0, "unexpected": 0 + } + }, + "rules": { + "catalog": 18, "required": 16, "excluded": 2, + "selected": 16, "passed": 16, "failed": 0 + }, + "caseCounts": {"selected": 32, "passed": 32, "failed": 0}, "failures": [] } ``` -Minimal skipped result: - -```json -{ - "schemaVersion": 1, - "status": "skipped", - "skipReason": "osd-headless-grammar-api-unavailable", - "sql": {"sha": "", "targetBranch": "X.Y", "version": "X.Y.Z"}, - "osd": {"ref": "X.Y", "sha": "", "version": "X.Y.Z"}, - "rules": {"selected": 0, "passed": 0, "failed": 0}, - "caseCounts": {"selected": 0, "passed": 0, "failed": 0} -} -``` - -Exit codes: +A legacy skip has `status: "skipped"` and +`skipReason: "osd-headless-grammar-api-unavailable"`. | Code | Meaning | | ---: | --- | -| `0` | Passed, or skipped because the paired OSD API is absent | +| `0` | Passed, or skipped because the paired OSD API is genuinely absent | | `1` | A diagnostic count mismatch or per-case execution failure | -| `2` | Structural input, pairing, bundle, coverage, or advertised-API failure | +| `2` | Pairing, input, bundle, coverage, or advertised-API failure | ## Common outcomes | Outcome | Action | | --- | --- | -| Exact `X.Y` version mismatch | Verify both checkouts and branch-cut state; do not use OSD `main` | -| `osd-headless-grammar-api-unavailable` | No product fix; confirm exact metadata and successful skip | -| Module exists but exports fail to load | Treat as an OSD supported-path API regression | -| Cluster startup or grammar GET fails | Inspect `ppl-grammar-cluster.log` | -| Bundle is malformed or cannot deserialize | Check the SQL grammar endpoint schema and generated grammar | -| Rule lacks trigger/control coverage | Add the missing grammar case before interpreting detector results | -| Case names a missing OSD rule | Review the OSD catalog change, then update the stale SQL case | -| Trigger count drops | SQL grammar and OSD rule owners inspect parser node names and tree shape | -| Control count rises | OSD rule owner checks whether matching broadened intentionally | +| Exact `X.Y` version mismatch | Verify both checkouts and branch-cut state; do not substitute OSD `main` | +| `osd-headless-grammar-api-unavailable` | Confirm the exact old OSD SHA and the visible skip | +| Module resolves but import/exports fail | Treat as an OSD headless API regression | +| Export task fails | Inspect the `:ppl` build and generated ANTLR sources | +| Bundle cannot deserialize | Check exporter schema and generated grammar compatibility | +| Catalog coverage is incomplete | Add trigger/control cases or a justified exclusion | +| Trigger count drops | Inspect SQL grammar changes and the OSD rule's parse-tree assumptions | +| Control count rises | Check whether the OSD rule broadened intentionally | diff --git a/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs b/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs index 52b08f469c5..fc22e25a9d4 100644 --- a/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs +++ b/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs @@ -11,26 +11,25 @@ import path from 'node:path'; import { test } from 'node:test'; import { fileURLToPath } from 'node:url'; -const SCRIPT = fileURLToPath( - new URL('../validate-osd-grammar.mjs', import.meta.url) -); +const SCRIPT = fileURLToPath(new URL('../validate-osd-grammar.mjs', import.meta.url)); +const CORPUS = fileURLToPath(new URL('../grammar-cases.json', import.meta.url)); const HASH = `sha256:${'a'.repeat(64)}`; -const HEADLESS = path.join( +const HEADLESS_BASE = path.join( 'src', 'plugins', 'data', 'public', 'antlr', 'opensearch_ppl', - 'headless_ppl_lint.js' + 'headless_ppl_lint' ); -const CATALOG = path.join( +const CATALOG_BASE = path.join( 'packages', 'osd-monaco', 'src', 'ppl', 'lint', - 'catalog.js' + 'catalog' ); function writeJson(file, value) { @@ -38,6 +37,11 @@ function writeJson(file, value) { fs.writeFileSync(file, JSON.stringify(value)); } +function writeText(file, value) { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, value); +} + function makeTarget({ branch = '3.8', sqlVersion = '3.8.0', @@ -62,9 +66,14 @@ function makeTarget({ }; } -function defaultCases(ruleIds = ['rule-a']) { +function exclusion(ruleId, reason = `Reason for excluding ${ruleId}.`) { + return { ruleId, reason }; +} + +function defaultCases(ruleIds = ['rule-a'], excludedRules = []) { return { - schemaVersion: 1, + schemaVersion: 2, + excludedRules, cases: ruleIds.flatMap((ruleId) => [ { id: `${ruleId}-trigger`, @@ -86,44 +95,57 @@ function defaultCases(ruleIds = ['rule-a']) { }; } -function headlessSource({ - missingExport = false, - includeBuildTree = true, - directTree = false, -} = {}) { - return ` -exports.deserializeBundleOrThrow = (bundle) => ({ - grammarHash: bundle.grammarHash -}); -${includeBuildTree ? ` -exports.buildRuntimeTree = (query) => - query.includes('no-tree') - ? undefined - : query.includes('syntax-error') - ? { tree: { children: [{ constructor: { name: 'ErrorNode' } }] } } - : ${directTree ? '{}' : '{ tree: {} }'};` : ''} +function headlessSource({ format = 'cjs', missingExport = false, includeRecoveredTree = false } = {}) { + const declarations = ` +const deserializeBundleOrThrow = (bundle) => ({ grammarHash: bundle.grammarHash }); +${includeRecoveredTree ? ` +const buildRuntimeTree = () => ({ + tree: { children: [{ constructor: { name: 'ErrorNode' } }] } +});` : ''} ${missingExport ? '' : ` -exports.lintQueryWithBundle = (query, grammar, context) => { +const lintQueryWithBundle = (query, grammar, context) => { if (query.includes('throws')) throw new Error('detector crashed'); const target = Object.entries(context.overrides) .find(([, override]) => override.enabled)?.[0]; - if (query.includes('wrong-rule')) { - return { diagnostics: [{ ruleId: 'some-other-rule' }] }; + const diagnostics = []; + if (query.includes('trigger') || query.includes('target-diagnostic')) { + diagnostics.push({ ruleId: target }); } - return { - diagnostics: query.includes('trigger') ? [{ ruleId: target }] : [] - }; + if (query.includes('wrong-rule')) diagnostics.push({ ruleId: 'some-other-rule' }); + if (query.includes('missing-rule-id')) diagnostics.push({}); + return { diagnostics }; };`} `; + const names = [ + 'deserializeBundleOrThrow', + ...(includeRecoveredTree ? ['buildRuntimeTree'] : []), + ...(missingExport ? [] : ['lintQueryWithBundle']), + ]; + if (format === 'esm') { + return `${declarations}\nexport { ${names.join(', ')} };\n`; + } + return `${declarations}\n${names.map((name) => `exports.${name} = ${name};`).join('\n')}\n`; +} + +function headlessPath(osdRoot, layout) { + const base = path.join(osdRoot, HEADLESS_BASE); + if (layout === 'directory') return path.join(base, 'index.js'); + if (layout === 'esm-js') return `${base}.js`; + return `${base}.${layout}`; } function makeFixture( t, { target = makeTarget(), - ruleIds = ['rule-a'], - cases = defaultCases(ruleIds), + catalogRuleIds = ['rule-a'], + coveredRuleIds = catalogRuleIds, + excludedRules = [], + cases = defaultCases(coveredRuleIds, excludedRules), api = 'valid', + headlessLayout = 'js', + includeRecoveredTree = false, + catalogPresent = true, } = {} ) { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-osd-grammar-')); @@ -140,23 +162,32 @@ function makeFixture( writeJson(casesPath, cases); if (api !== 'absent') { - const headlessPath = path.join(osdRoot, HEADLESS); - fs.mkdirSync(path.dirname(headlessPath), { recursive: true }); - fs.writeFileSync( - headlessPath, - headlessSource({ - missingExport: api === 'missing-export', - includeBuildTree: api !== 'no-build-tree', - directTree: api === 'direct-tree', - }) - ); - const catalogPath = path.join(osdRoot, CATALOG); - fs.mkdirSync(path.dirname(catalogPath), { recursive: true }); - fs.writeFileSync( - catalogPath, + const modulePath = headlessPath(osdRoot, headlessLayout); + if (api === 'import-failure') { + writeText(modulePath, `throw new Error('top-level import failed');\n`); + } else if (api === 'transitive-failure') { + writeText(modulePath, `require('./missing-transitive-dependency');\n`); + } else { + if (headlessLayout === 'esm-js') { + writeJson(path.join(path.dirname(modulePath), 'package.json'), { type: 'module' }); + } + writeText( + modulePath, + headlessSource({ + format: ['mjs', 'esm-js'].includes(headlessLayout) ? 'esm' : 'cjs', + missingExport: api === 'missing-export', + includeRecoveredTree, + }) + ); + } + } + + if (catalogPresent) { + writeText( + `${path.join(osdRoot, CATALOG_BASE)}.js`, `exports.getBundledCatalog = () => ${JSON.stringify( - ruleIds.map((id) => ({ id })) - )};` + catalogRuleIds.map((id) => ({ id })) + )};\n` ); } @@ -171,7 +202,7 @@ function makeFixture( }; } -function invoke(fixture, extraArgs = []) { +function invoke(fixture) { return spawnSync( process.execPath, [ @@ -190,7 +221,6 @@ function invoke(fixture, extraArgs = []) { fixture.reportPath, '--summary', fixture.summaryPath, - ...extraArgs, ], { encoding: 'utf8' } ); @@ -200,7 +230,7 @@ function readReport(fixture) { return JSON.parse(fs.readFileSync(fixture.reportPath, 'utf8')); } -test('main accepts different SQL and OSD product lines and records exact metadata', (t) => { +test('schema-v2 report records deterministic coverage and exact metadata', (t) => { const fixture = makeFixture(t, { target: makeTarget({ branch: 'main', @@ -208,45 +238,45 @@ test('main accepts different SQL and OSD product lines and records exact metadat osdVersion: '3.8.2', osdRef: 'main', }), + catalogRuleIds: ['rule-b', 'rule-a', 'explain-rule'], + coveredRuleIds: ['rule-b', 'rule-a'], + excludedRules: [exclusion('explain-rule')], }); const result = invoke(fixture); assert.equal(result.status, 0, result.stderr); const report = readReport(fixture); + assert.equal(report.schemaVersion, 2); assert.equal(report.status, 'passed'); assert.equal(report.sql.version, '3.9.0'); assert.equal(report.osd.version, '3.8.2'); assert.equal(report.osd.sha, 'osd-immutable-sha'); - assert.equal(report.manualOverride, false); assert.equal(report.grammarHash, HASH); - assert.match(fs.readFileSync(fixture.summaryPath, 'utf8'), /Status: \*\*passed\*\*/); -}); - -test('matching exact release versions accept a RuntimeParseOutcome tree', (t) => { - const fixture = makeFixture(t); - const result = invoke(fixture); - - assert.equal(result.status, 0, result.stderr); - const report = readReport(fixture); - assert.equal(report.cases[0].parseTreeCheck, 'verified'); + assert.deepEqual(report.coverage.catalogRuleIds, ['explain-rule', 'rule-a', 'rule-b']); + assert.deepEqual(report.coverage.requiredRuleIds, ['rule-a', 'rule-b']); + assert.deepEqual(report.coverage.coveredRuleIds, ['rule-a', 'rule-b']); + assert.deepEqual(report.coverage.excludedRuleIds, ['explain-rule']); + assert.deepEqual(report.coverage.missingRuleIds, []); + assert.deepEqual(report.coverage.unexpectedRuleIds, []); + assert.deepEqual(report.coverage.counts, { + catalog: 3, + required: 2, + covered: 2, + excluded: 1, + missing: 0, + unexpected: 0, + }); assert.deepEqual(report.rules, { - selected: 1, - passed: 1, + catalog: 3, + required: 2, + excluded: 1, + selected: 2, + passed: 2, failed: 0, }); }); -test('matching exact release versions accept a direct ParserRuleContext', (t) => { - const fixture = makeFixture(t, { api: 'direct-tree' }); - const result = invoke(fixture); - - assert.equal(result.status, 0, result.stderr); - const report = readReport(fixture); - assert.equal(report.status, 'passed'); - assert.ok(report.cases.every((entry) => entry.parseTreeCheck === 'verified')); -}); - test('exact release branch version mismatch is structural and writes a report', (t) => { const fixture = makeFixture(t, { target: makeTarget({ branch: '3.8', sqlVersion: '3.9.0' }), @@ -255,48 +285,29 @@ test('exact release branch version mismatch is structural and writes a report', assert.equal(result.status, 2); assert.match(result.stderr, /requires SQL and OSD 3\.8\.x versions/); - const report = readReport(fixture); - assert.equal(report.status, 'error'); - assert.equal(report.releaseLineValidationBypassed, false); - assert.match(report.error, /SQL 3\.9\.0/); + assert.equal(readReport(fixture).status, 'error'); }); -test('explicit release-line bypass permits mismatched product lines', (t) => { - const fixture = makeFixture(t, { - target: makeTarget({ - branch: '3.8', - sqlVersion: '3.9.0', - osdVersion: '3.7.2', - releaseLineValidationBypassed: true, - }), +test('explicit release-line bypass and manual override remain visible', (t) => { + const target = makeTarget({ + branch: '3.8', + sqlVersion: '3.9.0', + osdVersion: '3.7.2', + osdRef: 'candidate', + releaseLineValidationBypassed: true, }); - const result = invoke(fixture); - - assert.equal(result.status, 0, result.stderr); - const report = readReport(fixture); - assert.equal(report.status, 'passed'); - assert.equal(report.releaseLineValidationBypassed, true); - assert.equal(report.sql.version, '3.9.0'); - assert.equal(report.osd.version, '3.7.2'); - assert.match( - fs.readFileSync(fixture.summaryPath, 'utf8'), - /Release-line validation bypassed: `true`/ - ); -}); - -test('manual OSD override is explicit in the report and summary', (t) => { - const target = makeTarget({ branch: 'main', osdRef: 'candidate' }); target.osd.override = true; const fixture = makeFixture(t, { target }); + const result = invoke(fixture); assert.equal(result.status, 0, result.stderr); const report = readReport(fixture); assert.equal(report.manualOverride, true); - assert.match(fs.readFileSync(fixture.summaryPath, 'utf8'), /Manual OSD override: `true`/); + assert.equal(report.releaseLineValidationBypassed, true); }); -test('absent headless API skips before grammar and cases are read', (t) => { +test('true headless target-module absence is the only capability skip', (t) => { const fixture = makeFixture(t, { api: 'absent' }); fs.rmSync(fixture.grammarPath); fs.rmSync(fixture.casesPath); @@ -306,15 +317,40 @@ test('absent headless API skips before grammar and cases are read', (t) => { assert.equal(result.status, 0, result.stderr); const report = readReport(fixture); assert.equal(report.status, 'skipped'); - assert.equal( - report.skipReason, - 'osd-headless-grammar-api-unavailable' - ); - assert.equal(report.caseCounts.selected, 0); - assert.deepEqual(report.cases, []); + assert.equal(report.skipReason, 'osd-headless-grammar-api-unavailable'); + assert.equal(report.schemaVersion, 2); + assert.deepEqual(report.coverage.catalogRuleIds, []); +}); + +for (const layout of ['js', 'ts', 'mjs', 'esm-js', 'directory']) { + test(`headless ${layout} module layout is resolved and loaded by the adapter`, (t) => { + const fixture = makeFixture(t, { headlessLayout: layout }); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + assert.equal(readReport(fixture).status, 'passed'); + }); +} + +test('resolved target-module import failure is structural, not skipped', (t) => { + const fixture = makeFixture(t, { api: 'import-failure' }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /Could not import OSD module.*top-level import failed/); + assert.equal(readReport(fixture).status, 'error'); }); -test('an advertised API with a missing export fails structurally', (t) => { +test('resolved target-module transitive dependency failure is structural', (t) => { + const fixture = makeFixture(t, { api: 'transitive-failure' }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /missing-transitive-dependency/); + assert.equal(readReport(fixture).status, 'error'); +}); + +test('resolved target module with a missing export is structural', (t) => { const fixture = makeFixture(t, { api: 'missing-export' }); const result = invoke(fixture); @@ -323,6 +359,15 @@ test('an advertised API with a missing export fails structurally', (t) => { assert.equal(readReport(fixture).status, 'error'); }); +test('missing catalog after a present headless API is structural', (t) => { + const fixture = makeFixture(t, { catalogPresent: false }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /Could not resolve an OSD catalog module/); + assert.equal(readReport(fixture).status, 'error'); +}); + test('a malformed bundle fails structurally when capability is present', (t) => { const fixture = makeFixture(t); fs.writeFileSync(fixture.grammarPath, '{not json'); @@ -330,12 +375,33 @@ test('a malformed bundle fails structurally when capability is present', (t) => assert.equal(result.status, 2); assert.match(result.stderr, /Could not .*parse grammar bundle/); - assert.equal(readReport(fixture).status, 'error'); }); -test('diagnostic count mismatch returns one and preserves every normalized case', (t) => { +test('recovered parse trees are ignored when diagnostics match', (t) => { + const cases = defaultCases(); + cases.cases[1].query = 'source=accounts | recovered-syntax control'; + const fixture = makeFixture(t, { cases, includeRecoveredTree: true }); + + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.equal(report.status, 'passed'); + assert.ok(report.cases.every((entry) => !('parseTreeCheck' in entry))); +}); + +test('buildRuntimeTree is not required or reported', (t) => { + const fixture = makeFixture(t); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); + const report = readReport(fixture); + assert.ok(report.cases.every((entry) => !('parseTreeCheck' in entry))); +}); + +test('diagnostic count mismatch returns one and preserves normalized cases', (t) => { const cases = defaultCases(); - cases.cases[0].query = 'source=accounts | wrong-rule trigger'; + cases.cases[0].query = 'source=accounts | no-diagnostic'; const fixture = makeFixture(t, { cases }); const result = invoke(fixture); @@ -343,23 +409,29 @@ test('diagnostic count mismatch returns one and preserves every normalized case' const report = readReport(fixture); assert.equal(report.status, 'failed'); assert.equal(report.cases.length, 2); - assert.equal(report.cases[0].status, 'failed'); assert.equal(report.cases[0].actualCount, 0); assert.equal(report.cases[1].status, 'passed'); - assert.deepEqual(report.failures[0], { - ruleId: 'rule-a', - caseId: 'rule-a-trigger', - query: 'source=accounts | wrong-rule trigger', - expectedCount: 1, - actualCount: 0, - }); }); -test('one case exception does not hide later cases and the failure report exists', (t) => { +test('non-target diagnostics fail even when the target count matches', (t) => { + const cases = defaultCases(); + cases.cases[0].query = 'source=accounts | target-diagnostic wrong-rule'; + const fixture = makeFixture(t, { cases }); + const result = invoke(fixture); + + assert.equal(result.status, 1); + const report = readReport(fixture); + assert.equal(report.cases[0].actualCount, 1); + assert.deepEqual(report.cases[0].unexpectedDiagnosticRuleIds, ['some-other-rule']); + assert.match(report.cases[0].error, /non-target rule/); + assert.deepEqual(report.failures[0].unexpectedDiagnosticRuleIds, ['some-other-rule']); +}); + +test('one lint exception does not hide later cases', (t) => { const cases = defaultCases(['rule-a', 'rule-b']); cases.cases[0].query = 'source=accounts | throws'; const fixture = makeFixture(t, { - ruleIds: ['rule-a', 'rule-b'], + catalogRuleIds: ['rule-a', 'rule-b'], cases, }); const result = invoke(fixture); @@ -370,84 +442,151 @@ test('one case exception does not hide later cases and the failure report exists assert.equal(report.cases.length, 4); assert.equal(report.cases[0].error, 'detector crashed'); assert.equal(report.cases[2].status, 'passed'); - assert.deepEqual(report.rules, { - selected: 2, - passed: 1, - failed: 1, +}); + +test('deleting a rule corpus fails catalog coverage with deterministic missing data', (t) => { + const fixture = makeFixture(t, { + catalogRuleIds: ['rule-b', 'rule-a'], + coveredRuleIds: ['rule-a'], }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /missing: rule-b/); + const report = readReport(fixture); + assert.deepEqual(report.coverage.requiredRuleIds, ['rule-a', 'rule-b']); + assert.deepEqual(report.coverage.coveredRuleIds, ['rule-a']); + assert.deepEqual(report.coverage.missingRuleIds, ['rule-b']); + assert.equal(report.coverage.counts.missing, 1); }); -test('a missing parse tree is a case failure and later cases still execute', (t) => { - const cases = defaultCases(); - cases.cases[0].query = 'source=accounts | no-tree'; - const fixture = makeFixture(t, { cases }); +test('a newly added catalog rule fails until it is classified', (t) => { + const fixture = makeFixture(t, { + catalogRuleIds: ['rule-a', 'new-rule'], + coveredRuleIds: ['rule-a'], + }); const result = invoke(fixture); - assert.equal(result.status, 1); + assert.equal(result.status, 2); + assert.match(result.stderr, /missing: new-rule/); + assert.deepEqual(readReport(fixture).coverage.missingRuleIds, ['new-rule']); +}); + +test('a reasoned exclusion removes a catalog rule from the required set', (t) => { + const fixture = makeFixture(t, { + catalogRuleIds: ['rule-a', 'explain-rule'], + coveredRuleIds: ['rule-a'], + excludedRules: [exclusion('explain-rule', 'Requires backend explain data.')], + }); + const result = invoke(fixture); + + assert.equal(result.status, 0, result.stderr); const report = readReport(fixture); - assert.match(report.cases[0].error, /no parse tree/); - assert.equal(report.cases[1].status, 'passed'); + assert.deepEqual(report.coverage.requiredRuleIds, ['rule-a']); + assert.deepEqual(report.coverage.excludedRules, [ + { ruleId: 'explain-rule', reason: 'Requires backend explain data.' }, + ]); }); -test('a recovered syntax error cannot pass as a zero-diagnostic control', (t) => { +test('a stale exclusion is structural and reported as unexpected', (t) => { + const fixture = makeFixture(t, { + catalogRuleIds: ['rule-a'], + coveredRuleIds: ['rule-a'], + excludedRules: [exclusion('removed-rule')], + }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /Excluded rule.*absent.*removed-rule/); + assert.deepEqual(readReport(fixture).coverage.unexpectedRuleIds, ['removed-rule']); +}); + +test('duplicate exclusion IDs are structural', (t) => { + const duplicate = [exclusion('explain-rule'), exclusion('explain-rule', 'Second reason.')]; + const fixture = makeFixture(t, { + catalogRuleIds: ['rule-a', 'explain-rule'], + coveredRuleIds: ['rule-a'], + excludedRules: duplicate, + }); + const result = invoke(fixture); + + assert.equal(result.status, 2); + assert.match(result.stderr, /Duplicate exclusion rule ID.*explain-rule/); + assert.deepEqual(readReport(fixture).coverage.catalogRuleIds, ['explain-rule', 'rule-a']); +}); + +test('duplicate case IDs are structural and retain coverage details', (t) => { const cases = defaultCases(); - cases.cases[1].query = 'source=accounts | syntax-error'; + cases.cases[1].id = cases.cases[0].id; const fixture = makeFixture(t, { cases }); const result = invoke(fixture); - assert.equal(result.status, 1); - const report = readReport(fixture); - assert.equal(report.cases[0].status, 'passed'); - assert.equal(report.cases[1].status, 'failed'); - assert.match(report.cases[1].error, /recovered from a syntax error/); + assert.equal(result.status, 2); + assert.match(result.stderr, /Duplicate case ID.*rule-a-trigger/); + assert.deepEqual(readReport(fixture).coverage.coveredRuleIds, ['rule-a']); }); -test('controls fail explicitly when buildRuntimeTree is not exported', (t) => { - const fixture = makeFixture(t, { api: 'no-build-tree' }); +test('a rule cannot be both covered and excluded', (t) => { + const fixture = makeFixture(t, { + excludedRules: [exclusion('rule-a')], + }); const result = invoke(fixture); - assert.equal(result.status, 1); - const report = readReport(fixture); - assert.equal(report.cases[0].status, 'passed'); - assert.equal( - report.cases[0].parseTreeCheck, - 'inferred-from-target-diagnostic' - ); - assert.equal(report.cases[1].status, 'failed'); - assert.equal(report.cases[1].parseTreeCheck, 'unavailable'); - assert.match(report.cases[1].error, /control parse tree cannot be verified/); + assert.equal(result.status, 2); + assert.match(result.stderr, /both covered and excluded: rule-a/); }); -test('anti-vacuous case validation requires trigger and control coverage per rule', (t) => { +test('a case naming an unknown OSD rule is structural and reported', (t) => { + const cases = defaultCases(['rule-a', 'unknown-rule']); const fixture = makeFixture(t, { - cases: { - schemaVersion: 1, - cases: [ - { - id: 'only-trigger', - ruleId: 'rule-a', - kind: 'trigger', - query: 'source=accounts | rule-a trigger', - expectedCount: 1, - }, - ], - }, + catalogRuleIds: ['rule-a'], + cases, }); const result = invoke(fixture); + assert.equal(result.status, 2); + assert.match(result.stderr, /Case rule.*absent.*unknown-rule/); + assert.deepEqual(readReport(fixture).coverage.unexpectedRuleIds, ['unknown-rule']); +}); + +test('every required rule needs trigger and control classifications', (t) => { + const cases = defaultCases(); + cases.cases.pop(); + const fixture = makeFixture(t, { cases }); + const result = invoke(fixture); + assert.equal(result.status, 2); assert.match(result.stderr, /must have trigger and control cases/); assert.equal(readReport(fixture).status, 'error'); }); -test('a case naming a missing OSD rule fails structurally', (t) => { - const fixture = makeFixture(t, { - ruleIds: ['different-rule'], - cases: defaultCases(['rule-a']), - }); +test('case documents must use schema version two', (t) => { + const cases = defaultCases(); + cases.schemaVersion = 1; + const fixture = makeFixture(t, { cases }); const result = invoke(fixture); assert.equal(result.status, 2); - assert.match(result.stderr, /names missing OSD rule "rule-a"/); - assert.equal(readReport(fixture).status, 'error'); + assert.match(result.stderr, /schemaVersion must be 2/); +}); + +test('the repository corpus covers 16 rules and excludes only two explain rules', () => { + const corpus = JSON.parse(fs.readFileSync(CORPUS, 'utf8')); + const coveredRuleIds = [...new Set(corpus.cases.map((entry) => entry.ruleId))].sort(); + const excludedRuleIds = corpus.excludedRules.map((entry) => entry.ruleId).sort(); + + assert.equal(corpus.schemaVersion, 2); + assert.equal(corpus.cases.length, 32); + assert.equal(coveredRuleIds.length, 16); + assert.deepEqual(excludedRuleIds, [ + 'operation-not-pushed', + 'operation-pushed-as-script', + ]); + for (const ruleId of coveredRuleIds) { + const kinds = corpus.cases + .filter((entry) => entry.ruleId === ruleId) + .map((entry) => entry.kind) + .sort(); + assert.deepEqual(kinds, ['control', 'trigger'], ruleId); + } }); diff --git a/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs b/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs index ab43aa0d919..3c52008e22b 100644 --- a/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs +++ b/scripts/ppl-lint/__tests__/workflow-pairing.test.mjs @@ -20,6 +20,7 @@ const WORKFLOW = path.join( ); const WRAPPER = path.join(ROOT, 'scripts', 'ppl-lint-rule-validation.sh'); const SOURCE = fs.readFileSync(WORKFLOW, 'utf8'); +const WRAPPER_SOURCE = fs.readFileSync(WRAPPER, 'utf8'); function stepScript(name) { const lines = SOURCE.split('\n'); @@ -126,9 +127,7 @@ function resolveTarget( version = '4.2.1-SNAPSHOT', event = 'pull_request', target = 'main', - osdRepo = '', osdRef = '', - bypass = '', } = {} ) { const directory = initializeSqlCheckout(t, version); @@ -141,9 +140,7 @@ function resolveTarget( EVENT_NAME: event, TARGET_BRANCH: target, SQL_HEAD_SHA: 'pull-request-head', - REQUESTED_OSD_REPO: osdRepo, REQUESTED_OSD_REF: osdRef, - REQUESTED_BYPASS: bypass, GITHUB_OUTPUT: output, }, }); @@ -153,34 +150,6 @@ function resolveTarget( }; } -function probeCapability(t, extension) { - const directory = temporaryDirectory(t); - const output = path.join(directory, 'github-output'); - if (extension) { - const module = path.join( - directory, - '.ci', - 'OpenSearch-Dashboards', - 'src', - 'plugins', - 'data', - 'public', - 'antlr', - 'opensearch_ppl', - `headless_ppl_lint.${extension}` - ); - fs.mkdirSync(path.dirname(module), { recursive: true }); - fs.writeFileSync(module, ''); - } - const result = spawnSync('/bin/bash', ['-c', stepScript('Detect headless grammar capability')], { - cwd: directory, - encoding: 'utf8', - env: { ...process.env, GITHUB_OUTPUT: output }, - }); - assert.equal(result.status, 0, result.stderr); - return readOutputs(output).available; -} - test('workflow has focused triggers, read-only permissions, and no fixed product version', () => { assert.match(SOURCE, /^ pull_request:$/m); assert.match(SOURCE, /^ push:$/m); @@ -191,8 +160,13 @@ test('workflow has focused triggers, read-only permissions, and no fixed product assert.doesNotMatch(SOURCE, /^\s+schedule:$/m); assert.doesNotMatch(SOURCE, /\b(?:secrets|vars)\./); assert.doesNotMatch(SOURCE, /compiled-version|latestEligibleGa|release-tags/); + assert.doesNotMatch(SOURCE, /allow_release_line_mismatch|inputs\.osd_repo|REQUESTED_OSD_REPO/); + assert.doesNotMatch(SOURCE, /settings\.gradle/); + assert.doesNotMatch(SOURCE, /plugin\/(?:build\.gradle|src\/main)/); assert.equal( - SOURCE.match(/plugin\/src\/main\/java\/org\/opensearch\/sql\/plugin\/SQLPlugin\.java/g) + SOURCE.match( + /ppl\/src\/main\/java\/org\/opensearch\/sql\/ppl\/autocomplete\/PPLGrammarBundleExporter\.java/g + ) ?.length, 2 ); @@ -203,37 +177,53 @@ test('workflow has focused triggers, read-only permissions, and no fixed product assert.deepEqual(fixedProductVersions, []); }); -test('skip path uses the adapter before bootstrap and supports TypeScript or JavaScript modules', (t) => { - const capability = SOURCE.indexOf(' - name: Detect headless grammar capability'); - const skip = SOURCE.indexOf(' - name: Record unsupported paired branch with adapter'); +test('workflow and wrapper delegate capability to one adapter invocation', () => { + const exportGrammar = SOURCE.indexOf(' - name: Export candidate runtime grammar'); const java = SOURCE.indexOf(' - name: Set up JDK 21'); - const capture = SOURCE.indexOf(' - name: Capture candidate runtime grammar'); const bootstrap = SOURCE.indexOf(' - name: Bootstrap OpenSearch Dashboards'); - assert.ok(capability < skip); - assert.ok(skip < java); - assert.ok(skip < capture); - assert.ok(skip < bootstrap); + const validate = SOURCE.indexOf(' - name: Validate OSD linter against candidate grammar'); + assert.ok(java < exportGrammar); + assert.ok(exportGrammar < bootstrap); + assert.ok(bootstrap < validate); - const skipScript = stepScript('Record unsupported paired branch with adapter'); + const exportScript = stepScript('Export candidate runtime grammar'); + assert.match(exportScript, /\.\/gradlew :ppl:exportPplGrammarBundle --no-daemon/); assert.match( - skipScript, - /^node "\$GITHUB_WORKSPACE\/scripts\/ppl-lint\/validate-osd-grammar\.mjs"/m + exportScript, + /"-PpplGrammarBundleOutput=\$GITHUB_WORKSPACE\/ppl-grammar-bundle\.json"/ ); - assert.doesNotMatch(skipScript, /node -r /); - assert.match(skipScript, /--grammar "\$GITHUB_WORKSPACE\/ppl-grammar-bundle\.json"/); - assert.match(skipScript, /--summary "\$GITHUB_STEP_SUMMARY"/); + const validateScript = stepScript('Validate OSD linter against candidate grammar'); + assert.match(validateScript, /^set \+e$/m); + assert.match(validateScript, /^node -r \.\/src\/setup_node_env/m); + assert.match(validateScript, /--summary "\$GITHUB_STEP_SUMMARY"/); - assert.equal(probeCapability(t, 'ts'), 'true'); - assert.equal(probeCapability(t, 'js'), 'true'); - assert.equal(probeCapability(t), 'false'); + assert.equal( + SOURCE.match(/scripts\/ppl-lint\/validate-osd-grammar\.mjs/g)?.length, + 1 + ); + assert.equal(WRAPPER_SOURCE.match(/node -r \.\/src\/setup_node_env/g)?.length, 1); + for (const content of [SOURCE, WRAPPER_SOURCE]) { + assert.doesNotMatch(content, /Detect headless grammar capability/); + assert.doesNotMatch(content, /headless_ppl_lint/); + assert.doesNotMatch(content, /steps\.capability/); + assert.doesNotMatch(content, /opensearch-sql-plugin:run/); + assert.doesNotMatch(content, /\bcurl\b|127\.0\.0\.1:9200|ppl-grammar-cluster\.log/); + assert.doesNotMatch(content, /\btrap\b|gradle_pid|GRADLE_PID/); + } + assert.match(WRAPPER_SOURCE, /\.\/gradlew :ppl:exportPplGrammarBundle --no-daemon/); }); test('pre-adapter failure fallback writes the structural report contract without target metadata', (t) => { const fallback = stepScript('Record pre-report failure'); assert.match(fallback, /ppl-lint-grammar-compatibility-report\.json/); + assert.match(fallback, /schemaVersion: 2/); assert.match(fallback, /status: "error"/); assert.match(fallback, /error: \$error/); assert.match(fallback, /manualOverride: \(\.osd\.override \/\/ false\)/); + assert.match(fallback, /releaseLineValidationBypassed: false/); + assert.match(fallback, /catalogRuleIds: \[\]/); + assert.match(fallback, /excludedRuleIds: \[\]/); + assert.match(fallback, /excludedRules: \[\]/); assert.match(fallback, /caseCounts: \{selected: 0, passed: 0, failed: 0\}/); assert.match(fallback, /cases: \[\]/); assert.match(fallback, /failures: \[\]/); @@ -259,8 +249,25 @@ test('pre-adapter failure fallback writes the structural report contract without ) ); assert.equal(report.status, 'error'); + assert.equal(report.schemaVersion, 2); assert.match(report.error, /before the compatibility adapter/); - assert.deepEqual(report.rules, { selected: 0, passed: 0, failed: 0 }); + assert.deepEqual(report.rules, { + catalog: 0, + required: 0, + excluded: 0, + selected: 0, + passed: 0, + failed: 0, + }); + assert.deepEqual(report.coverage.counts, { + catalog: 0, + required: 0, + covered: 0, + excluded: 0, + missing: 0, + unexpected: 0, + }); + assert.deepEqual(report.coverage.missingRuleIds, []); assert.equal('sql' in report, false); }); @@ -285,23 +292,28 @@ test('extracted resolver rejects mismatches and limits overrides to dispatch', ( const pushOverride = resolveTarget(t, { event: 'push', - osdRepo: 'example/OpenSearch-Dashboards', + osdRef: 'candidate', }); assert.notEqual(pushOverride.result.status, 0); - assert.match(pushOverride.result.stdout, /allowed only for workflow_dispatch/); + assert.match(pushOverride.result.stdout, /ref overrides are allowed only for workflow_dispatch/); const dispatch = resolveTarget(t, { event: 'workflow_dispatch', - target: '4.1', - osdRepo: 'example/OpenSearch-Dashboards', osdRef: 'candidate', - bypass: 'true', }); assert.equal(dispatch.result.status, 0, dispatch.result.stderr); - assert.equal(dispatch.outputs.osd_repo, 'example/OpenSearch-Dashboards'); + assert.equal(dispatch.outputs.osd_repo, 'opensearch-project/OpenSearch-Dashboards'); assert.equal(dispatch.outputs.osd_ref, 'candidate'); assert.equal(dispatch.outputs.osd_override, 'true'); - assert.equal(dispatch.outputs.release_line_bypass, 'true'); + assert.equal('release_line_bypass' in dispatch.outputs, false); + + const dispatchMismatch = resolveTarget(t, { + event: 'workflow_dispatch', + target: '4.1', + osdRef: 'candidate', + }); + assert.notEqual(dispatchMismatch.result.status, 0); + assert.match(dispatchMismatch.result.stdout, /does not match target branch 4\.1/); const featureBranch = resolveTarget(t, { target: 'feature/test' }); assert.notEqual(featureBranch.result.status, 0); @@ -312,7 +324,7 @@ test('extracted resolver rejects mismatches and limits overrides to dispatch', ( assert.match(malformedVersion.result.stdout, /Invalid SQL product version/); }); -test('wrapper reproduces a dispatch release-line bypass on exact checkouts', (t) => { +test('wrapper rejects stale target metadata before exporting a grammar', (t) => { const directory = temporaryDirectory(t); const osdRoot = initializeOsdCheckout(t); const target = path.join(directory, 'target.json'); @@ -326,18 +338,17 @@ test('wrapper reproduces a dispatch release-line bypass on exact checkouts', (t) target, JSON.stringify({ sql: { - sha: gitHead(ROOT), - targetBranch: '9.9', + sha: 'stale-sql-sha', + targetBranch: 'main', versionRaw: sqlVersionRaw, version: sqlVersion, }, osd: { repository: 'local/OpenSearch-Dashboards', - ref: '9.9', + ref: 'main', sha: gitHead(osdRoot), version: '3.7.0', }, - releaseLineValidationBypassed: true, }) ); @@ -346,10 +357,9 @@ test('wrapper reproduces a dispatch release-line bypass on exact checkouts', (t) ['--target', target, '--osd-root', osdRoot, '--report', report, '--summary', summary], { cwd: ROOT, encoding: 'utf8' } ); - assert.equal(result.status, 0, result.stderr); - const output = JSON.parse(fs.readFileSync(report, 'utf8')); - assert.equal(output.status, 'skipped'); - assert.equal(output.releaseLineValidationBypassed, true); + assert.equal(result.status, 2); + assert.match(result.stderr, /target SQL SHA stale-sql-sha does not match checkout/); + assert.equal(fs.existsSync(report), false); }); test('wrapper refuses to label an unrelated local OSD checkout as the paired ref', (t) => { diff --git a/scripts/ppl-lint/grammar-cases.json b/scripts/ppl-lint/grammar-cases.json index 7fb9d234c1c..d39504c6c09 100644 --- a/scripts/ppl-lint/grammar-cases.json +++ b/scripts/ppl-lint/grammar-cases.json @@ -1,5 +1,15 @@ { - "schemaVersion": 1, + "schemaVersion": 2, + "excludedRules": [ + { + "ruleId": "operation-not-pushed", + "reason": "Requires backend explain data; backend execution is outside this grammar/linter check." + }, + { + "ruleId": "operation-pushed-as-script", + "reason": "Requires backend explain data; backend execution is outside this grammar/linter check." + } + ], "cases": [ { "id": "head-without-sort", @@ -150,6 +160,182 @@ "query": "source=accounts | rex field=email \"(?[^@]+)@(?.+)\"", "expectedCount": 0, "context": { "isCalcite": true } + }, + { + "id": "unknown-field-reference", + "ruleId": "field-validation", + "kind": "trigger", + "query": "source=accounts | where missing_field = 1", + "expectedCount": 1, + "context": { + "fields": ["age"] + } + }, + { + "id": "known-field-reference-control", + "ruleId": "field-validation", + "kind": "control", + "query": "source=accounts | where age = 1", + "expectedCount": 0, + "context": { + "fields": ["age"] + } + }, + { + "id": "aggregate-text-field", + "ruleId": "agg-on-text", + "kind": "trigger", + "query": "source=accounts | stats avg(name)", + "expectedCount": 1, + "context": { + "isCalcite": true, + "fields": ["name", "age"], + "typeMap": { + "name": "text", + "age": "long" + } + } + }, + { + "id": "aggregate-numeric-field-control", + "ruleId": "agg-on-text", + "kind": "control", + "query": "source=accounts | stats avg(age)", + "expectedCount": 0, + "context": { + "isCalcite": true, + "fields": ["name", "age"], + "typeMap": { + "name": "text", + "age": "long" + } + } + }, + { + "id": "flat-object-subfield-reference", + "ruleId": "flat-object-subfield", + "kind": "trigger", + "query": "source=accounts | fields attributes.child", + "expectedCount": 1, + "context": { + "isCalcite": true, + "typeMap": { + "attributes": "flat_object", + "name": "text" + } + } + }, + { + "id": "non-flat-field-reference-control", + "ruleId": "flat-object-subfield", + "kind": "control", + "query": "source=accounts | fields name", + "expectedCount": 0, + "context": { + "isCalcite": true, + "typeMap": { + "attributes": "flat_object", + "name": "text" + } + } + }, + { + "id": "numeric-field-text-comparison", + "ruleId": "type-mismatch-numeric", + "kind": "trigger", + "query": "source=accounts | where age = \"thirty\"", + "expectedCount": 1, + "context": { + "isCalcite": true, + "typeMap": { + "age": "long" + } + } + }, + { + "id": "numeric-field-coercible-comparison-control", + "ruleId": "type-mismatch-numeric", + "kind": "control", + "query": "source=accounts | where age = \"32\"", + "expectedCount": 0, + "context": { + "isCalcite": true, + "typeMap": { + "age": "long" + } + } + }, + { + "id": "disabled-object-subfield-reference", + "ruleId": "enabled-false-object", + "kind": "trigger", + "query": "source=accounts | where session.id = 1", + "expectedCount": 1, + "context": { + "isCalcite": true, + "fields": ["status"], + "disabledObjectFields": ["session"] + } + }, + { + "id": "searchable-field-reference-control", + "ruleId": "enabled-false-object", + "kind": "control", + "query": "source=accounts | where status = 1", + "expectedCount": 0, + "context": { + "isCalcite": true, + "fields": ["status"], + "disabledObjectFields": ["session"] + } + }, + { + "id": "wildcard-source-without-match", + "ruleId": "wildcard-source-zero-match", + "kind": "trigger", + "query": "source=nope-*", + "expectedCount": 1, + "context": { + "visibleIndices": ["logs-2024", "accounts"] + } + }, + { + "id": "wildcard-source-with-match-control", + "ruleId": "wildcard-source-zero-match", + "kind": "control", + "query": "source=logs-*", + "expectedCount": 0, + "context": { + "visibleIndices": ["logs-2024", "accounts"] + } + }, + { + "id": "rex-over-text-field", + "ruleId": "rex-scan-cost", + "kind": "trigger", + "query": "source=logs | rex field=raw_log \"GET (?\\S+)\"", + "expectedCount": 1, + "context": { + "fields": ["raw_log", "host"], + "typeMap": { + "raw_log": "text", + "host": "keyword" + } + } + }, + { + "id": "rex-over-keyword-field-control", + "ruleId": "rex-scan-cost", + "kind": "control", + "query": "source=logs | rex field=host \"(?\\S+)\"", + "expectedCount": 0, + "context": { + "fields": ["raw_log", "host"], + "typeMap": { + "raw_log": "text", + "host": "keyword" + } + } } ] } diff --git a/scripts/ppl-lint/validate-osd-grammar.mjs b/scripts/ppl-lint/validate-osd-grammar.mjs index 815f171e620..4642c314f97 100644 --- a/scripts/ppl-lint/validate-osd-grammar.mjs +++ b/scripts/ppl-lint/validate-osd-grammar.mjs @@ -6,20 +6,27 @@ import fs from 'node:fs'; import path from 'node:path'; import { createRequire } from 'node:module'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; const HEADLESS = 'src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint'; const CATALOGS = [ 'packages/osd-monaco/ppl-lint', 'packages/osd-monaco/src/ppl/lint/catalog', ]; +const MODULE_EXTENSIONS = ['', '.js', '.cjs', '.mjs', '.ts']; +const INDEX_FILES = ['index.js', 'index.cjs', 'index.mjs', 'index.ts']; const SKIP_REASON = 'osd-headless-grammar-api-unavailable'; const OPTIONS = ['grammar', 'cases', 'target', 'osd-root', 'osd-sha', 'report', 'summary']; -class StructuralError extends Error {} +class StructuralError extends Error { + constructor(message, coverage) { + super(message); + this.coverage = coverage; + } +} -function fail(message) { - throw new StructuralError(message); +function fail(message, coverage) { + throw new StructuralError(message, coverage); } function parseArgs(argv) { @@ -124,9 +131,25 @@ function validatePairing(target) { } } -function modulePath(root, name) { +function moduleCandidates(root, name) { const base = path.join(root, name); - return [base, `${base}.js`, `${base}.ts`].find((candidate) => fs.existsSync(candidate)); + return [ + ...MODULE_EXTENSIONS.map((extension) => `${base}${extension}`), + ...INDEX_FILES.map((file) => path.join(base, file)), + ]; +} + +function resolveModule(requireFromOsd, osdRoot, name) { + for (const candidate of moduleCandidates(osdRoot, name)) { + try { + return requireFromOsd.resolve(candidate); + } catch (error) { + if (error?.code !== 'MODULE_NOT_FOUND') { + fail(`Could not resolve OSD module ${name}: ${error.message}`); + } + } + } + return undefined; } function exportsOf(module) { @@ -135,14 +158,30 @@ function exportsOf(module) { : module; } -function loadApi(osdRoot) { - const requireFromOsd = createRequire(path.join(osdRoot, 'noop.js')); - let headless; +async function importModule(requireFromOsd, resolved, name) { try { - headless = exportsOf(requireFromOsd(path.join(osdRoot, HEADLESS))); + if (path.extname(resolved) === '.mjs') { + return exportsOf(await import(pathToFileURL(resolved).href)); + } + return exportsOf(requireFromOsd(resolved)); } catch (error) { - fail(`OSD advertises ${HEADLESS}, but import failed: ${error.message}`); + if (error?.code === 'ERR_REQUIRE_ESM') { + try { + return exportsOf(await import(pathToFileURL(resolved).href)); + } catch (importError) { + fail(`Could not import OSD module ${name} from ${resolved}: ${importError.message}`); + } + } + fail(`Could not import OSD module ${name} from ${resolved}: ${error.message}`); } +} + +async function loadApi(osdRoot) { + const requireFromOsd = createRequire(path.join(osdRoot, 'noop.js')); + const resolvedHeadless = resolveModule(requireFromOsd, osdRoot, HEADLESS); + if (!resolvedHeadless) return undefined; + + const headless = await importModule(requireFromOsd, resolvedHeadless, HEADLESS); if ( typeof headless?.deserializeBundleOrThrow !== 'function' || typeof headless?.lintQueryWithBundle !== 'function' @@ -151,21 +190,23 @@ function loadApi(osdRoot) { } let catalogModule; - const errors = []; + const missingCatalogs = []; for (const name of CATALOGS) { - if (!modulePath(osdRoot, name)) continue; - try { - const candidate = exportsOf(requireFromOsd(path.join(osdRoot, name))); - if (typeof candidate?.getBundledCatalog === 'function') { - catalogModule = candidate; - break; - } - errors.push(`${name} has no getBundledCatalog export`); - } catch (error) { - errors.push(`${name}: ${error.message}`); + const resolved = resolveModule(requireFromOsd, osdRoot, name); + if (!resolved) { + missingCatalogs.push(name); + continue; + } + const candidate = await importModule(requireFromOsd, resolved, name); + if (typeof candidate?.getBundledCatalog !== 'function') { + fail(`${name} must export getBundledCatalog.`); } + catalogModule = candidate; + break; + } + if (!catalogModule) { + fail(`Could not resolve an OSD catalog module: ${missingCatalogs.join(', ')}.`); } - if (!catalogModule) fail(`Could not load OSD catalog${errors.length ? `: ${errors.join('; ')}` : '.'}`); let catalog; try { @@ -182,8 +223,7 @@ function loadApi(osdRoot) { return { deserialize: headless.deserializeBundleOrThrow, lint: headless.lintQueryWithBundle, - buildTree: typeof headless.buildRuntimeTree === 'function' ? headless.buildRuntimeTree : undefined, - catalogIds, + catalogIds: [...catalogIds].sort(), }; } @@ -205,19 +245,70 @@ function loadGrammar(file, deserialize) { return { bundle, grammar }; } +function sorted(values) { + return [...new Set(values)].sort(); +} + +function makeCoverage(catalogIds, coveredRuleIds, excludedRules) { + const catalogRuleIds = sorted(catalogIds); + const covered = sorted(coveredRuleIds); + const excluded = [...excludedRules].sort((left, right) => left.ruleId.localeCompare(right.ruleId)); + const catalog = new Set(catalogRuleIds); + const excludedRuleIds = sorted(excluded.map((entry) => entry.ruleId)); + const excludedSet = new Set(excludedRuleIds); + const requiredRuleIds = catalogRuleIds.filter((ruleId) => !excludedSet.has(ruleId)); + const required = new Set(requiredRuleIds); + const missingRuleIds = requiredRuleIds.filter((ruleId) => !covered.includes(ruleId)); + const unexpectedRuleIds = sorted([ + ...covered.filter((ruleId) => !required.has(ruleId)), + ...excludedRuleIds.filter((ruleId) => !catalog.has(ruleId)), + ]); + return { + catalogRuleIds, + requiredRuleIds, + coveredRuleIds: covered, + excludedRuleIds, + excludedRules: excluded, + missingRuleIds, + unexpectedRuleIds, + counts: { + catalog: catalogRuleIds.length, + required: requiredRuleIds.length, + covered: covered.length, + excluded: excludedRuleIds.length, + missing: missingRuleIds.length, + unexpected: unexpectedRuleIds.length, + }, + }; +} + function loadCases(file, catalogIds) { - const document = readJson(file, 'grammar cases'); - const rawCases = Array.isArray(document) ? document : object(document, 'case document').cases; - if (!Array.isArray(rawCases) || !rawCases.length) fail('At least one grammar case is required.'); - const knownRules = new Set(catalogIds); - const ids = new Set(); - const cases = rawCases.map((rawCase, index) => { + const document = object(readJson(file, 'grammar cases'), 'case document'); + if (document.schemaVersion !== 2) fail('case document schemaVersion must be 2.'); + if (!Array.isArray(document.excludedRules)) fail('case document excludedRules must be an array.'); + if (!Array.isArray(document.cases) || !document.cases.length) { + fail('At least one grammar case is required.'); + } + + const exclusionIds = new Set(); + const duplicateExclusionIds = []; + const excludedRules = document.excludedRules.map((rawExclusion, index) => { + const exclusion = object(rawExclusion, `excludedRules[${index}]`); + const ruleId = string(exclusion.ruleId, `excludedRules[${index}].ruleId`); + const reason = string(exclusion.reason, `excludedRules[${index}].reason`); + if (exclusionIds.has(ruleId)) duplicateExclusionIds.push(ruleId); + exclusionIds.add(ruleId); + return { ruleId, reason }; + }); + + const caseIds = new Set(); + const duplicateCaseIds = []; + const cases = document.cases.map((rawCase, index) => { const candidate = object(rawCase, `case[${index}]`); const id = string(candidate.id, `case[${index}].id`); const ruleId = string(candidate.ruleId, `case ${id}.ruleId`); - if (ids.has(id)) fail(`Duplicate case ID ${JSON.stringify(id)}.`); - if (!knownRules.has(ruleId)) fail(`Case ${JSON.stringify(id)} names missing OSD rule ${JSON.stringify(ruleId)}.`); - ids.add(id); + if (caseIds.has(id)) duplicateCaseIds.push(id); + caseIds.add(id); if (!['trigger', 'control'].includes(candidate.kind)) fail(`Case ${id} has invalid kind.`); if (!Number.isInteger(candidate.expectedCount) || candidate.expectedCount < 0) { fail(`Case ${id} expectedCount must be a non-negative integer.`); @@ -242,13 +333,45 @@ function loadCases(file, catalogIds) { }; }); - for (const ruleId of new Set(cases.map((entry) => entry.ruleId))) { + const coverage = makeCoverage(catalogIds, cases.map((entry) => entry.ruleId), excludedRules); + const catalog = new Set(coverage.catalogRuleIds); + const covered = new Set(coverage.coveredRuleIds); + if (duplicateExclusionIds.length) { + fail( + `Duplicate exclusion rule ID(s): ${sorted(duplicateExclusionIds).join(', ')}.`, + coverage + ); + } + if (duplicateCaseIds.length) { + fail(`Duplicate case ID(s): ${sorted(duplicateCaseIds).join(', ')}.`, coverage); + } + const staleExclusions = coverage.excludedRuleIds.filter((ruleId) => !catalog.has(ruleId)); + if (staleExclusions.length) { + fail(`Excluded rule(s) are absent from the OSD catalog: ${staleExclusions.join(', ')}.`, coverage); + } + const unknownCases = coverage.coveredRuleIds.filter((ruleId) => !catalog.has(ruleId)); + if (unknownCases.length) { + fail(`Case rule(s) are absent from the OSD catalog: ${unknownCases.join(', ')}.`, coverage); + } + const overlap = coverage.excludedRuleIds.filter((ruleId) => covered.has(ruleId)); + if (overlap.length) { + fail(`Rule(s) cannot be both covered and excluded: ${overlap.join(', ')}.`, coverage); + } + if (coverage.missingRuleIds.length || coverage.unexpectedRuleIds.length) { + fail( + `Case coverage must equal catalog rules minus exclusions; ` + + `missing: ${coverage.missingRuleIds.join(', ') || 'none'}; ` + + `unexpected: ${coverage.unexpectedRuleIds.join(', ') || 'none'}.`, + coverage + ); + } + for (const ruleId of coverage.requiredRuleIds) { const kinds = new Set(cases.filter((entry) => entry.ruleId === ruleId).map((entry) => entry.kind)); if (!kinds.has('trigger') || !kinds.has('control')) { - fail(`Selected rule ${JSON.stringify(ruleId)} must have trigger and control cases.`); + fail(`Required rule ${JSON.stringify(ruleId)} must have trigger and control cases.`, coverage); } } - return cases; + return { cases, coverage }; } function contextFor(grammarCase, target, catalogIds) { @@ -269,26 +392,6 @@ function contextFor(grammarCase, target, catalogIds) { }; } -function hasParseTree(result) { - if (!result) return false; - return typeof result === 'object' && 'tree' in result ? Boolean(result.tree) : true; -} - -function parseTreeOf(result) { - return typeof result === 'object' && result && 'tree' in result ? result.tree : result; -} - -function hasErrorNode(tree) { - const pending = [tree]; - while (pending.length) { - const node = pending.pop(); - if (!node || typeof node !== 'object') continue; - if (node.constructor?.name === 'ErrorNode') return true; - if (Array.isArray(node.children)) pending.push(...node.children); - } - return false; -} - function executeCases(api, grammar, cases, target) { return cases.map((grammarCase) => { const result = { @@ -299,30 +402,26 @@ function executeCases(api, grammar, cases, target) { expectedCount: grammarCase.expectedCount, }; try { - if (api.buildTree) { - const parse = api.buildTree(grammarCase.query, grammar); - if (!hasParseTree(parse)) throw new Error('candidate parser produced no parse tree'); - if (hasErrorNode(parseTreeOf(parse))) { - throw new Error('candidate parser recovered from a syntax error'); - } - } const lint = api.lint(grammarCase.query, grammar, contextFor(grammarCase, target, api.catalogIds)); if (!Array.isArray(lint?.diagnostics)) throw new Error('lintQueryWithBundle returned no diagnostics array'); result.actualCount = lint.diagnostics.filter((entry) => entry?.ruleId === grammarCase.ruleId).length; - result.parseTreeCheck = api.buildTree - ? 'verified' - : grammarCase.kind === 'trigger' && result.actualCount > 0 - ? 'inferred-from-target-diagnostic' - : 'unavailable'; - const countMatches = result.actualCount === result.expectedCount; - const treeVerified = result.parseTreeCheck !== 'unavailable'; - result.status = countMatches && treeVerified ? 'passed' : 'failed'; - if (!treeVerified) { - result.error = 'buildRuntimeTree is not exported; control parse tree cannot be verified'; + const unexpectedDiagnosticRuleIds = sorted( + lint.diagnostics + .filter((entry) => entry?.ruleId !== grammarCase.ruleId) + .map((entry) => entry?.ruleId || '') + ); + if (unexpectedDiagnosticRuleIds.length) { + result.unexpectedDiagnosticRuleIds = unexpectedDiagnosticRuleIds; + result.error = + `lintQueryWithBundle returned diagnostics for non-target rule(s): ` + + unexpectedDiagnosticRuleIds.join(', '); } + result.status = + result.actualCount === result.expectedCount && unexpectedDiagnosticRuleIds.length === 0 + ? 'passed' + : 'failed'; } catch (error) { result.actualCount = null; - result.parseTreeCheck = api.buildTree ? 'failed' : 'unavailable'; result.status = 'failed'; result.error = error instanceof Error ? error.message : String(error); } @@ -334,43 +433,61 @@ function executeCases(api, grammar, cases, target) { }); } -function counts(results) { +function caseCounts(results) { const passed = results.filter((entry) => entry.status === 'passed').length; return { selected: results.length, passed, failed: results.length - passed }; } -function makeReport(target, grammarHash, cases) { - const ruleResults = [...new Set(cases.map((entry) => entry.ruleId))].map((ruleId) => ({ - status: cases.filter((entry) => entry.ruleId === ruleId).every((entry) => entry.status === 'passed') - ? 'passed' - : 'failed', - })); +function ruleCounts(coverage, cases = []) { + const passed = coverage.requiredRuleIds.filter((ruleId) => + cases.length > 0 && + cases.filter((entry) => entry.ruleId === ruleId).every((entry) => entry.status === 'passed') + ).length; + return { + catalog: coverage.counts.catalog, + required: coverage.counts.required, + excluded: coverage.counts.excluded, + selected: coverage.counts.covered, + passed, + failed: cases.length > 0 ? coverage.counts.required - passed : 0, + }; +} + +function makeReport(target, grammarHash, coverage, cases) { const failures = cases.filter((entry) => entry.status === 'failed').map((entry) => ({ ruleId: entry.ruleId, caseId: entry.caseId, query: entry.query, expectedCount: entry.expectedCount, actualCount: entry.actualCount, + ...(entry.unexpectedDiagnosticRuleIds + ? { unexpectedDiagnosticRuleIds: entry.unexpectedDiagnosticRuleIds } + : {}), ...(entry.error ? { error: entry.error } : {}), })); return { - schemaVersion: 1, + schemaVersion: 2, status: failures.length ? 'failed' : 'passed', sql: target.sql, osd: target.osd, manualOverride: target.manualOverride, releaseLineValidationBypassed: target.releaseLineValidationBypassed, grammarHash, - rules: counts(ruleResults), - caseCounts: counts(cases), + coverage, + rules: ruleCounts(coverage, cases), + caseCounts: caseCounts(cases), cases, failures, }; } -function emptyReport(status, target, extra = {}) { +function emptyCoverage() { + return makeCoverage([], [], []); +} + +function emptyReport(status, target, extra = {}, coverage = emptyCoverage()) { return { - schemaVersion: 1, + schemaVersion: 2, status, ...extra, ...(target @@ -381,7 +498,8 @@ function emptyReport(status, target, extra = {}) { releaseLineValidationBypassed: target.releaseLineValidationBypassed, } : {}), - rules: { selected: 0, passed: 0, failed: 0 }, + coverage, + rules: ruleCounts(coverage), caseCounts: { selected: 0, passed: 0, failed: 0 }, cases: [], failures: [], @@ -400,7 +518,16 @@ function summary(report) { lines.push(`- Release-line validation bypassed: \`${report.releaseLineValidationBypassed}\``); } if (report.grammarHash) lines.push(`- Grammar: \`${report.grammarHash}\``); - lines.push(`- Rules: ${report.rules.selected} selected, ${report.rules.passed} passed, ${report.rules.failed} failed`); + lines.push( + `- Rules: ${report.rules.required} required, ${report.rules.selected} covered, ` + + `${report.rules.excluded} excluded, ${report.rules.passed} passed, ${report.rules.failed} failed` + ); + if (report.coverage.missingRuleIds.length) { + lines.push(`- Missing rules: ${report.coverage.missingRuleIds.map((id) => `\`${id}\``).join(', ')}`); + } + if (report.coverage.unexpectedRuleIds.length) { + lines.push(`- Unexpected rules: ${report.coverage.unexpectedRuleIds.map((id) => `\`${id}\``).join(', ')}`); + } if (report.error) lines.push('', `Structural error: ${report.error.replaceAll('\n', ' ')}`); if (report.failures.length) { lines.push('', '| Rule | Case | Expected | Actual | Error |', '| --- | --- | ---: | ---: | --- |'); @@ -421,9 +548,10 @@ function writeOutputs(args, report) { fs.appendFileSync(args.summary, summary(report)); } -export function run(argv = process.argv.slice(2)) { +export async function run(argv = process.argv.slice(2)) { let args; let target; + let coverage; try { args = parseArgs(argv); if (args.help) { @@ -434,22 +562,29 @@ export function run(argv = process.argv.slice(2)) { validatePairing(target); const osdRoot = path.resolve(args['osd-root']); if (!fs.existsSync(osdRoot) || !fs.statSync(osdRoot).isDirectory()) fail(`Invalid OSD root ${osdRoot}.`); - if (!modulePath(osdRoot, HEADLESS)) { + const api = await loadApi(osdRoot); + if (!api) { writeOutputs(args, emptyReport('skipped', target, { skipReason: SKIP_REASON })); return 0; } - const api = loadApi(osdRoot); const { bundle, grammar } = loadGrammar(args.grammar, api.deserialize); - const cases = loadCases(args.cases, api.catalogIds); - const report = makeReport(target, bundle.grammarHash, executeCases(api, grammar, cases, target)); + const loadedCases = loadCases(args.cases, api.catalogIds); + coverage = loadedCases.coverage; + const report = makeReport( + target, + bundle.grammarHash, + coverage, + executeCases(api, grammar, loadedCases.cases, target) + ); writeOutputs(args, report); return report.status === 'passed' ? 0 : 1; } catch (error) { + coverage = error?.coverage || coverage; const message = error instanceof Error ? error.message : String(error); console.error(`[ppl-lint-grammar] ERROR: ${message}`); if (args?.report && args?.summary) { try { - writeOutputs(args, emptyReport('error', target, { error: message })); + writeOutputs(args, emptyReport('error', target, { error: message }, coverage)); } catch (writeError) { console.error(`[ppl-lint-grammar] ERROR: could not write artifacts: ${writeError.message}`); } @@ -459,5 +594,5 @@ export function run(argv = process.argv.slice(2)) { } if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { - process.exitCode = run(); + process.exitCode = await run(); } From a7e3b2f8ad125d1d430e7daec5b2a5efab497c9f Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Thu, 6 Aug 2026 14:44:32 -0700 Subject: [PATCH 3/5] fix(ci): restore active PPL lint rule inventory Signed-off-by: Hanyu Wei --- docs/dev/ppl-lint-grammar-compatibility-ci.md | 10 +- scripts/ppl-lint/README.md | 10 +- .../__tests__/validate-osd-grammar.test.mjs | 23 +++- scripts/ppl-lint/grammar-cases.json | 102 ++++-------------- 4 files changed, 50 insertions(+), 95 deletions(-) diff --git a/docs/dev/ppl-lint-grammar-compatibility-ci.md b/docs/dev/ppl-lint-grammar-compatibility-ci.md index 42ef404dfc7..1f687548ddb 100644 --- a/docs/dev/ppl-lint-grammar-compatibility-ci.md +++ b/docs/dev/ppl-lint-grammar-compatibility-ci.md @@ -76,9 +76,13 @@ coveredRuleIds == catalogRuleIds - excludedRuleIds Every covered rule needs trigger and control cases. Every exclusion needs a non-empty reason. Missing rules, unknown cases, stale exclusions, overlaps, and -duplicate IDs fail structurally. The two explain-backed rules are excluded -because a grammar-only run has no backend explain plan; the other 16 catalog -rules are covered. +duplicate IDs fail structurally. The blocking set is the 12 catalog rules +enabled by default in the approved OSD release inventory. Four default-off +headless rules are explicitly excluded from the active gate, and the two +default-off explain-backed rules are excluded because a grammar-only run has no +backend explain plan. The separately configured `command-suggestion` check is a +syntax-channel feature, not a catalog detector, so it is outside this headless +lint adapter. ## Events diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md index 93e73550fb3..7bc69e3fbdd 100644 --- a/scripts/ppl-lint/README.md +++ b/scripts/ppl-lint/README.md @@ -143,15 +143,15 @@ Schema version 2 records catalog classification as well as behavior: "missingRuleIds": [], "unexpectedRuleIds": [], "counts": { - "catalog": 18, "required": 16, "covered": 16, - "excluded": 2, "missing": 0, "unexpected": 0 + "catalog": 18, "required": 12, "covered": 12, + "excluded": 6, "missing": 0, "unexpected": 0 } }, "rules": { - "catalog": 18, "required": 16, "excluded": 2, - "selected": 16, "passed": 16, "failed": 0 + "catalog": 18, "required": 12, "excluded": 6, + "selected": 12, "passed": 12, "failed": 0 }, - "caseCounts": {"selected": 32, "passed": 32, "failed": 0}, + "caseCounts": {"selected": 24, "passed": 24, "failed": 0}, "failures": [] } ``` diff --git a/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs b/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs index fc22e25a9d4..b5cbd7b0e9d 100644 --- a/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs +++ b/scripts/ppl-lint/__tests__/validate-osd-grammar.test.mjs @@ -570,15 +570,32 @@ test('case documents must use schema version two', (t) => { assert.match(result.stderr, /schemaVersion must be 2/); }); -test('the repository corpus covers 16 rules and excludes only two explain rules', () => { +test('the repository corpus covers the 12 default-enabled catalog rules', () => { const corpus = JSON.parse(fs.readFileSync(CORPUS, 'utf8')); const coveredRuleIds = [...new Set(corpus.cases.map((entry) => entry.ruleId))].sort(); const excludedRuleIds = corpus.excludedRules.map((entry) => entry.ruleId).sort(); assert.equal(corpus.schemaVersion, 2); - assert.equal(corpus.cases.length, 32); - assert.equal(coveredRuleIds.length, 16); + assert.equal(corpus.cases.length, 24); + assert.deepEqual(coveredRuleIds, [ + 'agg-on-text', + 'division-by-zero', + 'enabled-false-object', + 'field-validation', + 'invalid-capture-group-name', + 'multisearch-min-subsearch', + 'replace-wildcard-asymmetry', + 'rex-scan-cost', + 'type-mismatch-numeric', + 'union-min-datasets', + 'unsupported-window-function-in-eventstats', + 'wildcard-source-zero-match', + ]); assert.deepEqual(excludedRuleIds, [ + 'dedup-consecutive-unsupported', + 'disabled-join-type', + 'flat-object-subfield', + 'head-without-sort', 'operation-not-pushed', 'operation-pushed-as-script', ]); diff --git a/scripts/ppl-lint/grammar-cases.json b/scripts/ppl-lint/grammar-cases.json index d39504c6c09..97cf9043292 100644 --- a/scripts/ppl-lint/grammar-cases.json +++ b/scripts/ppl-lint/grammar-cases.json @@ -1,32 +1,32 @@ { "schemaVersion": 2, "excludedRules": [ + { + "ruleId": "head-without-sort", + "reason": "Disabled by default in the approved OSD release catalog; outside the active compatibility gate." + }, + { + "ruleId": "disabled-join-type", + "reason": "Disabled by default in the approved OSD release catalog; outside the active compatibility gate." + }, + { + "ruleId": "dedup-consecutive-unsupported", + "reason": "Disabled by default in the approved OSD release catalog; outside the active compatibility gate." + }, + { + "ruleId": "flat-object-subfield", + "reason": "Disabled by default in the approved OSD release catalog; outside the active compatibility gate." + }, { "ruleId": "operation-not-pushed", - "reason": "Requires backend explain data; backend execution is outside this grammar/linter check." + "reason": "Disabled by default and requires backend explain data; backend execution is outside this grammar/linter check." }, { "ruleId": "operation-pushed-as-script", - "reason": "Requires backend explain data; backend execution is outside this grammar/linter check." + "reason": "Disabled by default and requires backend explain data; backend execution is outside this grammar/linter check." } ], "cases": [ - { - "id": "head-without-sort", - "ruleId": "head-without-sort", - "kind": "trigger", - "query": "source=accounts | head 5", - "expectedCount": 1, - "context": { "isCalcite": true } - }, - { - "id": "head-with-sort-control", - "ruleId": "head-without-sort", - "kind": "control", - "query": "source=accounts | sort age | head 5", - "expectedCount": 0, - "context": { "isCalcite": true } - }, { "id": "division-by-zero", "ruleId": "division-by-zero", @@ -75,44 +75,6 @@ "expectedCount": 0, "context": { "isCalcite": true } }, - { - "id": "right-join-disabled", - "ruleId": "disabled-join-type", - "kind": "trigger", - "query": "source=accounts | join type=right left=l right=r on l.account_number=r.account_number accounts", - "expectedCount": 1, - "context": { - "isCalcite": true, - "settings": { "allJoinTypesAllowed": false } - } - }, - { - "id": "inner-join-control", - "ruleId": "disabled-join-type", - "kind": "control", - "query": "source=accounts | join left=l right=r on l.account_number=r.account_number accounts", - "expectedCount": 0, - "context": { - "isCalcite": true, - "settings": { "allJoinTypesAllowed": false } - } - }, - { - "id": "dedup-consecutive-true", - "ruleId": "dedup-consecutive-unsupported", - "kind": "trigger", - "query": "source=accounts | dedup firstname consecutive=true", - "expectedCount": 1, - "context": { "isCalcite": true } - }, - { - "id": "dedup-plain-control", - "ruleId": "dedup-consecutive-unsupported", - "kind": "control", - "query": "source=accounts | dedup firstname", - "expectedCount": 0, - "context": { "isCalcite": true } - }, { "id": "union-single-dataset", "ruleId": "union-min-datasets", @@ -211,34 +173,6 @@ } } }, - { - "id": "flat-object-subfield-reference", - "ruleId": "flat-object-subfield", - "kind": "trigger", - "query": "source=accounts | fields attributes.child", - "expectedCount": 1, - "context": { - "isCalcite": true, - "typeMap": { - "attributes": "flat_object", - "name": "text" - } - } - }, - { - "id": "non-flat-field-reference-control", - "ruleId": "flat-object-subfield", - "kind": "control", - "query": "source=accounts | fields name", - "expectedCount": 0, - "context": { - "isCalcite": true, - "typeMap": { - "attributes": "flat_object", - "name": "text" - } - } - }, { "id": "numeric-field-text-comparison", "ruleId": "type-mismatch-numeric", From 1db750da4fee349df3e2d7fd9cbf5fd9c0b987c2 Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Thu, 6 Aug 2026 16:06:55 -0700 Subject: [PATCH 4/5] chore: retrigger CI Signed-off-by: Hanyu Wei From b360c801e29e25dab6e6283babbb1f2c0327141e Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Thu, 6 Aug 2026 16:18:33 -0700 Subject: [PATCH 5/5] chore: retrigger CI Signed-off-by: Hanyu Wei