[draft] Add `LogicalType`, try to support user-defined types by yukkit · Pull Request #8143 · apache/datafusion

yukkit · 2023-11-12T08:53:22Z

Which issue does this PR close?

Closes #7923 .

Current Pull Request is an Experimental Demo for Validating the Feasibility of Logical Types

Rationale for this change

What changes are included in this PR?

Features

Create User-Defined Types (UDTs) through SQL, specifying the field types as UDTs during table creation.
Support the use of UDT as a function signature in udf/udaf.
Register extension types through the register_data_type function in the SessionContext.

New Additions

LogicalType struct.
ExtensionType trait. Abstraction for extension types.
TypeSignature struct. Uniquely identifies a data type.

Major Changes

Added get_data_type(&self, _name: &TypeSignature) -> Option<LogicalType> function to the ContextProvider trait.
In DFSchema, DFField now uses LogicalType, removing arrow Field and retaining only data_type, nullable, metadata since dict_id, dict_is_ordered are not necessary at the logical stage.
ExprSchemable and ExprSchema now use LogicalType.
ast to logical plan conversion now uses LogicalType.

To Be Implemented

TypeCoercionRewriter in the analyze stage uses logical types. For example, functions like comparison_coercion, get_input_types, get_valid_types, etc.
Functions signatures for udf/udaf use TypeSignature instead of the existing DataType for ease of use in udf/udaf.

To Be Determined

Should ScalarValue use LogicalType or arrow DataType?
- LogicalType.
- DataType
Should TableSource return DFSchema or arrow Schema?
- Schema.
- DFSchema
Conversion between physical types and logical types (in Datafusion, type conversion is achieved through the conversion of DFSchema to Schema; logical plans use DFSchema, physical plans use Schema).
Conversion between Schema and DFSchema
- When to convert Schema to DFSchema?
  - During the construction of the logical TableScan node, obtain arrow Schema through TableSource/TableProvider and then convert it to DFSchema.
  - TableSource/TableProvider returns DFSchema instead of Schema.
- When to convert DFSchema to Schema?
  - Directly obtain arrow Schema from TableSource in physical planner, no need for conversion.
  - Convert the DFSchema returned by TableSource to Schema in the physical planner stage.

Some Thoughts

In this comment, the use case of converting from dyn Array to LineStringArray or MultiPointArray was raised. In my perspective, assuming there is a function specifically designed for handling LineString data, the function signature can be defined as LineString, ensuring that the input data must be of a type acceptable by LineStringArray.

Are these changes tested?

Are there any user-facing changes?

yukkit · 2023-11-12T08:57:23Z

Current PR has some unresolved issues requiring collaboration for discussion.
Once there is a consensus on all the issues among the team, I will reorganize the PR accordingly.

yukkit · 2023-11-13T12:37:07Z

alamb · 2023-11-13T18:39:00Z

Thanks @yukkit -- I plan to give this a look, but probably will not have time until tomorrow

yukkit · 2023-11-14T03:52:39Z

datafusion/core/src/execution/context/mod.rs

        }
    }

+    async fn create_type(&self, cmd: CreateType) -> Result<DataFrame> {


This is the core processing logic for creating SQL UDT.

yukkit · 2023-11-14T03:53:43Z

datafusion/core/src/execution/context/mod.rs

+        self.register_data_type(
+            type_signature.to_owned_type_signature(),
+            extension_type,
+        )?;


Register the extension types created through SQL UDT.

yukkit · 2023-11-14T03:54:52Z

datafusion/core/src/execution/context/mod.rs

    }
 }

