Re: [PR] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-17 Thread via GitHub 


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

   Thanks @timsaucer and @paleolimbot 


-- 
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] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-17 Thread via GitHub 


alamb merged PR #16320:
URL: https://github.com/apache/datafusion/pull/16320


-- 
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] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-10 Thread via GitHub 


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

   🤖: Benchmark completed
   
   Details
   
   
   
   ```
   group alamb_field_metadata2  
main
   - -  

   logical_aggregate_with_join   1.02729.6±3.11µs? 
?/sec1.00716.1±3.21µs? ?/sec
   logical_select_all_from_1000  1.00123.8±0.46ms? 
?/sec1.01125.1±0.26ms? ?/sec
   logical_select_one_from_700   1.03419.2±2.70µs? 
?/sec1.00408.8±2.44µs? ?/sec
   logical_trivial_join_high_numbered_columns1.02377.3±2.28µs? 
?/sec1.00369.5±1.77µs? ?/sec
   logical_trivial_join_low_numbered_columns 1.03363.4±1.60µs? 
?/sec1.00354.1±1.70µs? ?/sec
   physical_intersection 1.02835.9±5.05µs? 
?/sec1.00822.0±6.34µs? ?/sec
   physical_join_consider_sort   1.02   1379.8±6.90µs? 
?/sec1.00   1354.7±5.39µs? ?/sec
   physical_join_distinct1.03353.3±1.61µs? 
?/sec1.00344.5±1.38µs? ?/sec
   physical_many_self_joins  1.03 10.3±0.04ms? 
?/sec1.00 10.0±0.04ms? ?/sec
   physical_plan_clickbench_all  1.00139.7±1.30ms? 
?/sec1.03144.0±1.47ms? ?/sec
   physical_plan_clickbench_q1   1.00  1697.7±18.59µs? 
?/sec1.00  1694.5±26.06µs? ?/sec
   physical_plan_clickbench_q10  1.00  2.4±0.02ms? 
?/sec1.01  2.5±0.03ms? ?/sec
   physical_plan_clickbench_q11  1.00  2.5±0.02ms? 
?/sec1.03  2.6±0.03ms? ?/sec
   physical_plan_clickbench_q12  1.00  2.6±0.02ms? 
?/sec1.02  2.7±0.02ms? ?/sec
   physical_plan_clickbench_q13  1.00  2.3±0.03ms? 
?/sec1.01  2.4±0.02ms? ?/sec
   physical_plan_clickbench_q14  1.00  2.5±0.02ms? 
?/sec1.02  2.6±0.03ms? ?/sec
   physical_plan_clickbench_q15  1.00  2.4±0.02ms? 
?/sec1.01  2.4±0.02ms? ?/sec
   physical_plan_clickbench_q16  1.00  2.3±0.02ms? 
?/sec1.02  2.3±0.02ms? ?/sec
   physical_plan_clickbench_q17  1.00  2.4±0.01ms? 
?/sec1.03  2.4±0.03ms? ?/sec
   physical_plan_clickbench_q18  1.00  1933.0±18.29µs? 
?/sec1.02  1966.0±19.30µs? ?/sec
   physical_plan_clickbench_q19  1.00  2.8±0.03ms? 
?/sec1.03  2.9±0.03ms? ?/sec
   physical_plan_clickbench_q2   1.00  1904.1±23.73µs? 
?/sec1.01  1917.6±22.98µs? ?/sec
   physical_plan_clickbench_q20  1.00  1622.7±14.08µs? 
?/sec1.03  1663.9±18.50µs? ?/sec
   physical_plan_clickbench_q21  1.00  1932.7±27.82µs? 
?/sec1.03  2.0±0.02ms? ?/sec
   physical_plan_clickbench_q22  1.00  2.5±0.02ms? 
?/sec1.02  2.6±0.02ms? ?/sec
   physical_plan_clickbench_q23  1.00  2.8±0.02ms? 
?/sec1.03  2.9±0.03ms? ?/sec
   physical_plan_clickbench_q24  1.00  4.5±0.02ms? 
?/sec1.05  4.7±0.04ms? ?/sec
   physical_plan_clickbench_q25  1.00  1989.6±21.12µs? 
?/sec1.02  2.0±0.02ms? ?/sec
   physical_plan_clickbench_q26  1.00  1779.7±13.43µs? 
?/sec1.04  1849.8±20.58µs? ?/sec
   physical_plan_clickbench_q27  1.00  2.0±0.02ms? 
?/sec1.03  2.1±0.03ms? ?/sec
   physical_plan_clickbench_q28  1.00  2.7±0.02ms? 
?/sec1.03  2.8±0.03ms? ?/sec
   physical_plan_clickbench_q29  1.00  3.4±0.03ms? 
?/sec1.04  3.6±0.05ms? ?/sec
   physical_plan_clickbench_q3   1.01  1898.1±14.34µs? 
?/sec1.00  1887.6±16.67µs? ?/sec
   physical_plan_clickbench_q30  1.00 12.7±0.09ms? 
?/sec1.04 13.2±0.11ms? ?/sec
   physical_plan_clickbench_q31  1.00  2.8±0.03ms? 
?/sec1.03  2.9±0.02ms? ?/sec
   physical_plan_clickbench_q32  1.00  2.8±0.03ms? 
?/sec1.03  2.9±0.03ms? ?/sec
   physical_plan_clickbench_q33  1.00  2.4±0.02ms? 
?/sec1.03  2.5±0.03ms? ?/sec
   physical_plan

Re: [PR] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-10 Thread via GitHub 


alamb commented on code in PR #16320:
URL: https://github.com/apache/datafusion/pull/16320#discussion_r2139153886


##
datafusion/expr/src/expr.rs:
##
@@ -601,7 +625,7 @@ pub struct Alias {
 pub expr: Box,
 pub relation: Option,
 pub name: String,
-pub metadata: Option>,
+pub metadata: Option,

Review Comment:
   This point of this PR is to use the `FieldMetadata` struct introduced in 
this PR everywhere: 
   - https://github.com/apache/datafusion/pull/16317
   
   
   



-- 
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] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-10 Thread via GitHub 


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

   🤖 `./gh_compare_branch_bench.sh` [Benchmark 
Script](https://github.com/alamb/datafusion-benchmarking/blob/main/gh_compare_branch_bench.sh)
 Running
   Linux aal-dev 6.11.0-1015-gcp #15~24.04.1-Ubuntu SMP Thu Apr 24 20:41:05 UTC 
2025 x86_64 x86_64 x86_64 GNU/Linux
   Comparing alamb/field_metadata2 (3a7c9c80724182c03e1b4cc5e1d411dfd434e107) 
to 1d61f31d121632ca27c77c472cae7d604e9aa9d7 
[diff](https://github.com/apache/datafusion/compare/1d61f31d121632ca27c77c472cae7d604e9aa9d7..3a7c9c80724182c03e1b4cc5e1d411dfd434e107)
   BENCH_NAME=sql_planner
   BENCH_COMMAND=cargo bench --bench sql_planner
   BENCH_FILTER=
   BENCH_BRANCH_NAME=alamb_field_metadata2
   Results will be posted here when complete
   


-- 
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] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-10 Thread via GitHub 


alamb commented on code in PR #16320:
URL: https://github.com/apache/datafusion/pull/16320#discussion_r2139153886


##
datafusion/expr/src/expr.rs:
##
@@ -601,7 +625,7 @@ pub struct Alias {
 pub expr: Box,
 pub relation: Option,
 pub name: String,
-pub metadata: Option>,
+pub metadata: Option,

Review Comment:
   This line is the point of  the PR is to use the `FieldMetadata` struct 
introduced in this PR everywhere: 
   - https://github.com/apache/datafusion/pull/16317
   
   The rest of the changes in this PR are consequences of this change
   
   



-- 
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] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-07 Thread via GitHub 


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

   🤖: Benchmark completed
   
   Details
   
   
   
   ```
   group alamb_field_metadata2  
