SERVER-108947 Expand PBT model to test empty paths - second reapply (#52033)

GitOrigin-RevId: 0e0764353236ad2b4b984f8471608b2d1cdae3f7
This commit is contained in:
Hagar Mohamed 2026-05-21 16:49:30 +02:00 committed by MongoDB Bot
parent 333ce46dac
commit f6c66c1a94
14 changed files with 121 additions and 55 deletions

View File

@ -17,9 +17,9 @@
import {fc} from "jstests/third_party/fast_check/fc-3.1.0.js";
import {
assignableFieldArb,
dottedDollarFieldArb,
dottedFieldArb,
getAssignableFieldArb,
nonEmptyDottedDollarFieldArb,
nonEmptyDottedFieldArb,
} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {getDatasetModel} from "jstests/libs/property_test_helpers/models/document_models.js";
import {isSlowBuild} from "jstests/libs/query/aggregation_pipeline_utils.js";
@ -52,21 +52,29 @@ const leafArb = fc.oneof(
);
// {$project: {[dest]: "$src"}} using dotted paths on both sides.
const projectRenameArb = getComputedProjectArb(dottedFieldArb, dottedDollarFieldArb);
const projectRenameArb = getComputedProjectArb(nonEmptyDottedFieldArb, nonEmptyDottedDollarFieldArb);
// {$addFields: {[dest]: "$src"}} using dotted paths on both sides.
const addFieldsRenameArb = getAddFieldsVarArb(dottedFieldArb, dottedDollarFieldArb);
const addFieldsRenameArb = getAddFieldsVarArb(nonEmptyDottedFieldArb, nonEmptyDottedDollarFieldArb);
// {$set: {[field]: <array>}}.
const setFieldArb = fc.oneof({arbitrary: assignableFieldArb, weight: 3}, {arbitrary: dottedFieldArb, weight: 1});
// Use non-empty field arbitraries because MongoDB rejects empty strings in FieldPath expressions.
const setFieldArb = fc.oneof(
{arbitrary: getAssignableFieldArb(false /*allowEmpty*/), weight: 3},
{arbitrary: nonEmptyDottedFieldArb, weight: 1},
);
const literalArraySetArb = fc
.tuple(
setFieldArb,
fc.array(
fc.dictionary(assignableFieldArb, fc.oneof(scalarArb, fc.array(scalarArb, {minLength: 1, maxLength: 2})), {
minKeys: 1,
maxKeys: 3,
}),
fc.dictionary(
getAssignableFieldArb(false /*allowEmpty*/),
fc.oneof(scalarArb, fc.array(scalarArb, {minLength: 1, maxLength: 2})),
{
minKeys: 1,
maxKeys: 3,
},
),
{minLength: 1, maxLength: 3},
),
)
@ -74,7 +82,9 @@ const literalArraySetArb = fc
// The reshaping done by a complex rename affects $eq, so it's redundant to model a larger subset
// of MatchExpressions.
const dottedMatchArb = fc.record({field: dottedFieldArb}).map(({field}) => ({$match: {[field]: {$eq: "equal"}}}));
const dottedMatchArb = fc
.record({field: nonEmptyDottedFieldArb})
.map(({field}) => ({$match: {[field]: {$eq: "equal"}}}));
// The top-level field is never an array, but subfields might be.
const nestedObjArb = fc.record({a: leafArb, b: leafArb, t: leafArb, m: leafArb});
const docModel = fc.record({
@ -87,7 +97,7 @@ const docModel = fc.record({
// Only single-field indexes to avoid CannotIndexParallelArrays errors.
const dottedIndexDefArb = fc
.record({field: dottedFieldArb.filter((f) => f !== "_id"), dir: fc.constantFrom(1, -1)})
.record({field: nonEmptyDottedFieldArb.filter((f) => f !== "_id"), dir: fc.constantFrom(1, -1)})
.map(({field, dir}) => ({[field]: dir}));
const indexModel = fc.record({def: dottedIndexDefArb, options: fc.constant({})});

View File

@ -18,7 +18,11 @@
import {fc} from "jstests/third_party/fast_check/fc-3.1.0.js";
import {getMatchArb} from "jstests/libs/property_test_helpers/models/match_models.js";
import {dottedDollarFieldArb, dottedFieldArb, intArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {
nonEmptyDottedDollarFieldArb,
nonEmptyDottedFieldArb,
intArb,
} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {makeWorkloadModel} from "jstests/libs/property_test_helpers/models/workload_models.js";
import {runWithParamsAllNonConfigNodes} from "jstests/noPassthrough/libs/server_parameter_helpers.js";
import {isSlowBuild} from "jstests/libs/query/aggregation_pipeline_utils.js";
@ -44,7 +48,7 @@ lookupColl.insertOne({});
// We use $multiply for the expression. $convert ensures we get a numeric input.
// Prime numbers should make it less likely that the result is correct due to chance in case of an incorrect rewrite.
const safeFieldArb = dottedDollarFieldArb.map((f) => ({
const safeFieldArb = nonEmptyDottedDollarFieldArb.map((f) => ({
$convert: {input: f, to: "double", onError: 11, onNull: 13},
}));
const exprArb = fc.oneof(
@ -62,7 +66,7 @@ const exprArb = fc.oneof(
const lookupStage = {$lookup: {from: lookupColl.getName(), pipeline: [], as: "a"}};
// Generate 1-2 dotted fields with distinct base fields.
const distinctBaseFieldsArb = fc.uniqueArray(dottedFieldArb, {
const distinctBaseFieldsArb = fc.uniqueArray(nonEmptyDottedFieldArb, {
minLength: 1,
maxLength: 2,
selector: (f) => f.split(".")[0],

View File

@ -19,6 +19,9 @@ import {testProperty} from "jstests/libs/property_test_helpers/property_testing_
import {isSlowBuild} from "jstests/libs/query/aggregation_pipeline_utils.js";
import {matchFirstStageAggModel} from "jstests/libs/property_test_helpers/common_models.js";
// TODO SERVER-126813: Re-enable this test.
quit();
if (isSlowBuild(db)) {
jsTest.log.info("Returning early because debug is on, opt is off, or a sanitizer is enabled.");
quit();

View File

@ -59,17 +59,39 @@ export function getScalarArb({allowUnicode, allowNullBytes} = {}) {
return oneof(intArb, fc.boolean(), getStringArb({allowUnicode, allowNullBytes}), dateArb, fc.constant(null));
}
export const fieldArb = fc.constantFrom("a", "b", "t", "m", "_id", "m.m1", "m.m2", "array");
export const dollarFieldArb = fieldArb.map((f) => "$" + f);
export const assignableFieldArb = fc.constantFrom("a", "b", "t", "m");
// FieldPath (used by aggregation operators, indexes, etc.) does not support empty strings.
// Use getFieldArb(false) wherever the field is used as a FieldPath expression.
export function getFieldArb(allowEmpty) {
const arb = fc.constantFrom("a", "b", "t", "m", "_id", "m.m1", "m.m2", "array", "");
return allowEmpty ? arb : arb.filter((f) => f !== "");
}
export const fieldArb = getFieldArb(true);
export const nonEmptyFieldArb = getFieldArb(false);
export const dollarFieldArb = nonEmptyFieldArb.map((f) => "$" + f);
export function getAssignableFieldArb(allowEmpty) {
const arb = fc.constantFrom("a", "b", "t", "m", "");
return allowEmpty ? arb : arb.filter((f) => f !== "");
}
// Dotted field paths up to depth 2 built from assignable base fields.
export const dottedFieldArb = fc.oneof(
fieldArb,
fc.tuple(assignableFieldArb, assignableFieldArb).map(([a, b]) => `${a}.${b}`),
fc
.tuple(getAssignableFieldArb(true /*allowEmpty*/), getAssignableFieldArb(true /*allowEmpty*/))
.map(([a, b]) => `${a}.${b}`),
);
export const dottedDollarFieldArb = dottedFieldArb.map((f) => "$" + f);
// Dotted field paths without empty string components. Use these wherever the field is used as a
// FieldPath expression and empty strings are not allowed.
export const nonEmptyDottedFieldArb = fc.oneof(
nonEmptyFieldArb,
fc
.tuple(getAssignableFieldArb(false /*allowEmpty*/), getAssignableFieldArb(false /*allowEmpty*/))
.map(([a, b]) => `${a}.${b}`),
);
export const nonEmptyDottedDollarFieldArb = nonEmptyDottedFieldArb.map((f) => "$" + f);
export const leafParametersPerFamily = 10;
export class LeafParameter {
constructor(concreteValues) {

View File

@ -15,5 +15,13 @@ export function getCollectionModel({isTS = false, indexesModel, docsModel} = {})
if (!indexesModel) {
indexesModel = getIndexesModel({isTS: isTSCollection});
}
return fc.record({isTS: fc.constant(isTSCollection), docs: docsModel, indexes: indexesModel});
// TODO SERVER-121084: Reevaluate whether metaField='' should be allowed.
const metaFieldArb = isTSCollection ? fc.constant("m") : fc.constant(undefined);
return fc.record({
isTS: fc.constant(isTSCollection),
metaField: metaFieldArb,
docs: docsModel,
indexes: indexesModel,
});
}

View File

@ -32,6 +32,7 @@ export function getDocModel({allowUnicode, allowNullBytes} = {}) {
array: oneof(scalarArb, arrayFieldModel),
a: scalarArb,
b: scalarArb,
"": scalarArb,
});
}

View File

@ -3,27 +3,29 @@
*
* Note that $avg cannot be supported because it can cause floating point differences in results.
*/
import {assignableFieldArb, dollarFieldArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {getAssignableFieldArb, dollarFieldArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {oneof} from "jstests/libs/property_test_helpers/models/model_utils.js";
import {fc} from "jstests/third_party/fast_check/fc-3.1.0.js";
// These all model accumulated fields, which is the output field and the accumulator. An example
// is `a: {count: {}}` which can by used in a $group.
const countAccArb = assignableFieldArb.map((out) => {
const countAccArb = getAssignableFieldArb(false /*allowEmpty*/).map((out) => {
return {[out]: {$count: {}}};
});
// $sum. Example is `a: {$sum: '$b'}`.
const sumAccArb = fc.record({input: dollarFieldArb, output: assignableFieldArb}).map(({input, output}) => {
return {[output]: {$sum: input}};
});
const sumAccArb = fc
.record({input: dollarFieldArb, output: getAssignableFieldArb(false /*allowEmpty*/)})
.map(({input, output}) => {
return {[output]: {$sum: input}};
});
// Examples are `a: {$min: '$b'}` and `a: {$min: {input: b, n: 2}}`.
const minMaxAccArb = fc
.record({
acc: fc.constantFrom("$min", "$max"),
input: dollarFieldArb,
output: assignableFieldArb,
output: getAssignableFieldArb(false /*allowEmpty*/),
n: fc.option(fc.integer({min: 1, max: 3})),
})
.map(({acc, input, output, n}) => {
@ -44,7 +46,10 @@ const minMaxAccArb = fc
const accumulatedFieldArb = oneof(countAccArb, sumAccArb, minMaxAccArb);
// A groupby key could be a single field `$a` or an object, `{a: '$b', c: '$d'}`
const objectGbKeyArb = fc.dictionary(assignableFieldArb, dollarFieldArb, {minKeys: 1, maxKeys: 3});
const objectGbKeyArb = fc.dictionary(getAssignableFieldArb(false /*allowEmpty*/), dollarFieldArb, {
minKeys: 1,
maxKeys: 3,
});
const groupByKeyArb = oneof(
dollarFieldArb,
// TODO SERVER-102229, re-enable object group keys once issue is fixed.

View File

@ -2,7 +2,7 @@
* Fast-check models for indexes.
* See property_test_helpers/README.md for more detail on the design.
*/
import {fieldArb, getScalarArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {nonEmptyFieldArb, getScalarArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {collationArb} from "jstests/libs/property_test_helpers/models/collation_models.js";
import {getPartialFilterPredicateArb} from "jstests/libs/property_test_helpers/models/match_models.js";
import {oneof} from "jstests/libs/property_test_helpers/models/model_utils.js";
@ -48,7 +48,7 @@ function replaceKeyValAtPosition(obj, ix, {newKey, newVal}) {
// Regular indexes
// Tuple of indexed field, and it's sort direction.
const singleIndexDefArb = fc.record({field: fieldArb, dir: fc.constantFrom(1, -1)});
const singleIndexDefArb = fc.record({field: nonEmptyFieldArb, dir: fc.constantFrom(1, -1)});
// Unique array of [[a, true], [b, false], ...] to be mapped to an index definition. Unique on the
// indexed field. Filter out any indexes that only use the _id field.
const arrayOfSingleIndexDefsArb = fc
@ -138,7 +138,7 @@ const wildcardProjectionIncludeExcludeArb = fc.oneof(
const wildcardProjectionArb = fc
.record({
fields: fc.uniqueArray(fieldArb, {minLength: 1, maxLength: 8}),
fields: fc.uniqueArray(nonEmptyFieldArb, {minLength: 1, maxLength: 8}),
// use the same value throughout the projection
value: wildcardProjectionIncludeExcludeArb,
})
@ -176,7 +176,7 @@ const fullWildcardDefArb = fc
const dottedWildcardDefArb = fc
.record({
indexDef: simpleIndexDefArb,
fieldPrefix: fieldArb,
fieldPrefix: nonEmptyFieldArb,
wcIx: fc.integer({min: 0, max: 4}),
})
.map(({indexDef, fieldPrefix, wcIx}) => {

View File

@ -2,15 +2,15 @@
* $lookup models for our core property tests.
*/
import {assignableFieldArb, fieldArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {getAssignableFieldArb, nonEmptyFieldArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {fc} from "jstests/third_party/fast_check/fc-3.1.0.js";
export function getEqLookupArb(from) {
return fc
.record({
localField: fieldArb,
foreignField: fieldArb,
as: assignableFieldArb,
localField: nonEmptyFieldArb,
foreignField: nonEmptyFieldArb,
as: getAssignableFieldArb(false /*allowEmpty*/),
})
.map(({localField, foreignField, as}) => {
return {$lookup: {from, localField, foreignField, as}};
@ -21,9 +21,9 @@ export function getEqLookupUnwindArb(fromArb) {
return fc
.record({
from: fromArb,
localField: fieldArb,
foreignField: fieldArb,
as: assignableFieldArb,
localField: nonEmptyFieldArb,
foreignField: nonEmptyFieldArb,
as: getAssignableFieldArb(false /*allowEmpty*/),
preserveNullAndEmptyArrays: fc.boolean(),
})
.map(({from, localField, foreignField, as, preserveNullAndEmptyArrays}) => {

View File

@ -1,7 +1,7 @@
/*
* Fast-check models for $match.
*/
import {fieldArb, leafParameterArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {getFieldArb, leafParameterArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {oneof, singleKeyObjArb} from "jstests/libs/property_test_helpers/models/model_utils.js";
import {fc} from "jstests/third_party/fast_check/fc-3.1.0.js";
@ -67,6 +67,9 @@ export function getMatchPredicateSpec({
allowOrs = true,
allowNors = true,
allowNot = true,
// $match predicates allow empty string field names, but FieldPath expressions (used in
// partial filter expressions, indexes, etc.) do not. Set this to true only for plain $match.
allowEmptyField = false,
// Leaf comparison types, like $eq, $ne, $gt, etc.
allowedSimpleComparisons = simpleComparators,
allowIn = true,
@ -120,7 +123,7 @@ export function getMatchPredicateSpec({
fc.array(tie("predicate"), {minLength: 1, maxLength: 3}),
),
// Example: {a: {$eq: 5}}
singleSimplePredicate: singleKeyObjArb(fieldArb, tie("condition")),
singleSimplePredicate: singleKeyObjArb(getFieldArb(allowEmptyField), tie("condition")),
// A single predicate is a simple predicate, or a compound predicate.
singlePredicate: fc.oneof(
{withCrossShrink: true, maxDepth},

View File

@ -1,13 +1,17 @@
/*
* Fast-check models for $project.
*/
import {assignableFieldArb, dollarFieldArb, fieldArb} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {
getAssignableFieldArb,
dollarFieldArb,
nonEmptyFieldArb,
} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {oneof} from "jstests/libs/property_test_helpers/models/model_utils.js";
import {fc} from "jstests/third_party/fast_check/fc-3.1.0.js";
// Inclusion/Exclusion projections. {$project: {_id: 1, a: 0}}
export function getSingleFieldProjectArb(isInclusion, {simpleFieldsOnly = false} = {}) {
const projectedFieldArb = simpleFieldsOnly ? assignableFieldArb : fieldArb;
const projectedFieldArb = simpleFieldsOnly ? getAssignableFieldArb(false /*allowEmpty*/) : nonEmptyFieldArb;
return fc.record({field: projectedFieldArb, includeId: fc.boolean()}).map(function ({field, includeId}) {
const includeIdVal = includeId ? 1 : 0;
const includeFieldVal = isInclusion ? 1 : 0;
@ -22,7 +26,7 @@ export const simpleProjectArb = oneof(
export function getMultipleFieldProjectArb(isInclusion = true, {simpleFieldsOnly = false} = {}) {
// Choosing only from assignable fields to avoid projecting both m and m.1.
const fieldVal = isInclusion ? 1 : 0;
const projectedFieldArb = simpleFieldsOnly ? assignableFieldArb : fieldArb;
const projectedFieldArb = simpleFieldsOnly ? getAssignableFieldArb(false /*allowEmpty*/) : nonEmptyFieldArb;
// We cannot have both a field and its subfield in the same $project.
function hasPathCollision(fields) {
return fields.some((f) => fields.some((g) => g !== f && g.startsWith(f + ".")));
@ -51,4 +55,4 @@ export function getComputedProjectArb(destFieldArb, srcFieldArb) {
return {$project: {[destField]: srcField}};
});
}
export const computedProjectArb = getComputedProjectArb(fieldArb, dollarFieldArb);
export const computedProjectArb = getComputedProjectArb(nonEmptyFieldArb, dollarFieldArb);

View File

@ -8,9 +8,9 @@
* See property_test_helpers/README.md for more detail on the design.
*/
import {
assignableFieldArb,
getAssignableFieldArb,
dollarFieldArb,
fieldArb,
nonEmptyFieldArb,
leafParameterArb,
} from "jstests/libs/property_test_helpers/models/basic_models.js";
import {collationArb} from "jstests/libs/property_test_helpers/models/collation_models.js";
@ -26,7 +26,7 @@ import {
} from "jstests/libs/property_test_helpers/models/project_models.js";
// Add field with a constant argument. {$addFields: {a: 5}}
export const addFieldsConstArb = fc.tuple(fieldArb, leafParameterArb).map(function ([destField, leafParams]) {
export const addFieldsConstArb = fc.tuple(nonEmptyFieldArb, leafParameterArb).map(function ([destField, leafParams]) {
return {$addFields: {[destField]: leafParams}};
});
// Add field from source field, parameterized on the dest and src field arbs.
@ -36,9 +36,12 @@ export function getAddFieldsVarArb(destFieldArb, srcFieldArb) {
return {$addFields: {[destField]: sourceField}};
});
}
export const addFieldsVarArb = getAddFieldsVarArb(fieldArb, dollarFieldArb);
export const addFieldsVarArb = getAddFieldsVarArb(nonEmptyFieldArb, dollarFieldArb);
// $replaceRoot projection
export const replaceRootArb = fc.tuple(assignableFieldArb, dollarFieldArb).map(function ([destField, sourceField]) {
export const replaceRootArb = fc.tuple(getAssignableFieldArb(false /*allowEmpty*/), dollarFieldArb).map(function ([
destField,
sourceField,
]) {
// We use $$ROOT to keep the overall schema in order to better cooperate with other stages.
return {$replaceRoot: {newRoot: {$mergeObjects: ["$$ROOT", {[destField]: sourceField}]}}};
});
@ -56,7 +59,7 @@ export const replaceRootArb = fc.tuple(assignableFieldArb, dollarFieldArb).map(f
*/
export function getSortArb(maxNumSortComponents = 1) {
const sortDirectionArb = fc.constantFrom(1, -1);
const sortComponent = fc.record({field: fieldArb, dir: sortDirectionArb});
const sortComponent = fc.record({field: nonEmptyFieldArb, dir: sortDirectionArb});
return fc
.uniqueArray(sortComponent, {
minLength: 1,
@ -80,7 +83,7 @@ export const unwindArb = fc
path: dollarFieldArb,
preserveNullAndEmptyArrays: fc.boolean(),
includeArrayIndex: fc.boolean(),
indexFieldName: assignableFieldArb,
indexFieldName: getAssignableFieldArb(false /*allowEmpty*/),
})
.map(({path, preserveNullAndEmptyArrays, includeArrayIndex, indexFieldName}) => {
const unwindSpec = {path: path};

View File

@ -34,8 +34,8 @@ export function concreteQueryFromFamily(queryShape, leafId) {
return queryShape;
}
function createColl(db, coll, isTS = false) {
const args = isTS ? {timeseries: {timeField: "t", metaField: "m"}} : {};
function createColl(db, coll, isTS, metaField) {
const args = isTS ? {timeseries: {timeField: "t", metaField}} : {};
assert.commandWorked(db.createCollection(coll.getName(), args));
}
@ -102,9 +102,9 @@ function runProperty(propertyFn, namespaces, workload, sortArrays) {
let {collSpec, foreignCollSpec, queries, extraParams} = workload;
const {controlColl, experimentColl, foreignControlColl, foreignExperimentColl} = namespaces;
function setUpCollection({collection, docs, isTS = false, indexes = []}) {
function setUpCollection({collection, docs, isTS = false, metaField = "m", indexes = []}) {
assertDropCollection(collection.getDB(), collection.getName());
createColl(collection.getDB(), collection, isTS);
createColl(collection.getDB(), collection, isTS, metaField);
assert.commandWorked(collection.insert(docs));
createIndexesForPBT(collection, indexes);
}
@ -115,6 +115,7 @@ function runProperty(propertyFn, namespaces, workload, sortArrays) {
collection: experimentColl,
docs: collSpec.docs,
isTS: collSpec.isTS,
metaField: collSpec.metaField,
indexes: collSpec.indexes,
});
@ -131,6 +132,7 @@ function runProperty(propertyFn, namespaces, workload, sortArrays) {
collection: foreignExperimentColl,
docs: foreignCollSpec.docs,
isTS: foreignCollSpec.isTS,
metaField: foreignCollSpec.metaField,
indexes: foreignCollSpec.indexes,
});
}

View File

@ -26,6 +26,7 @@ const fullyMinimizedDoc = {
array: 0,
a: 0,
b: 0,
"": 0,
};
function testShrinking(isTS) {