db-scnakandala commented on PR #39722: URL: https://github.com/apache/spark/pull/39722#issuecomment-1423094159
> > Maybe we can change the default value to 2. WDYT? > > Now the description of the config says the `The minimum number of consecutive commutative expressions`, but seemingly the logic uses the number of non-commutative operads below those commutative expressions (`operands.length < SQLConf.get.getConf(MULTI_COMMUTATIVE_OP_OPT_THRESHOLD)`). So I would argue that the default value 2 sounds good, but the logic should follow that description and we should optimize only when there are at least 2 nested expressions and shouldn't canonicalize a single expression to `MultiCommutativeOp`. @peter-toth Can you elaborate more on the statement "shouldn't canonicalize a single expression to `MultiCommutativeOp`" From what I understand since the operator is a commutative expression, it should at least have two operands (?) -- 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]