main
   - -  

   logical_aggregate_with_join   1.00727.2±5.41µs? 
?/sec1.00726.8±6.09µs? ?/sec
   logical_select_all_from_1000  1.00124.3±0.46ms? 
?/sec1.01125.2±1.46ms? ?/sec
   logical_select_one_from_700   1.00413.5±1.48µs? 
?/sec1.00414.5±1.00µs? ?/sec
   logical_trivial_join_high_numbered_columns1.00376.1±1.74µs? 
?/sec1.00376.7±1.88µs? ?/sec
   logical_trivial_join_low_numbered_columns 1.00361.4±2.00µs? 
?/sec1.01364.7±3.44µs? ?/sec
   physical_intersection 1.00833.0±9.70µs? 
?/sec1.00832.3±4.72µs? ?/sec
   physical_join_consider_sort   1.00  1382.7±20.89µs? 
?/sec1.01  1392.2±14.62µs? ?/sec
   physical_join_distinct1.00350.2±1.34µs? 
?/sec1.01352.3±1.62µs? ?/sec
   physical_many_self_joins  1.00 10.2±0.03ms? 
?/sec1.00 10.2±0.02ms? ?/sec
   physical_plan_clickbench_all  1.00142.1±1.85ms? 
?/sec1.01142.9±1.40ms? ?/sec
   physical_plan_clickbench_q1   1.00  1694.9±20.79µs? 
?/sec1.01  1703.7±23.20µs? ?/sec
   physical_plan_clickbench_q10  1.00  2.4±0.02ms? 
?/sec1.00  2.4±0.03ms? ?/sec
   physical_plan_clickbench_q11  1.01  2.6±0.03ms? 
?/sec1.00  2.5±0.01ms? ?/sec
   physical_plan_clickbench_q12  1.00  2.7±0.01ms? 
?/sec1.01  2.7±0.02ms? ?/sec
   physical_plan_clickbench_q13  1.02  2.4±0.03ms? 
