remove access methods of Filter and make field pub. #4464
Conversation
github-actions
bot
added
core
jackwener
changed the title
jackwener
changed the title
jackwener
changed the title
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
seems reasonable to me. Thanks @jackwener
| pub struct Filter { | ||
| /// The predicate expression, which must have Boolean type. | ||
| predicate: Expr, | ||
| pub predicate: Expr, |
There was a problem hiding this comment.
I think this would allow side-stepping the schema validation in try_new? I don't know how important this is
There was a problem hiding this comment.
Yes, but I think it isn't a problem.
Projection Aggregate Subquery .... also is like this.
There was a problem hiding this comment.
Maybe we can at least document what invariants are required between fields
There was a problem hiding this comment.
I think this would allow side-stepping the schema validation in try_new? I don't know how important this is
Yes, the motivation for the current design (introduced in #3796) was to prevent invalid filters from being created.
|
What problem is this PR solving? It is partially reverting changes from #3796 that were introduced to prevent some class of errors when constructing plans. |
|
Got it, Thanks your explanation @andygrove |
4 participants
Which issue does this PR close?
Filter has access method which function like
get()inobject-oriented.Rust don't need like
get()method for struct. Because default immutable.Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?