Remove `Expr::GetIndexedField`, replace `Expr::{field,index,range}` with `FieldAccessor`, `IndexAccessor`, and `SliceAccessor` by jayzhan211 · Pull Request #10568 · apache/datafusion

jayzhan211 · 2024-05-18T06:19:20Z

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

After the change, you need to import

use datafusion_functions::core::expr_ext::FieldAccessor;
use datafusion_functions_array::expr_ext::{IndexAccessor, SliceAccessor};

to use old expression api.

Are these changes tested?

Are there any user-facing changes?

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

jayzhan211 · 2024-05-18T06:19:54Z

datafusion/core/tests/optimizer_integration.rs

 }
+
+#[test]
+fn test_nested_schema_nullability() {


Move tests that use functions to core/tests

jayzhan211 · 2024-05-18T06:20:44Z

datafusion/optimizer/src/optimize_projections/mod.rs

    }

-    #[test]
-    fn test_struct_field_push_down() -> Result<()> {


Move to slt

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

alamb · 2024-05-20T16:06:24Z

datafusion/core/tests/expr_api/mod.rs

 fn test_get_field() {
    evaluate_expr_test(
-        get_field(col("props"), "a"),
+        col("props").field("a"),


alamb

This is great -- thank you @jayzhan211 . I had one small comment regarding the placement of docstrings but I also think we could do that as a follow on PR

alamb · 2024-05-20T16:11:54Z

datafusion/functions-array/src/expr_ext.rs

+// specific language governing permissions and limitations
+// under the License.
+
+//! Extension methods for Expr.


This is a very clever idea. Thank you @jayzhan211

alamb · 2024-05-20T16:12:34Z

datafusion/functions-array/src/expr_ext.rs

+}
+
+impl IndexAccessor for Expr {
+    /// Return access to the element field. Example `expr["name"]`


I think the way that rustdoc works, this documentation won't appear on index -- can you please move the docs and example to IndexAccessor itself?

alamb · 2024-05-20T16:13:10Z

datafusion/functions/src/core/expr_ext.rs

+}
+
+impl FieldAccessor for Expr {
+    /// Return access to the named field. Example `expr["name"]`


likewise here I think we need to move the docs / example to FieldAccessor rather than on the impl in Expr

alamb · 2024-05-20T16:13:51Z

datafusion/physical-expr/src/planner.rs

            input_dfschema,
            execution_props,
        )?),
-        Expr::GetIndexedField(GetIndexedField { expr: _, field }) => match field {


Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

jayzhan211 · 2024-05-21T00:07:18Z

Thanks, @alamb !

…ith `FieldAccessor`, `IndexAccessor`, and `SliceAccessor` (apache#10568) * remove expr Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add expr extension Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * doc Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * move test that has struct Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * fmt Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add foc and fix displayed name Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * rm test Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * rebase Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * move doc Signed-off-by: jayzhan211 <jayzhan211@gmail.com> --------- Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

datafusion replaced Expr::GetIndexField with a FieldAccessor trait. Ref apache/datafusion#10568 Ref apache/datafusion#10769

* deps: update datafusion to 39.0.0, pyo3 to 0.21, and object_store to 0.10.1 `datafusion-common` also depends on `pyo3`, so they need to be upgraded together. * feat: remove GetIndexField datafusion replaced Expr::GetIndexField with a FieldAccessor trait. Ref apache/datafusion#10568 Ref apache/datafusion#10769 * feat: update ScalarFunction The field `func_name` was changed to `func` as part of removing `ScalarFunctionDefinition` upstream. Ref apache/datafusion#10325 * feat: incorporate upstream array_slice fixes Fixes #670 * update ExectionPlan::children impl for DatasetExec Ref apache/datafusion#10543 * update value_interval_daytime Ref apache/arrow-rs#5769 * update regexp_replace and regexp_match Fixes #677 * add gil-refs feature to pyo3 This silences pyo3's deprecation warnings for its new Bounds api. It's the 1st step of the migration, and should be removed before merge. Ref https://pyo3.rs/v0.21.0/migration#from-020-to-021 * fix signature for octet_length Ref apache/datafusion#10726 * update signature for covar_samp AggregateUDF expressions now have a builder API design, which removes arguments like filter and order_by Ref apache/datafusion#10545 Ref apache/datafusion#10492 * convert covar_pop to expr_fn api Ref: https://github.com/apache/datafusion/pull/10418/files * convert median to expr_fn api Ref apache/datafusion#10644 * convert variance sample to UDF Ref apache/datafusion#10667 * convert first_value and last_value to UDFs Ref apache/datafusion#10648 * checkpointing with a few todos to fix remaining compile errors * impl PyExpr::python_value for IntervalDayTime and IntervalMonthDayNano * convert sum aggregate function to UDF * remove unnecessary clone on double reference * apply cargo fmt * remove duplicate allow-dead-code annotation * update tpch examples for new pyarrow interval Fixes #665 * marked q11 tpch example as expected fail Ref #730 * add default stride of None back to array_slice

…ith `FieldAccessor`, `IndexAccessor`, and `SliceAccessor` (apache#10568) * remove expr Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add expr extension Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * doc Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * move test that has struct Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * fmt Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add foc and fix displayed name Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * rm test Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * rebase Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * move doc Signed-off-by: jayzhan211 <jayzhan211@gmail.com> --------- Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

jayzhan211 added 5 commits May 18, 2024 10:56

remove expr

6c8abb4

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

add expr extension

3aff84b

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

doc

5bf1d82

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

move test that has struct

db6b48a

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

fmt

1a754b4

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

github-actions bot added sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels May 18, 2024

jayzhan211 commented May 18, 2024

View reviewed changes

jayzhan211 added 2 commits May 18, 2024 14:37

add foc and fix displayed name

dc3bfd7

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

rm test

136f3df

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

jayzhan211 marked this pull request as ready for review May 18, 2024 07:19

jayzhan211 added 2 commits May 19, 2024 14:01

Merge remote-tracking branch 'upstream/main' into rm-GetIndexedField

860b11b

rebase

26cb3d0

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

alamb added the api change Changes the API exposed to users of the crate label May 20, 2024

alamb reviewed May 20, 2024

View reviewed changes

alamb changed the title ~~Remove Expr::GetIndexedField and fix panic of field, index and range~~ Remove Expr::GetIndexedField, replace Expr::{field,index,range} with FieldAccessor, IndexAccessor, and SliceAccessor May 20, 2024

alamb approved these changes May 20, 2024

View reviewed changes

alamb mentioned this pull request May 20, 2024

Minor: Fix name in ArrayFunctionRewriter, error not panic if Expr::GetStructField is planned #10564

Closed

move doc

f70e1e6

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

jayzhan211 merged commit 3ebc31d into apache:main May 21, 2024

appletreeisyellow mentioned this pull request May 21, 2024

WIP(iox-10891): patched df upgrade 202-05-13 influxdata/arrow-datafusion#20

Closed

lewiszlw mentioned this pull request Jun 3, 2024

Cleanup GetIndexedField #10769

Merged

abhiaagarwal mentioned this pull request Jun 9, 2024

chore: bump to datafusion 39, arrow 52, pyo3 0.21 delta-io/delta-rs#2581

Merged

Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this pull request Jun 13, 2024

feat: remove GetIndexField

b98378e

datafusion replaced Expr::GetIndexField with a FieldAccessor trait. Ref apache/datafusion#10568 Ref apache/datafusion#10769

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `Expr::GetIndexedField`, replace `Expr::{field,index,range}` with `FieldAccessor`, `IndexAccessor`, and `SliceAccessor`#10568

Remove `Expr::GetIndexedField`, replace `Expr::{field,index,range}` with `FieldAccessor`, `IndexAccessor`, and `SliceAccessor`#10568
jayzhan211 merged 10 commits intoapache:mainfrom
jayzhan211:rm-GetIndexedField

jayzhan211 commented May 18, 2024 •

edited

Loading

Uh oh!

jayzhan211 May 18, 2024

Uh oh!

jayzhan211 May 18, 2024

Uh oh!

alamb May 20, 2024

Uh oh!

alamb left a comment

Uh oh!

alamb May 20, 2024

Uh oh!

alamb May 20, 2024

Uh oh!

alamb May 20, 2024

Uh oh!

alamb May 20, 2024

Uh oh!

jayzhan211 commented May 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jayzhan211 commented May 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

jayzhan211 May 18, 2024

Choose a reason for hiding this comment

Uh oh!

jayzhan211 May 18, 2024

Choose a reason for hiding this comment

Uh oh!

alamb May 20, 2024

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb May 20, 2024

Choose a reason for hiding this comment

Uh oh!

alamb May 20, 2024

Choose a reason for hiding this comment

Uh oh!

alamb May 20, 2024

Choose a reason for hiding this comment

Uh oh!

alamb May 20, 2024

Choose a reason for hiding this comment

Uh oh!

jayzhan211 commented May 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jayzhan211 commented May 18, 2024 •

edited

Loading