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.

Sign up for GitHub

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

Jump to bottom

exec: AVG aggregator should support ints #38823

Closed
rafiss opened this issue Jul 11, 2019 · 1 comment · Fixed by #49900
Closed

exec: AVG aggregator should support ints #38823

rafiss opened this issue Jul 11, 2019 · 1 comment · Fixed by #49900
Assignees
@yuzefovich
Labels
A-sql-vec SQL vectorized engine C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Jul 11, 2019

Currently only Decimal, Float32, and Float64 are supported (see avg_agg_gen.go).

It seems like it might require some special casing, since the output type should be a float if the input type is an int.
@rafiss rafiss added A-sql-execution Relating to SQL execution. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-vec SQL vectorized engine and removed A-sql-execution Relating to SQL execution. labels Jul 11, 2019
@rafiss rafiss mentioned this issue Jul 11, 2019
Merged
craig bot pushed a commit that referenced this issue Jul 11, 2019
@rafiss
Merge #38828
a8d548f 
38828: exec: handle nulls in AVG aggregator r=rafiss a=rafiss

closes #37738 

The random aggregation test uses float64 now so that AVG can be included in this test, until #38823 can be addressed.

```
BenchmarkAggregator/AVG/hash/Decimal/groupSize=1/hasNulls=false/numInputBatches=64-12       	      20	  72817604 ns/op	   7.20 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=1/hasNulls=true/numInputBatches=64-12        	      20	  67807960 ns/op	   7.73 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=2/hasNulls=false/numInputBatches=64-12       	      30	  45907478 ns/op	  11.42 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=2/hasNulls=true/numInputBatches=64-12        	      30	  44468409 ns/op	  11.79 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=512/hasNulls=false/numInputBatches=64-12     	     200	   8712009 ns/op	  60.18 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=512/hasNulls=true/numInputBatches=64-12      	     200	   8852871 ns/op	  59.22 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=1024/hasNulls=false/numInputBatches=64-12    	     200	   9058353 ns/op	  57.88 MB/s
BenchmarkAggregator/AVG/hash/Decimal/groupSize=1024/hasNulls=true/numInputBatches=64-12     	     200	   9289322 ns/op	  56.44 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=1/hasNulls=false/numInputBatches=64-12    	      20	  67161015 ns/op	   7.81 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=1/hasNulls=true/numInputBatches=64-12     	      20	  60712202 ns/op	   8.64 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=2/hasNulls=false/numInputBatches=64-12    	      30	  40953924 ns/op	  12.80 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=2/hasNulls=true/numInputBatches=64-12     	      30	  39335420 ns/op	  13.33 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=512/hasNulls=false/numInputBatches=64-12  	     300	   4201872 ns/op	 124.77 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=512/hasNulls=true/numInputBatches=64-12   	     300	   4202353 ns/op	 124.76 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=1024/hasNulls=false/numInputBatches=64-12 	     500	   3982187 ns/op	 131.66 MB/s
BenchmarkAggregator/AVG/ordered/Decimal/groupSize=1024/hasNulls=true/numInputBatches=64-12  	     500	   3766494 ns/op	 139.20 MB/s
```

Release note: None

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
@rafiss rafiss mentioned this issue Jul 12, 2019
Closed
@rafiss
Copy link
Collaborator Author

rafiss commented Jul 12, 2019

This would probably be resolved by #38845

@asubiotto asubiotto added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) and removed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Oct 22, 2019
@yuzefovich yuzefovich mentioned this issue Jun 5, 2020
Merged
@yuzefovich yuzefovich self-assigned this Jun 5, 2020
@craig craig bot closed this as completed in 1a9c9f2 Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yuzefovich yuzefovich

Labels
A-sql-vec SQL vectorized engine C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

colexec: increase support of aggregate functions yuzefovich/cockroach
3 participants
@rafiss @yuzefovich @asubiotto