You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "clintropolis (via GitHub)" <gi...@apache.org> on 2023/03/07 04:05:41 UTC

[GitHub] [druid] clintropolis commented on pull request #13885: relax native expression multi-value string usage validation for conditional and null coalescing functions

clintropolis commented on PR #13885:
URL: https://github.com/apache/druid/pull/13885#issuecomment-1457491087

   >As to motivation— are there some queries that can plan after this change, that couldn't plan before?
   
   Right now there are certain queries that are basically impossible to write, anything involving null value coercion or conditionals combined with explicit multi-value string functions, such as all of the where clauses in the query added in `CalciteMultiValueStringQueryTest`, because the first set of functions assume a multi-value string is being used as a scalar and mixing with any of the `MV_` triggers the validation failure.
   
   This same information is currently used to determine if an expression should be automatically 'applied' because the string is being used as a scalar value but is in fact multi-valued, so the risk of these changes is that if something was relying on NVL to be mapped across each element of the multi-value string instead of replace the value with some array then that behavior will no longer happen in this PR.
   
   This is why I am working on a follow-up that is what i consider the _real_ fix, which is to switch to using the type inference system so that way an expression could look at its inputs inferred output types to determine if a multi-value string is being used as a scalar or as an array, and enforce consistency (and determine if translation is required) that way, but I'm not completely sure how complicated this fix is so put this up as a interim fix because i feel like it improves the behavior, though its certainly worth a discussion.
   
   The more I think about it though, i'm feeling a bit less good about this interim fix so I've been continuing on the more correct fix of consolidating the stuff on type inference.


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org