Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Filter predicates should not be aliased." seems too strict #11306

Closed
samuelcolvin opened this issue Jul 6, 2024 · 0 comments · Fixed by #11307
Closed

"Filter predicates should not be aliased." seems too strict #11306

samuelcolvin opened this issue Jul 6, 2024 · 0 comments · Fixed by #11307
Labels
bug Something isn't working

Comments

@samuelcolvin
Copy link
Contributor

Describe the bug

Aliases are not allowed as filter predicates.

As per datafusion-contrib/datafusion-functions-json#26 (comment) @alamb suggested we work around change of alias when substituting functions to replace operators by using Expr::Alias.

However when substituting foo ? 'bar' with a function wrapped in an alias, we get an error

Plan("Attempted to create Filter predicate with expression `json_contains(test.json_data, Utf8(\"foo\"))` aliased as 'json_data ? Utf8(\"foo\")'. Filter predicates should not be aliased.")

This is caused by

// filter predicates should not be aliased
if let Expr::Alias(Alias { expr, name, .. }) = predicate {
return plan_err!(
"Attempted to create Filter predicate with \
expression `{expr}` aliased as '{name}'. Filter predicates should not be \
aliased."
);
}

Ref #3796 & #3795

This seems unnecessarily strict.

The problem is that I when replacing the operator, you don't know what part of the query the operator is used in, so you can't decide whether or not to apply the alias AFAIK.

To Reproduce

Add the following test to datafusion-contrib/datafusion-functions-json#26


#[tokio::test]
async fn test_question_filter() {
    let batches = run_query("select name from test where json_data ? 'foo'")
        .await
        .unwrap();

    let expected = [
        "+------------+",
        "| name       |",
        "+------------+",
        "| object_foo |",
        "+------------+",
    ];
    assert_batches_eq!(expected, &batches);
}

Expected behavior

I think aliases as predicates should be allowed.

Additional context

No response

@samuelcolvin samuelcolvin added the bug Something isn't working label Jul 6, 2024
@samuelcolvin samuelcolvin changed the title "Filter predicates should not be aliased." seems to strict "Filter predicates should not be aliased." seems too strict Jul 6, 2024
samuelcolvin added a commit to samuelcolvin/datafusion that referenced this issue Jul 6, 2024
@alamb alamb closed this as completed in 9991144 Jul 8, 2024
findepi pushed a commit to findepi/datafusion that referenced this issue Jul 16, 2024
…11307)

* allow alias in predicate, fix apache#11306

* Fix typo.

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

* unalise predicate

* use unalias_nested

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
xinlifoobar pushed a commit to xinlifoobar/datafusion that referenced this issue Jul 18, 2024
…11307)

* allow alias in predicate, fix apache#11306

* Fix typo.

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

* unalise predicate

* use unalias_nested

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant