ramitg254 commented on PR #6853: URL: https://github.com/apache/hive/pull/6853#issuecomment-6521871256
> it’s SUM-only, can't we come up with a generic fix? `accumulatePartitionOnlyBatch` copies the existing `isRepeating` branch in each sum evaluator (vector[0] * batch.size, and the same temp.setFromLong(size) / mutateMultiply for decimal). A partition-only argument is just a repeating column. There’s already setRepeatingColumn(partitionKey, ...) in VectorPTFGroupBatches, used for the overflow batch. > > An alternative would be to give the partition-only argument a slot in the buffered layout without copying data in bufferGroupBatch. Then fill it as a repeating vector from partitionKey in finishPartition. With that, every evaluator (MIN/MAX/AVG/FIRST_VALUE/LAST_VALUE/COUNT/LEAD/LAG) works through its existing repeating path. It would also remove PARTITION_ONLY_COL_ALLOWED_FUNCTIONS, the per-evaluator overrides, and the “not implemented” throw in the base class. Yes this makes sense, earlier I was honoring this comment: https://github.com/apache/hive/blob/7e1a4d91046719730e8a89080033688ee365a750/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java#L434 and went for the evaluaor explicit fix. but didn't realise this case is just a gap introduced when the code started selectively buffering columns in place of entire batch so this generic solution by adding index for partition only column case would be better, so will go ahead with that and will update the 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]