+impl TypeManager for SessionState {


SessionState implements TymeManager for registering and retrieving extended data types.

yukkit · 2023-11-14T04:02:49Z

datafusion/expr/src/expr_schema.rs

 pub trait ExprSchemable {
    /// given a schema, return the type of the expr
-    fn get_type<S: ExprSchema>(&self, schema: &S) -> Result<DataType>;
+    fn get_type<S: ExprSchema>(&self, schema: &S) -> Result<LogicalType>;


Replace all arrow DataType used in the logical planning stage with LogicalType(literal ScalarValue TBD).

yukkit · 2023-11-14T04:04:51Z

datafusion/expr/src/expr_schema.rs

+                // TODO not convert to DataType
+                let data_types = data_types
+                    .into_iter()
+                    .map(|e| e.physical_type())
+                    .collect::<Vec<_>>();
+
+                Ok((fun.return_type)(&data_types)?.as_ref().clone().into())


TODO: Functions signatures for udf/udaf use TypeSignature instead of the arrow DataType

yukkit · 2023-11-14T04:09:41Z

datafusion/expr/src/logical_plan/builder.rs

-                .ok_or_else(|| {
-                    plan_datafusion_err!(
+        // FIXME support logical data types, not convert to DataType
+        let data_type = comparison_coercion(


Modify the function comparison_coercion to accept LogicalType as parameters.

yukkit · 2023-11-14T04:49:37Z

datafusion/expr/src/utils.rs

 /// data types here come from function 'equal_rows', if more data types are supported
 /// in equal_rows(hash join), add those data types here to generate join logical plan.
-pub fn can_hash(data_type: &DataType) -> bool {
+pub fn can_hash(data_type: &LogicalType) -> bool {


This function can_hash might be abstracted as a property of logical data types.

yukkit · 2023-11-14T04:50:46Z

datafusion/optimizer/src/analyzer/type_coercion.rs

+                // FIXME like_coercion use LogicalType
+                let left_type = expr.get_type(&self.schema)?.physical_type();
+                let right_type = pattern.get_type(&self.schema)?.physical_type();
                let coerced_type = like_coercion(&left_type,  &right_type).ok_or_else(|| {


TODO: Modify the function like_coercion to accept LogicalType as parameters.

yukkit · 2023-11-14T04:53:37Z

datafusion/sql/src/planner.rs

    }

-    pub(crate) fn convert_data_type(&self, sql_type: &SQLDataType) -> Result<DataType> {
+    pub(crate) fn convert_data_type(


Convert data types in the AST to logical data types.

yukkit · 2023-11-14T04:54:10Z

datafusion/sql/src/planner.rs

+            SQLDataType::Custom(name, params) => {
+                let name = object_name_to_string(name);
+                let params = params.iter().map(Into::into).collect();
+                let type_signature = TypeSignature::new_with_params(name, params);
+                if let Some(data_type) = self.context_provider.get_data_type(&type_signature) {
+                    return Ok(data_type);
+                }
+
+                plan_err!("User-Defined SQL type {sql_type:?} not found")
+            }


Convert custom data types.

alamb

This is an epic PR @yukkit -- I looked through it and I really liked the structure and the overall code. Really nice 👏

I think the next steps for this PR are to circulate it more widely -- perhaps via a note to the arrow mailing list, and discord / slack channels (I can help with this) and potentially have people gauge how much disruption this will cause with downstream crates (I can test with IOx perhaps, and see what that experience is like)

Again, thanks for pushing this forward

alamb · 2023-11-14T21:55:03Z

datafusion/common/src/dfschema.rs

-            _ => dt1 == dt2,
-        }
+    fn datatype_is_logically_equal(dt1: &LogicalType, dt2: &LogicalType) -> bool {
+        dt1 == dt2


that is certainly nicer

alamb · 2023-11-14T21:55:59Z

datafusion/common/src/dfschema.rs

+    data_type: LogicalType,
+    nullable: bool,
+    /// A map of key-value pairs containing additional custom meta data.
+    metadata: HashMap<String, String>,


Maybe we could store in a BTreeMap to avoid sorting them each time 🤔

alamb · 2023-11-14T21:57:13Z

datafusion/common/src/logical_type.rs

+use arrow_schema::{DataType, Field, IntervalUnit, TimeUnit};
+
+#[derive(Clone, Debug)]
+pub enum LogicalType {


Eventually we should add doc comments here, but it also makes sense to avoid over doing it on RFC / draft.

alamb · 2023-11-14T22:00:09Z

datafusion/common/src/logical_type.rs

+                    .collect::<Vec<_>>();
+                DataType::Struct(fields.into())
+            }
+            other => panic!("not implemented {other:?}"),


is the idea that DataType::Dictionary and DataType::RunEndEncoded would also be included here? If so it makes sense

Thanks for the heads up. This should include DataType::Dictionary and DataType::RunEndEncoded here.

alamb · 2023-11-14T22:02:25Z

datafusion/core/src/datasource/listing/table.rs

    pub format: Arc<dyn FileFormat>,
    /// The expected partition column names in the folder structure.
    /// See [Self::with_table_partition_cols] for details
+    /// TODO this maybe LogicalType


I think one usecase is to use dictionary encoding for the partition columns to minimize the overhead of creating such columns. As long as they can be represented / specified with LogicalType I think it is a good idea to try changing this too.

lewiszlw · 2024-01-10T08:38:58Z

What's the status of this pr? This should be a very useful feature.

alamb · 2024-01-11T19:12:54Z

I think this PR is stalled and I don't have any update

yukkit · 2024-03-09T07:55:30Z

Please accept my apologies for the delay. Due to personal circumstances, I have been unable to attend to any work. I will now proceed to resume work on this PR.

alamb · 2024-03-09T09:08:58Z

No worries at all -- I hope all is well and we look forward to this work

github-actions · 2024-05-09T01:46:40Z

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

therealsharath · 2024-05-13T23:16:47Z

Hello, sorry if this is a redundant question. What is the status of this PR?

alamb · 2024-05-14T19:20:42Z

Hello, sorry if this is a redundant question. What is the status of this PR?

I think it is stale and on track to be closed from what I can see

github-actions · 2024-07-14T01:55:37Z

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

alamb · 2024-07-22T11:58:20Z

FYI #11160 tracks a new proposal for this feature. It seems to be gaining traction

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 labels Nov 12, 2023

feat: try to support user-defined types

b2fc8e7

yukkit force-pushed the main branch from a91870c to b2fc8e7 Compare November 13, 2023 12:37

yukkit commented Nov 14, 2023

View reviewed changes

alamb changed the title ~~[draft] try to support user-defined types~~ [draft] Add LogicalType, try to support user-defined types Nov 14, 2023

This was referenced Nov 14, 2023

Add support for Arrow Extension Types apache/arrow-rs#4472

Closed

Remove dictionary type from Arrow logical type apache/arrow-rs#4729

Open

alamb reviewed Nov 14, 2023

View reviewed changes

alamb mentioned this pull request Nov 14, 2023

DataFusion weekly project plan (Andrew Lamb) - Nov 13, 2023 #8151

Closed

9 tasks

github-actions bot added the Stale PR has not had any activity for some time label May 9, 2024

github-actions bot removed the Stale PR has not had any activity for some time label May 14, 2024

notfilippo mentioned this pull request Jun 28, 2024

[draft] Add LogicalType, try to support user-defined types #11160

Closed

4 tasks

github-actions bot added the Stale PR has not had any activity for some time label Jul 14, 2024

github-actions bot closed this Jul 22, 2024

Conversation

yukkit commented Nov 12, 2023 • 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?

Features

New Additions

Major Changes

To Be Implemented

To Be Determined

Some Thoughts

Are these changes tested?

Are there any user-facing changes?

Uh oh!

yukkit commented Nov 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yukkit commented Nov 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DFSchema to Schema

No need to change

To be changed

Schema to DFSchema (To be changed)

Uh oh!

alamb commented Nov 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lewiszlw commented Jan 10, 2024

Uh oh!

alamb commented Jan 11, 2024

Uh oh!

yukkit commented Mar 9, 2024

Uh oh!

alamb commented Mar 9, 2024

Uh oh!

github-actions bot commented May 9, 2024

Uh oh!

therealsharath commented May 13, 2024

Uh oh!

alamb commented May 14, 2024

Uh oh!

github-actions bot commented Jul 14, 2024

Uh oh!

alamb commented Jul 22, 2024

Uh oh!

yukkit commented Nov 12, 2023 •

edited

Loading

yukkit commented Nov 12, 2023 •

edited

Loading

yukkit commented Nov 13, 2023 •

edited

Loading