?/sec1.00  2.3±0.01ms? ?/sec
   physical_plan_clickbench_q14  1.00  2.5±0.03ms? 
?/sec1.00  2.5±0.01ms? ?/sec
   physical_plan_clickbench_q15  1.00  2.4±0.04ms? 
?/sec1.00  2.4±0.04ms? ?/sec
   physical_plan_clickbench_q16  1.00  2.3±0.03ms? 
?/sec1.00  2.3±0.03ms? ?/sec
   physical_plan_clickbench_q17  1.01  2.4±0.03ms? 
?/sec1.00  2.4±0.02ms? ?/sec
   physical_plan_clickbench_q18  1.01  1961.5±17.48µs? 
?/sec1.00  1948.4±23.03µs? ?/sec
   physical_plan_clickbench_q19  1.00  2.9±0.03ms? 
?/sec1.01  2.9±0.04ms? ?/sec
   physical_plan_clickbench_q2   1.00  1914.2±19.20µs? 
?/sec1.00  1918.1±28.03µs? ?/sec
   physical_plan_clickbench_q20  1.01  1670.0±14.37µs? 
?/sec1.00  1652.7±31.38µs? ?/sec
   physical_plan_clickbench_q21  1.02  1998.1±32.35µs? 
?/sec1.00  1949.9±19.90µs? ?/sec
   physical_plan_clickbench_q22  1.00  2.6±0.02ms? 
?/sec1.00  2.5±0.02ms? ?/sec
   physical_plan_clickbench_q23  1.00  2.8±0.03ms? 
?/sec1.01  2.8±0.02ms? ?/sec
   physical_plan_clickbench_q24  1.00  4.5±0.03ms? 
?/sec1.05  4.7±0.04ms? ?/sec
   physical_plan_clickbench_q25  1.00  2.0±0.03ms? 
?/sec1.00  2.0±0.02ms? ?/sec
   physical_plan_clickbench_q26  1.00  1839.7±19.85µs? 
?/sec1.00  1833.7±28.51µs? ?/sec
   physical_plan_clickbench_q27  1.00  2.0±0.02ms? 
?/sec1.00  2.1±0.02ms? ?/sec
   physical_plan_clickbench_q28  1.00  2.8±0.05ms? 
?/sec1.00  2.8±0.03ms? ?/sec
   physical_plan_clickbench_q29  1.00  3.5±0.06ms? 
?/sec1.00  3.5±0.04ms? ?/sec
   physical_plan_clickbench_q3   1.00  1900.5±24.04µs? 
?/sec1.00  1894.8±15.69µs? ?/sec
   physical_plan_clickbench_q30  1.00 12.9±0.10ms? 
?/sec1.01 13.1±0.07ms? ?/sec
   physical_plan_clickbench_q31  1.00  2.8±0.05ms? 
?/sec1.01  2.9±0.02ms? ?/sec
   physical_plan_clickbench_q32  1.00  2.8±0.04ms? 
?/sec1.02  2.9±0.06ms? ?/sec
   physical_plan_clickbench_q33  1.00  2.4±0.02ms? 
?/sec1.00  2.4±0.02ms? ?/sec
   physical_plan

Re: [PR] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-07 Thread via GitHub 


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

   🤖 `./gh_compare_branch_bench.sh` [Benchmark 
Script](https://github.com/alamb/datafusion-benchmarking/blob/main/gh_compare_branch_bench.sh)
 Running
   Linux aal-dev 6.11.0-1013-gcp #13~24.04.1-Ubuntu SMP Wed Apr  2 16:34:16 UTC 
2025 x86_64 x86_64 x86_64 GNU/Linux
   Comparing alamb/field_metadata2 (b27552aa5da4a17860aef3acd39527f119669834) 
to 1daa5ed5cc51546904d45e23cc148601d973942a 
[diff](https://github.com/apache/datafusion/compare/1daa5ed5cc51546904d45e23cc148601d973942a..b27552aa5da4a17860aef3acd39527f119669834)
   BENCH_NAME=sql_planner
   BENCH_COMMAND=cargo bench --bench sql_planner
   BENCH_FILTER=
   BENCH_BRANCH_NAME=alamb_field_metadata2
   Results will be posted here when complete
   


-- 
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] Unify Metadata Handing: use `FieldMetadata` in `Expr::Alias` and `ExprSchemable` [datafusion]

2025-06-07 Thread via GitHub 


alamb commented on code in PR #16320:
URL: https://github.com/apache/datafusion/pull/16320#discussion_r2133774218


##
datafusion/expr/src/expr.rs:
##
@@ -3657,7 +3834,7 @@ mod test {
 // If this test fails when you change `Expr`, please try
 // `Box`ing the fields to make `Expr` smaller
 // See https://github.com/apache/datafusion/issues/16199 for details
-assert_eq!(size_of::(), 144);
+assert_eq!(size_of::(), 128);

Review Comment:
   And `Expr` gets smaller!



-- 
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]