Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-30 Thread via GitHub


alamb commented on PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#issuecomment-2922372187

   Thanks again @zhuqi-lucas and @xudong963 
   
   I have also made a PR to add a mention of this change to the upgrade guide:
   - https://github.com/apache/datafusion/pull/16216


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-29 Thread via GitHub


xudong963 merged PR #16142:
URL: https://github.com/apache/datafusion/pull/16142


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-29 Thread via GitHub


xudong963 commented on PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#issuecomment-2921292733

   Let's go, thank you all


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-29 Thread via GitHub


zhuqi-lucas commented on PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#issuecomment-2919746777

   Thank you @xudong963 for review!


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-29 Thread via GitHub


xudong963 commented on PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#issuecomment-2919399177

   I'm reviewing


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-28 Thread via GitHub


alamb commented on PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#issuecomment-2916703395

   I'll plan to merge this tomorrow unless I hear anything different


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-27 Thread via GitHub


zhuqi-lucas commented on PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#issuecomment-2914788318

   > Thank you @zhuqi-lucas -- this is great
   > 
   > FYI @jayzhan211 , @timsaucer @xudong963 and @comphead
   > 
   > I have a few suggestions on how to simplify the examples which would be 
nice to get in prior to merge but I don't think it is needed
   
   Thank you @alamb for review, i also addressed the suggestions about example 
in latest PR.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-27 Thread via GitHub


zhuqi-lucas commented on code in PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#discussion_r2110816762


##
datafusion-examples/examples/dataframe.rs:
##
@@ -182,7 +182,11 @@ async fn write_out(ctx: &SessionContext) -> 
std::result::Result<(), DataFusionEr
 let mut df = ctx.sql("values ('a'), ('b'), ('c')").await.unwrap();
 
 // Ensure the column names and types match the target table
-df = df.with_column_renamed("column1", "tablecol1").unwrap();
+df = df

Review Comment:
   Good suggestion! Thank you @alamb , addressed in latest PR.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` [datafusion]

2025-05-27 Thread via GitHub


zhuqi-lucas commented on code in PR #16142:
URL: https://github.com/apache/datafusion/pull/16142#discussion_r211081


##
datafusion/core/tests/user_defined/user_defined_plan.rs:
##
@@ -796,22 +798,30 @@ fn accumulate_batch(
 k: &usize,
 ) -> BTreeMap {
 let num_rows = input_batch.num_rows();
+
 // Assuming the input columns are
-// column[0]: customer_id / UTF8
+// column[0]: customer_id / UTF8 or UTF8View

Review Comment:
   Good suggestion! Thank you @alamb , addressed in latest PR.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]