Fang Xing
Fang Xing
> I think this may require [opening up the `PropagateEvalFoldables` optimization rule to aggregations](https://github.com/elastic/elasticsearch/blob/6827f002bb3af3774beecd480b03d1246bd8395d/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java#L501) - if so, it might be best if we tackle aggs on consts first (#100634). Indeed,...
> Thanks @fang-xing-esql , looks good! > > I'd increase test coverage a bit, though; maybe we could also add an analyzer/verifier test that confirms that arbitrary functions producing a...
@costin @astefan @bpintea Could you help take another look at this? Thank you!
Thanks for reviewing @costin @bpintea @alex-spies ! Just need to clean up CI.
Thanks for the first round of reviewing @costin @bpintea !! I think I have addressed most of the comments if not all yet. Could you please take another look at...
> @fang-xing-esql I am playing with your PR, really looking forward to it! > > I see that when the params is not provided then the query will fail >...
> @fang-xing-esql Let's improve the error messages and make them more user friendly: instead of `no parameter defined for name ?avg`, how about `unknown query parameter [avg]`. The message should...
> I haven't looked at the tests yet, but there are aspects that can be improved regarding the error messaging and, probably, the way these error messages are tested. Thanks...
> I haven't looked at the tests yet, but there are aspects that can be improved regarding the error messaging and, probably, the way these error messages are tested. Thank...