chore: update tests to use Column::new and other expr_fn functions
#3265
Conversation
alamb
requested review from
wjones127,
roeap,
rtyler,
hntd187 and
ion-elgreco
as code owners
| ); | ||
| let filter = col(Column::new(Some(target_name.clone()), "id")) | ||
| .between(lit("B"), lit("C")) | ||
| .and(lit("2023-07-04").eq(col(Column::new(Some(target_name.clone()), "modified")))); |
There was a problem hiding this comment.
While I was working on this, I also updated uses of patterns like Expr::Literal(ScalarValue::Utf8(Some("B".to_string()))), to lit("B") as it does the same thing and is more concise
alamb
force-pushed
the
alamb/prepare_upgrade_46
branch
from
f3bf6ca to
49227fb
Compare
| @@ -453,14 +453,7 @@ mod tests { | |||
| .build() | |||
| .unwrap(); | |||
|
|
|||
| let join_predicate = col(Column { | |||
There was a problem hiding this comment.
the key rationale for this change is that Column has a new field in DataFUsion 46, so if it is constructed explicitly like this I need to change all the callsites
If I switch to using Column::new() then nothing needs to be changed as part of the upgrade (as Column::new handles initializing the newly added field)
Signed-off-by: Andrew Lamb <[email protected]>
ion-elgreco
force-pushed
the
alamb/prepare_upgrade_46
branch
from
49227fb to
bd0ec41
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3265 +/- ##
==========================================
- Coverage 72.14% 72.05% -0.09%
==========================================
Files 143 143
Lines 45724 45606 -118
Branches 45724 45606 -118
==========================================
- Hits 32986 32860 -126
+ Misses 10666 10665 -1
- Partials 2072 2081 +9 ☔ View full report in Codecov by Sentry. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Description
Note this PR should have no functional change
This is part of the DataFusion upgrade in #3262, which I am trying to split into multiple parts to reduce the size of the diff (so to make it easier to review and debug)
DataFusion 46.0.0 adds a new field,
Column::spanswhich requires changing all locations that createColumneither to have aspanin it or to use the constructor. So let's change the code to use the constructor hereRelated Issue(s)
46.0.0apache/datafusion#14123Documentation