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

🛠 refactor: return an explicit error for unrecognized data types instead of defaulting to null #1223

Open
de-sh opened this issue Mar 4, 2025 · 0 comments

Comments

@de-sh
Copy link
Contributor

de-sh commented Mar 4, 2025

Falling back to DataType::Null for unknown types can silently mask typos or misunderstandings in the schema. Consider returning an error to highlight the invalid type rather than converting it to null.

- _ => DataType::Null,
+ other => {
+     return Err(anyhow!("Unsupported or unrecognized data type: {other}"));
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            let parsed_field = Fields {
                name: field.name.clone(),

                data_type: {
                    match field.data_type.as_str() {
                        "int" => DataType::Int64,
                        "double" | "float" => DataType::Float64,
                        "boolean" => DataType::Boolean,
                        "string" => DataType::Utf8,
                        "datetime" => DataType::Timestamp(TimeUnit::Millisecond, None),
                        "string_list" => {
                            DataType::List(Arc::new(Field::new("item", DataType::Utf8, true)))
                        }
                        "int_list" => {
                            DataType::List(Arc::new(Field::new("item", DataType::Int64, true)))
                        }
                        "double_list" | "float_list" => {
                            DataType::List(Arc::new(Field::new("item", DataType::Float64, true)))
                        }
                        "boolean_list" => {
                            DataType::List(Arc::new(Field::new("item", DataType::Boolean, true)))
                        }
                        other => {
                            return Err(anyhow!("Unsupported or unrecognized data type: {other}"));
                        }
                    }
                },
                nullable: default_nullable(),
                dict_id: default_dict_id(),
                dict_is_ordered: default_dict_is_ordered(),
                metadata: HashMap::new(),
            };

            fields.push(parsed_field);

Originally posted by @coderabbitai[bot] in #1192 (comment)

@de-sh de-sh changed the title Refactor suggestion: Return an explicit error for unrecognized data types instead of defaulting to null 🛠 Refactor suggestion: Return an explicit error for unrecognized data types instead of defaulting to null Mar 4, 2025
@de-sh de-sh changed the title 🛠 Refactor suggestion: Return an explicit error for unrecognized data types instead of defaulting to null 🛠 refactor: return an explicit error for unrecognized data types instead of defaulting to null Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant