Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 5 additions & 12 deletions .github/workflows/ppl-lint-multiversion-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,11 @@ name: PPL lint multi-version validation
# happens to be. A rule that is correct on main can be a false positive on 3.6 or
# a false negative on 3.7, and nothing notices.
#
# This workflow validates every DEFAULT-ERROR rule (enabled: true + severity:
# error in OSD's rules_catalog.json) against SEVERAL released engine versions
# plus the PR build, and — when a rule disagrees with any of them — says what to
# change in the linter rather than only that a count was wrong.
#
# Why default-error only: an error-severity rule is one the user cannot opt out
# of and which marks their query as broken. A wrong error is the most expensive
# possible lint defect, so that set gets the multi-version treatment first.
# Warning/info rules stay on the single-version check. The set is not hand-copied:
# the detector run records the catalog's default-error census, and the aggregate
# step fails if a rule in that census has no contract file (see manifest.json's
# `defaultError` note).
# This workflow validates every active shipping contract — 12 detector rules and
# the command-suggestion syntax feature where its runtime grammar surface is
# available — against released engine versions plus the PR build. The aggregate
# still records the exact default-error census separately, but --all-rules makes
# warning/info omissions and syntax regressions visible too.
#
# Shape — a per-version matrix of observation legs, then one aggregation:
#
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/ppl-lint-rule-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ jobs:
# green (design §4.4). A workflow_dispatch run is pre-merge evidence and is
# intentionally not what repo admins pin to branch protection.
- name: Require both validation jobs to have succeeded
if: ${{ always() }}
env:
BACKEND_RESULT: ${{ needs.backend-validation.result }}
DETECTOR_RESULT: ${{ needs.detector-validation.result }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import org.opensearch.sql.ppl.PPLIntegTestCase;

/**
* Backend half of the schema-v3/schema-v4 PPL lint rule validation contract.
* Backend half of the schema-v3/schema-v4 PPL frontend validation contract.
*
* <p>This test drives the live {@code POST /_plugins/_ppl} endpoint on the SQL plugin built from
* the current checkout. For every contract (see {@code
Expand All @@ -50,25 +50,24 @@
* confirmed by a single run, e.g. head nondeterminism, fallback warnings).
* </ul>
*
* <p>The contract files are shared verbatim with the SQL-owned OSD detector runner ({@code
* <p>The contract files are shared verbatim with the SQL-owned OSD frontend runner ({@code
* scripts/ppl-lint/run-frontend-contract.mjs}) so the same reviewed cases pin both the OSD analyzer
* diagnostic count and the SQL backend behavior; neither side can drift without a red build. The
* output and the SQL backend behavior; neither side can drift without a red build. The
* rejection-body parsing mirrors {@link
* org.opensearch.sql.calcite.remote.CalciteErrorReportStageIT}; the Calcite setup follows {@link
* org.opensearch.sql.calcite.remote.CalcitePPLEventstatsIT}.
*
* <p>While the ephemeral cluster is alive, the test also exports the candidate runtime grammar
* bundle it built ({@code GET /_plugins/_ppl/_grammar}) and a small target manifest pairing the
* bundle with the backend version and grammar hash. These become workflow artifacts that the
* detector-validation job injects into OSD's headless lint API, so both halves validate against the
* SAME candidate grammar (design §4.2, §4.3). Export runs only when {@code
* detector-validation job injects into OSD's headless lint and syntax APIs, so both halves validate
* against the SAME candidate grammar (design §4.2, §4.3). Export runs only when {@code
* -Dppl.lint.grammar.bundle} is set (CI); local runs without it are unaffected.
*
* <p>The suite honors {@code -Dppl.lint.schedule=pr|nightly} (default {@code pr}): a PR run skips
* contracts declaring {@code schedule: "nightly"}, while nightly runs the full corpus. Every
* contract in the corpus currently declares {@code schedule: "pr"}, so the two are equivalent
* today; the filter stays because it is the only mechanism for holding a new contract back from PR
* runs while its oracle is still settling.
* contracts declaring {@code schedule: "nightly"}, while nightly runs all 13 active contracts. The
* filter holds new detector and syntax contracts back from PR runs while their standard and
* analytics oracles are still settling.
*
* <p>Note that a contract which RUNS also ASSERTS. This class does not consult the manifest's
* {@code enforced} list — that list records oracle quality and review status, not blocking
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,11 @@ public enum Index {
"flat_object",
getFlatObjectIndexMapping(),
"src/test/resources/flat_object.json"),
PPL_LINT_DISABLED_OBJECT(
TestsConstants.TEST_INDEX_PPL_LINT_DISABLED_OBJECT,
"ppl_lint_disabled_object",
getPplLintDisabledObjectIndexMapping(),
"src/test/resources/ppl_lint_disabled_object.json"),
DUPLICATION_NULLABLE(
TestsConstants.TEST_INDEX_DUPLICATION_NULLABLE,
"duplication_nullable",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,11 @@ public static String getFlatObjectIndexMapping() {
return getMappingFile(mappingFile);
}

public static String getPplLintDisabledObjectIndexMapping() {
String mappingFile = "ppl_lint_disabled_object_index_mapping.json";
return getMappingFile(mappingFile);
}

public static String getPhraseIndexMapping() {
String mappingFile = "phrase_index_mapping.json";
return getMappingFile(mappingFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public class TestsConstants {
public static final String TEST_INDEX_ALIAS = TEST_INDEX + "_alias";
public static final String TEST_INDEX_FLATTENED_VALUE = TEST_INDEX + "_flattened_value";
public static final String TEST_INDEX_FLAT_OBJECT = TEST_INDEX + "_flat_object";
public static final String TEST_INDEX_PPL_LINT_DISABLED_OBJECT =
TEST_INDEX + "_ppl_lint_disabled_object";
public static final String TEST_INDEX_GEOIP = TEST_INDEX + "_geoip";
public static final String DATASOURCES = ".ql-datasources";
public static final String TEST_INDEX_STATE_COUNTRY = TEST_INDEX + "_state_country";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"mappings": {
"properties": {
"session": {
"type": "object",
"enabled": false
},
"status": {
"type": "keyword"
}
}
}
}
127 changes: 127 additions & 0 deletions integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
{
"schemaVersion": 4,
"ruleId": "agg-on-text",

Check failure on line 3 in integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json

View workflow job for this annotation

GitHub Actions / validation-result

PPL lint required validation: agg-on-text

[agg-on-text] normalized wiring mismatch: contract={"id":"agg-on-text","detector":"agg-on-text","enabled":true,"severity":"warning","appliesTo":{"minVersion":"3.7.0","engine":"calcite"},"runtimeOnly":false,"needsContext":true,"needsExplain":false,"sourceScoped":true} catalog={"id":"agg-on-text","detector":"agg-on-text","enabled":true,"severity":"warning","appliesTo":{},"runtimeOnly":false,"needsContext":true,"needsExplain":false,"sourceScoped":false}.
"channel": "lint",
"note": "The standard engine accepts both text aggregations: avg(text) returns null while sum(text) returns a non-empty numeric result (observed as 0.0). The warning catches this misleading coercion rather than predicting backend rejection.",
"grammarSurface": "both",
"schedule": "nightly",
"wiring": {
"detector": "agg-on-text",
"enabled": true,
"severity": "warning",
"runtimeOnly": false,
"needsContext": true,
"needsExplain": false,
"sourceScoped": true,
"appliesTo": {
"minVersion": "3.7.0",
"engine": "calcite"
}
},
"backendFixture": {
"indices": [
"ACCOUNT"
],
"clusterSettings": {
"calcite": true,
"calciteFallback": false
}
},
"frontendContext": {
"isCalcite": true,
"deriveFromMapping": {
"firstname": "text",
"balance": "long"
}
},
"index": "opensearch-sql_test_index_account",
"queries": {
"avg-text-field": {
"role": "trigger",
"query": "source={{index}} | stats avg(firstname) as avg_firstname"
},
"sum-text-field": {
"role": "trigger",
"query": "source={{index}} | stats sum(firstname) as sum_firstname"
},
"avg-numeric-control": {
"role": "control",
"query": "source={{index}} | stats avg(balance) as avg_balance"
}
},
"expectations": [
{
"version": ">=3.7.0",
"engine": "calcite",
"queries": {
"avg-text-field": {
"frontend": {
"count": 1,
"severity": "warning",
"matchMessage": "text field"
},
"backends": {
"standard": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"columnAllNull": "avg_firstname"
}
},
"analytics": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"columnAllNull": "avg_firstname"
}
}
}
},
"sum-text-field": {
"frontend": {
"count": 1,
"severity": "warning",
"matchMessage": "text field"
},
"backends": {
"standard": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"datarowsNonEmpty": true
}
},
"analytics": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"datarowsNonEmpty": true
}
}
}
},
"avg-numeric-control": {
"frontend": {
"count": 0
},
"backends": {
"standard": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"datarowsNonEmpty": true
}
},
"analytics": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"datarowsNonEmpty": true
}
}
}
}
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
{
"schemaVersion": 4,
"ruleId": "command-suggestion",
"channel": "syntax",
"grammarSurface": "runtime-bundle",
"schedule": "nightly",
"wiring": {
"code": "UNKNOWN_COMMAND"
},
"backendFixture": {
"indices": [
"ACCOUNT"
],
"clusterSettings": {
"calcite": true,
"calciteFallback": false
}
},
"frontendContext": {
"isCalcite": true
},
"index": "opensearch-sql_test_index_account",
"queries": {
"misspelled-command": {
"role": "trigger",
"query": "source={{index}} | wherre age > 1"
},
"valid-command-control": {
"role": "control",
"query": "source={{index}} | where age > 1"
},
"unrecognizable-command": {
"role": "suppression-control",
"query": "source={{index}} | zzzzzzzz"
},
"incomplete-expression": {
"role": "suppression-control",
"query": "source={{index}} | where age >"
}
},
"expectations": [
{
"version": ">=0.0.0",
"queries": {
"misspelled-command": {
"frontend": {
"count": 1,
"code": "UNKNOWN_COMMAND",
"fixText": "where",
"matchMessage": "where",
"rawMessage": true
},
"backends": {
"standard": {
"kind": "rejection",
"httpStatus": 400,
"body": {
"status": 400
}
},
"analytics": {
"kind": "rejection",
"httpStatus": 400,
"body": {
"status": 400
}
}
}
},
"valid-command-control": {
"frontend": {
"count": 0,
"code": "UNKNOWN_COMMAND",
"totalErrors": 0
},
"backends": {
"standard": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"datarowsNonEmpty": true
}
},
"analytics": {
"kind": "result-shape",
"httpStatus": 200,
"expect": {
"datarowsNonEmpty": true
}
}
}
},
"unrecognizable-command": {
"frontend": {
"count": 0,
"code": "UNKNOWN_COMMAND",
"totalErrors": 1
},
"backends": {
"standard": {
"kind": "rejection",
"httpStatus": 400,
"body": {
"status": 400
}
},
"analytics": {
"kind": "rejection",
"httpStatus": 400,
"body": {
"status": 400
}
}
}
},
"incomplete-expression": {
"frontend": {
"count": 0,
"code": "UNKNOWN_COMMAND",
"totalErrors": 1
},
"backends": {
"standard": {
"kind": "rejection",
"httpStatus": 400,
"body": {
"status": 400
}
},
"analytics": {
"kind": "rejection",
"httpStatus": 400,
"body": {
"status": 400
}
}
}
}
}
}
]
}
Loading
Loading