From 18e5204513dc340ce42d732f1db7a35fa040178c Mon Sep 17 00:00:00 2001 From: John Kerl Date: Sun, 1 Feb 2026 12:36:38 -0500 Subject: [PATCH] Require that filter expressions be boolean (or absent) (#1935) * Insist that filter expressions be boolean * docs --- docs/src/reference-dsl.md | 10 ++++-- docs/src/reference-dsl.md.in | 10 ++++-- pkg/runtime/state.go | 4 ++- pkg/transformers/put_or_filter.go | 37 ++++++++++++++++++-- test/cases/dsl-multipart-scripts/0005/cmd | 2 +- test/cases/dsl-multipart-scripts/0005/expout | 2 ++ 6 files changed, 57 insertions(+), 8 deletions(-) diff --git a/docs/src/reference-dsl.md b/docs/src/reference-dsl.md index 46651921a..b51ddef4a 100644 --- a/docs/src/reference-dsl.md +++ b/docs/src/reference-dsl.md @@ -178,9 +178,15 @@ a=wye,b=pan,i=5,x=0.573288,y=0.863624,ab=wye_pan The two verbs `mlr filter` and `mlr put` are essentially the same. The only differences are: -* Expressions sent to `mlr filter` should contain a boolean expression, which is the filtering criterion. (If not, all records pass through.) - * `mlr filter` expressions may not reference the `filter` keyword within them. +* Before Miller 6.17: + * Expressions sent to `mlr filter` should contain a boolean expression, which is the filtering criterion. (If not, all records pass through.) +* As of Miller 6.17: + * Expressions sent to `mlr filter` must contain a boolean expression, which is the filtering criterion. + * If the expression evaluates to `false`, the record does not pass through. + * If the expression evaluates to `true` or `absent`, the record passes through. + * If the expression evaluates to anything other than boolean or absent, that is a fatal error. + * The reason for accepting `absent` is for Miller's [record-heterogeneity guarantees](record-heterogeneity.md). It's not an error to filter for `$x > 10` if the current record has no `$x` field. ## Location of boolean expression for filter diff --git a/docs/src/reference-dsl.md.in b/docs/src/reference-dsl.md.in index eb2ae470a..fde1e7029 100644 --- a/docs/src/reference-dsl.md.in +++ b/docs/src/reference-dsl.md.in @@ -120,9 +120,15 @@ GENMD-EOF The two verbs `mlr filter` and `mlr put` are essentially the same. The only differences are: -* Expressions sent to `mlr filter` should contain a boolean expression, which is the filtering criterion. (If not, all records pass through.) - * `mlr filter` expressions may not reference the `filter` keyword within them. +* Before Miller 6.17: + * Expressions sent to `mlr filter` should contain a boolean expression, which is the filtering criterion. (If not, all records pass through.) +* As of Miller 6.17: + * Expressions sent to `mlr filter` must contain a boolean expression, which is the filtering criterion. + * If the expression evaluates to `false`, the record does not pass through. + * If the expression evaluates to `true` or `absent`, the record passes through. + * If the expression evaluates to anything other than boolean or absent, that is a fatal error. + * The reason for accepting `absent` is for Miller's [record-heterogeneity guarantees](record-heterogeneity.md). It's not an error to filter for `$x > 10` if the current record has no `$x` field. ## Location of boolean expression for filter diff --git a/pkg/runtime/state.go b/pkg/runtime/state.go index 3fe93aa18..389a0e9dc 100644 --- a/pkg/runtime/state.go +++ b/pkg/runtime/state.go @@ -54,7 +54,9 @@ func NewEmptyState(options *cli.TOptions, strictMode bool) *State { Inrec: nil, Context: nil, Oosvars: oosvars, - FilterExpression: mlrval.TRUE, + // XXX + //FilterExpression: mlrval.TRUE, + FilterExpression: mlrval.NULL, Stack: NewStack(), regexCapturesByFrame: regexCapturesByFrame, diff --git a/pkg/transformers/put_or_filter.go b/pkg/transformers/put_or_filter.go index 648595ce2..0b2f05ead 100644 --- a/pkg/transformers/put_or_filter.go +++ b/pkg/transformers/put_or_filter.go @@ -364,7 +364,10 @@ func transformerPutOrFilterParseCLI( dslInstanceType = cst.DSLInstanceTypeFilter } + doFilter := (verb == "filter") + transformer, err := NewTransformerPut( + doFilter, dslStrings, dslInstanceType, presets, @@ -390,6 +393,7 @@ func transformerPutOrFilterParseCLI( // ---------------------------------------------------------------- type TransformerPut struct { + doFilter bool // false for the put verb, true for the filter verb cstRootNode *cst.RootNode runtimeState *runtime.State callCount int @@ -399,6 +403,7 @@ type TransformerPut struct { } func NewTransformerPut( + doFilter bool, // false for the put verb, true for the filter verb dslStrings []string, dslInstanceType cst.DSLInstanceType, presets []string, @@ -483,6 +488,7 @@ func NewTransformerPut( } return &TransformerPut{ + doFilter: doFilter, cstRootNode: cstRootNode, runtimeState: runtimeState, callCount: 0, @@ -527,10 +533,37 @@ func (tr *TransformerPut) Transform( } if !tr.suppressOutputRecord { + // The tr.runtimeState.FilterExpression defaults to null. It evaluates to null + // for assignment statements, etc. + // * If the verb is put, then tr.runtimeState.FilterExpression will get set to + // something only when a filter DSL statement is encountered. + // * If the verb is filter, then we insist that the expression evaluate to either + // boolean, or absent. The latter is for Miller's record-heterogeneity guarantees, + // e.g. mlr filter '$x > 10' for records not having a $x. + filterBool, isBool := tr.runtimeState.FilterExpression.GetBoolValue() - if !isBool { - filterBool = false + + if tr.doFilter { + // This is mlr filter + if !isBool { + if tr.runtimeState.FilterExpression.IsAbsent() { + filterBool = false + } else { + fmt.Fprintf(os.Stderr, + "Filter expression did not evaluate to boolean: got %s value %s", + tr.runtimeState.FilterExpression.String(), + tr.runtimeState.FilterExpression.GetTypeName(), + ) + os.Exit(1) + } + } + } else { + // This is mlr put. + if !isBool { + filterBool = true + } } + wantToEmit := lib.BooleanXOR(filterBool, tr.invertFilter) if wantToEmit { outputRecordsAndContexts.PushBack(types.NewRecordAndContext(outrec, &context)) diff --git a/test/cases/dsl-multipart-scripts/0005/cmd b/test/cases/dsl-multipart-scripts/0005/cmd index 8db8d1d62..b9f0a58f9 100644 --- a/test/cases/dsl-multipart-scripts/0005/cmd +++ b/test/cases/dsl-multipart-scripts/0005/cmd @@ -1 +1 @@ -mlr --opprint --from test/input/abixy filter -e -f ${CASEDIR}/mlr +mlr --opprint --from test/input/abixy filter -f ${CASEDIR}/mlr diff --git a/test/cases/dsl-multipart-scripts/0005/expout b/test/cases/dsl-multipart-scripts/0005/expout index e69de29bb..11ad96e44 100644 --- a/test/cases/dsl-multipart-scripts/0005/expout +++ b/test/cases/dsl-multipart-scripts/0005/expout @@ -0,0 +1,2 @@ +a b i x y +eks zee 7 0.61178406 0.18788492