You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kosiew (via GitHub)" <gi...@apache.org> on 2026/02/07 02:40:43 UTC

[PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

kosiew opened a new pull request, #20202:
URL: https://github.com/apache/datafusion/pull/20202

   
   ## Which issue does this PR close?
   
   * Closes #20162.
   
   ## Rationale for this change
   
   DataFusion’s physical expression adapter needs a reliable, schema-aware way to cast columns—especially nested `Struct` columns—while honouring field-level nullability metadata.
   
   Today, casting pathways often depend on Arrow’s `CastOptions<'static>` / `FormatOptions<'static>`, which are awkward for long-lived expressions (and effectively require static string lifetimes). This makes it hard to propagate dynamic formatting options (e.g. from SQL, IPC, or protobuf) without leaking or interning strings.
   
   This PR introduces:
   
   * A dedicated `CastColumnExpr` physical expression for struct-aware casting with explicit input/target fields.
   * Owned cast/format options (`OwnedCastOptions`, `OwnedFormatOptions`) so format strings can be carried safely across planning/serialization without requiring `'static` lifetimes.
   
   Together, these changes improve correctness (nullability validation), reliability (schema-accurate casting), and extensibility (future cast formatting support).
   
   ## What changes are included in this PR?
   
   * **Owned formatting + cast options**
   
     * Added `OwnedFormatOptions` (owned `String`-based variant of Arrow’s `FormatOptions`).
     * Added `OwnedCastOptions` (pairs `safe` + `OwnedFormatOptions`) with conversion helpers to Arrow `CastOptions<'a>`.
     * Re-exported these types from `datafusion_common`.
   
   * **CastOptions lifetime improvements**
   
     * Updated scalar/columnar casting APIs to accept `CastOptions<'_>` instead of requiring `CastOptions<'static>`.
     * Updated `ColumnarValue::cast_to` to avoid cloning options unnecessarily and to cleanly fall back to defaults.
   
   * **Schema-aware, struct-aware CastColumnExpr**
   
     * Reworked `CastColumnExpr` to:
   
       * Store `OwnedCastOptions` and an `input_schema` for proper column resolution.
       * Add `new_with_schema(...)` constructor for cases where expression resolution depends on a broader schema.
       * Validate cast compatibility up-front (including index bounds checks and nullability constraints).
       * Use `validate_struct_compatibility` and newly-exported `validate_field_compatibility` for consistent checks across scalar and nested contexts.
   
   * **PhysicalExprAdapter improvements**
   
     * Adapter now uses `validate_field_compatibility` to validate non-struct casts, producing clearer errors.
     * Uses `CastColumnExpr::new_with_schema(...)` to ensure the constructed cast expression is schema-accurate when columns are rewritten/reindexed.
   
   * **Nested struct casting consistency**
   
     * Exposed `validate_field_compatibility` as `pub` and aligned struct validation behavior.
     * Minor cleanup to field matching wording/comments and some test expectations.
   
   * **Proto updates (physical expr + options)**
   
     * Added protobuf support for `PhysicalCastColumnNode` and `PhysicalCastOptions`.
     * Added protobuf `FormatOptions` + `DurationFormat` to represent owned formatting options.
     * Kept backward compatibility fields (`safe`, `format_options`) in `PhysicalCastColumnNode` with a deprecation note and precedence rule.
     * Removed deprecated/unused protobuf messages/fields (e.g. `BufferExecNode`, `FileOutputMode` / `file_output_mode`, expr_id).
   
   * **Tests and fixture adjustments**
   
     * Added/updated unit tests for:
   
       * nullable → non-nullable cast rejection
       * schema mismatch errors
       * struct casting behavior with missing children
     * Updated a number of parquet-related tests/schemas to mark columns as nullable where appropriate to avoid invalid nullable→non-nullable casts after stricter validation.
   
   ## Are these changes tested?
   
   Yes.
   
   * Added new unit tests in `cast_column.rs` covering:
   
     * schema mismatch (type incompatibility)
     * nullability enforcement (nullable → non-nullable rejection)
   * Updated existing tests in:
   
     * `nested_struct.rs`
     * parquet filter / adapter tests
     * physical expr adapter tests
   
   These tests validate both the new expression behavior and the updated validation rules.
   
   ## Are there any user-facing changes?
   
   Potentially yes:
   
   * **Stricter nullability enforcement during casting**: casts that would silently allow nullable → non-nullable coercions may now be rejected earlier with clearer errors.
   * **Improved struct casting behavior**: struct fields are matched by name and validated consistently; missing fields in the target are filled with nulls (when allowed), extra fields are ignored.
   * **Better error messages** for incompatible casts during physical plan adaptation.
   
   
   ## LLM-generated code disclosure
   
   This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed and tested.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2839521339


##########
datafusion-examples/examples/custom_data_source/custom_file_casts.rs:
##########
@@ -49,9 +49,9 @@ use object_store::{ObjectStore, PutPayload};
 pub async fn custom_file_casts() -> Result<()> {
     println!("=== Creating example data ===");
 
-    // Create a logical / table schema with an Int32 column
+    // Create a logical / table schema with an Int32 column (nullable)
     let logical_schema =
-        Arc::new(Schema::new(vec![Field::new("id", DataType::Int32, false)]));
+        Arc::new(Schema::new(vec![Field::new("id", DataType::Int32, true)]));

Review Comment:
   Created #20486 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2804845043


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -896,6 +896,8 @@ message PhysicalExprNode {
     UnknownColumn unknown_column = 20;
 
     PhysicalHashExprNode hash_expr = 21;
+
+    PhysicalCastColumnNode cast_column = 22;

Review Comment:
   This should be it's own PR. You can make one right now to add `PhysicalCastColumnNode` and then after merging `OwnedFormatOptions` (or in the same PR that is added) you can add `message FormatOptions`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2839461314


##########
datafusion/physical-expr-adapter/src/schema_rewriter.rs:
##########
@@ -468,7 +468,10 @@ impl DefaultPhysicalExprAdapterRewriter {
         column: Column,
         logical_field: &Field,
     ) -> Result<Transformed<Arc<dyn PhysicalExpr>>> {
-        let actual_physical_field = self.physical_file_schema.field(column.index());
+        // Look up the column index in the physical schema by name to ensure correctness
+        let physical_column_index = self.physical_file_schema.index_of(column.name())?;
+        let actual_physical_field =
+            self.physical_file_schema.field(physical_column_index);

Review Comment:
   Good point.
   
   Created PR - https://github.com/apache/datafusion/pull/20485



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#issuecomment-3877017211

   @adriangb 
   
   > @kosiew could you help me understand your long term vision here? Are we going to fold cast_column(...) into cast(...) or something else?
   
   Yes, this is the plan:
   // Today 
   CastExpr::new(expr, DataType::Int64)  // Fast path for simple casting
   CastColumnExpr::new(expr, input_field, target_field)  // Schema-aware
   
   // next phaase (#20163 or #20164)
   CastExpr::with_schema_fields(expr, input_field, target_field)  // New builder method
     // Internally delegates to CastColumnExpr if struct type
   
   
   > Also at least the PR description is outdated.....and if it is here IMO is should be it's own PR, it seems like a valuable but drive by improvement unrelated to the rest of these changes).
   
   Thanks for spotting this.
   I reverted the [drive by refactor](https://github.com/apache/datafusion/pull/20202/commits/96b461414b4e6a49c1b6317ed136bfe5d5a6238d) but forgot to update PR description


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#issuecomment-3898808974

   
   > @comphead since your team uses struct casting and you've basically written your own custom casting implementation, I wonder if you can find time to help get these improvements to struct casing in DataFusion so you can remove your custom implementations?
   
   Thanks @adriangb we extremely close to wrap up the migration, once we have stable code merged we gonna have post migration. It would be awesome having all the cast in a single place I agree
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2845831147


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -1003,6 +1005,41 @@ message PhysicalTryCastNode {
 message PhysicalCastNode {
   PhysicalExprNode expr = 1;
   datafusion_common.ArrowType arrow_type = 2;
+  PhysicalCastOptions cast_options = 3;
+}
+
+message PhysicalCastColumnNode {

Review Comment:
   I agree that adding protobuf for CastColumnExpr is not consistent with the goal of merging into CastExpr later.
   
   I would like to close this in favour of #20269 instead.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#issuecomment-3872184302

   @kosiew could you help me understand your long term vision here? Are we going to fold `cast_column(...)` into `cast(...)` or something else?
   
   Also at least the PR description is outdated:
   
   > Simplified and clarified SpillPoolWriter example by removing unnecessary concurrent task spawning from the doc snippet.
   
   I don't see this change anywhere (and if it is here IMO is should be it's own PR, it seems like a valuable but drive by improvement unrelated to the rest of these changes).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2804845043


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -896,6 +896,8 @@ message PhysicalExprNode {
     UnknownColumn unknown_column = 20;
 
     PhysicalHashExprNode hash_expr = 21;
+
+    PhysicalCastColumnNode cast_column = 22;

Review Comment:
   This should be it's own PR, sequenced after adding `OwnedFormatOptions`



##########
datafusion-examples/examples/custom_data_source/custom_file_casts.rs:
##########
@@ -49,9 +49,9 @@ use object_store::{ObjectStore, PutPayload};
 pub async fn custom_file_casts() -> Result<()> {
     println!("=== Creating example data ===");
 
-    // Create a logical / table schema with an Int32 column
+    // Create a logical / table schema with an Int32 column (nullable)
     let logical_schema =
-        Arc::new(Schema::new(vec![Field::new("id", DataType::Int32, false)]));
+        Arc::new(Schema::new(vec![Field::new("id", DataType::Int32, true)]));

Review Comment:
   This could also be it's own PR



##########
datafusion/common/src/format.rs:
##########
@@ -35,6 +35,204 @@ pub const DEFAULT_CAST_OPTIONS: CastOptions<'static> = CastOptions {
     format_options: DEFAULT_FORMAT_OPTIONS,
 };
 
+/// Owned version of Arrow's `FormatOptions` with all `String` values instead of `&str`.
+///
+/// Arrow's `FormatOptions<'static>` requires `&'static str` references, which makes it
+/// difficult to work with dynamic format options. This struct uses `String` values,
+/// allowing format options to be created and owned at runtime without lifetime constraints.
+///

Review Comment:
   Should we make changes to arrow? It seems unfortunate to have duplicate types that differ only in their lifetime. 



##########
datafusion/physical-expr-adapter/src/schema_rewriter.rs:
##########
@@ -468,7 +468,10 @@ impl DefaultPhysicalExprAdapterRewriter {
         column: Column,
         logical_field: &Field,
     ) -> Result<Transformed<Arc<dyn PhysicalExpr>>> {
-        let actual_physical_field = self.physical_file_schema.field(column.index());
+        // Look up the column index in the physical schema by name to ensure correctness
+        let physical_column_index = self.physical_file_schema.index_of(column.name())?;
+        let actual_physical_field =
+            self.physical_file_schema.field(physical_column_index);

Review Comment:
   @kosiew this change seems like it could be it's own PR. I (and other reviewers) would really appreciate if each PR is minimally scoped. A PR with just this fix + some tests can be reviewed and merged in 5 minutes instead of getting stuck behind other review comments for unrelated things. They can also be easily cherry picked, back-ported, etc.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#issuecomment-3891559646

   Okay sounds good. In my mind as long as we land on a unified cast expr that can handle "all types" we're 👍🏻. I will review this today. I think you might guess that a bit of my feedback is going to be "how can we split this change up". I know it's annoying to keep asking that but review difficulty grows exponentially with PR size.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew closed pull request #20202: Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting
URL: https://github.com/apache/datafusion/pull/20202


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2845831147


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -1003,6 +1005,41 @@ message PhysicalTryCastNode {
 message PhysicalCastNode {
   PhysicalExprNode expr = 1;
   datafusion_common.ArrowType arrow_type = 2;
+  PhysicalCastOptions cast_options = 3;
+}
+
+message PhysicalCastColumnNode {

Review Comment:
   I agree that adding protobuf for CastColumnExpr is not consistent with the goal of merging into CastExpr later.
   
   I would like to close this in favour of #20269 which has a more focused scope and does not include protobuf.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2839461314


##########
datafusion/physical-expr-adapter/src/schema_rewriter.rs:
##########
@@ -468,7 +468,10 @@ impl DefaultPhysicalExprAdapterRewriter {
         column: Column,
         logical_field: &Field,
     ) -> Result<Transformed<Arc<dyn PhysicalExpr>>> {
-        let actual_physical_field = self.physical_file_schema.field(column.index());
+        // Look up the column index in the physical schema by name to ensure correctness
+        let physical_column_index = self.physical_file_schema.index_of(column.name())?;
+        let actual_physical_field =
+            self.physical_file_schema.field(physical_column_index);

Review Comment:
   Good point.
   
   Created https://github.com/apache/datafusion/pull/20485



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#issuecomment-3896222554

   @adriangb 
   
   > review difficulty grows exponentially with PR size.
   
   Please ignore the diff in these 2 files. 
   
   <img width="1468" height="752" alt="image" src="https://github.com/user-attachments/assets/38b73dbf-b3e1-47fb-9d0e-92db643397d9" />
   
   They are auto-generated by the protobuf codegen in the datafusion-proto crate by these schema changes:
   - Added PhysicalCastColumnNode and wired it into PhysicalExprNode (new oneof arm).
   - Added PhysicalCastOptions, FormatOptions, and DurationFormat, plus new cast_options fields and compatibility fields (safe, format_options) on cast nodes.
   - Added cast_options to PhysicalCastNode.
   
   The changes to review are therefore 700+ lines.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2804880951


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -1003,6 +1005,41 @@ message PhysicalTryCastNode {
 message PhysicalCastNode {
   PhysicalExprNode expr = 1;
   datafusion_common.ArrowType arrow_type = 2;
+  PhysicalCastOptions cast_options = 3;
+}
+
+message PhysicalCastColumnNode {
+  PhysicalExprNode expr = 1;
+  datafusion_common.Field input_field = 2;
+  datafusion_common.Field target_field = 3;
+  // DEPRECATED: Use cast_options instead of safe/format_options.
+  // These fields retained for backward compatibility with DataFusion < 52.0
+  // When deserializing, safe and format_options are only used if cast_options is not set.
+  bool safe = 4;

Review Comment:
   I'm confused: if this message is new, why do we need backwards compat fields?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Add schema-aware CastColumnExpr with owned cast/format options for safe struct casting [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20202:
URL: https://github.com/apache/datafusion/pull/20202#discussion_r2804884293


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -1003,6 +1005,41 @@ message PhysicalTryCastNode {
 message PhysicalCastNode {
   PhysicalExprNode expr = 1;
   datafusion_common.ArrowType arrow_type = 2;
+  PhysicalCastOptions cast_options = 3;
+}
+
+message PhysicalCastColumnNode {

Review Comment:
   I'm concerned that `CastColumnExpr` is growing wider in usage, e.g. adding protobuf. If our goal is to merge everything into `Cast` isn't this counterproductive?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org