feat: warn when limit reached in expr.execute()

[scottcode]

When executing an expression a limit is silently applied to what is returned (unless you pass limit=None). A user could accidentally continue with local analysis on an arbitrary subset without realizing it's a subset. I'd like an optional parameter/setting that causes it to write a warning to stderr or raise an exception if the limit was exceeded.

Since the limit is currently implemented by adding a limit expression to the original query, it may be difficult to know whether the limit would have been exceeded. In that case, maybe it could just check to see if the number of returned records equals the limit and issue the warning then. It might not definitively mean that the limit would've been exceeded, but would still be useful to get a warning that the limit was potentially exceeded.

[ian-r-rose]

I very much agree with this.

[xmnlab]

Is there a reason to keep this? IIRC we removed that for omniscidb backend

[scottcode]

To keep what, @xmnlab ?

[scottcode]

Do you mean the application of a (default) limit in the execute method?

[xmnlab]

sorry I was not clear ... I mean .. is there a reason to keep a default limit != None?

we are setting that to None inside omniscidb because it was generating conflict with sql method at at that time ... after that we change the algorithm related to this method .. and probably it wouldn't be a problem now .. but we kept it as None.

[jcrist]

I agree with the arguments to change the default limit to None. We'll keep the mechanism for a default limit around in case people want to enable an extra guard rail, but by default it seems safer to have no limit (to avoid silent truncation) than to enable an arbitrary limit. There's a PR up implementing this change at #4650.