This is quite subtle, but this implies that supportedTypes returns an empty set.

This has an effect in case of union types, which is currently the only useage of supportedTypes.

Test:

curl -u elastic:password -H "Content-Type: application/json" "127.0.0.1:9200/test" -XPUT -d '{                                                                    11:17
  "mappings": {                                                                                                         
        "properties": {
            "x": {       
                "type": "keyword"
            }
        }
  }
}'

curl -u elastic:password -H "Content-Type: application/json" "127.0.0.1:9200/test2" -XPUT -d '{                                                                   11:18
  "mappings": {
        "properties": {
            "x": {
                "type": "text"   
            }
        }
  }
}'

curl -u elastic:password -H "Content-Type: application/json" "127.0.0.1:9200/_query?format=txt" -d '{                                                             11:19
    "query": "from test* | eval y = now() + to_dateperiod(x)"}
'

This currently fails for the wrong reason:

"verification_exception","reason":"Found 1 problem\nline 1:45: Cannot use field [x] due to ambiguities being mapped as [2] incompatible types: [keyword] in [test], [text] in [test2]"}]

I think we need to do the following:

1. This PR makes supportedTypes change meaning! It's not "types I can convert" anymore, but "types I can convert if they come from a field". I think it's important we reflect that by renaming supportedTypes or at least giving it a better javadoc. Maybe rename to supportedFieldTypes and add a javadoc that says that we may be able to fold on more types than that in case of unrepresentable types.
2. See if we can make the error message better in case of union types. The problem is not that to_dateperiod doesn't accept keyword or text fields, it's that it doesn't apply to index fields at all. This can be in a follow up, or let's create an issue and come back to it.

Thanks for catching this issue! I agree we should show the correct error messages. I looked into it, and it looks to me that this could be related to the order of validations.

I have an idea to make it show the correct message, but it modifies resolveType() in an unusual way. resolveType() usually checks if its children is resolved at the very beginning, if there is unresolved child, the rest of the validations/checks will be skipped, and it kinds of hide the error messages that we wanted.

For this particular case, these two conversion functions don't support fields, if we add a check in resolveType() - if it sees a field, then return a type resolution error, the expected messages can be seen. However the check to fields need to happen before the check to its children, in order to have the expected messages returned by Analyzer's Verifier.

+ curl -u elastic:password -H 'Content-Type: application/json' '127.0.0.1:9200/_query?format=txt' -d '{
    "query": "from test* | eval y = now() + to_dateperiod(x)"}
'
{"error":{"root_cause":[{"type":"verification_exception","reason":"Found 2 problems\nline 1:45: Cannot use field [x] due to ambiguities being mapped as [2] incompatible types: [keyword] in [test], [text] in [test2]\nline 1:31: argument of [to_dateperiod(x)] must be a constant, received [x]"}],"type":"verification_exception","reason":"Found 2 problems\nline 1:45: Cannot use field [x] due to ambiguities being mapped as [2] incompatible types: [keyword] in [test], [text] in [test2]\nline 1:31: argument of [to_dateperiod(x)] must be a constant, received [x]"},"status":400}%       

With this change, the expected message can be seen in the VerificationException. This is what I can think so far to make the correct message show, please let me know if there is a better way, I think we should return the expected error messages.

I think this is wrong, because we can end up with 2 validation errors. This happens e.g. when you do from test* | eval y = (now() + to_dateperiod(to_dateperiod(x))) in the union type setup from before, where test* maps x as keyword and text in different indices. Or just from test* | eval x = now() + to_dateperiod(x).

This is only needed for union types, right? Because in case of union types, we'll have a FoldablesConvertFunction wrapping an UnsupportedAttribute. So, we end up with a plan like

[2024-09-09T12:35:40,032][INFO ][o.e.x.e.a.Analyzer       ] [runTask-0] Rule analysis.Analyzer$UnionTypesCleanup applied
Limit[1000[INTEGER]]                         = Limit[1000[INTEGER]]
\_Eval[[NOW() + TODATEPERIOD(x{f}#15) AS x]] ! \_Eval[[NOW() + TODATEPERIOD(!x) AS x]]
  \_EsRelation[test*][x{f}#15]               !   \_EsRelation[test*][!x]

Where the EVAL has an unsupported attribute !x and TODATEPERIOD(...) has unresolved types because !x is not a constant.

Let's do the following:

• Let's undo the fix for union types, as well as the extra check in FoldablesConvertFunction.resolveType. Let's tackle this in a follow up. It's annoying, but this is more intricate than we thought because we ended up in union type territory and we need to deal with this :)
• Commenting out this line had no tests fail for me when running ./gradlew ":x-pack:plugin:esql:test". The follow up needs tests with union types that fail without a fix. Union types are currently a bit painful to set up, but we have yaml tests for them - or we could improve the verifier tests to enable testing union types. The two queries I wrote above could be a good start to cover these edge cases in yaml tests.
• I believe the verifier code here is not the best place to add union type specific workarounds, and ideally we also shouldn't need the extra check for field attributes in FoldablesConvertFunction.resolveType. I think the best way is to fix this in Analyzer.ResolveUnionTypes, more specifically in resolveConvertFunction (Analyzer.java, line 1227). We need to generate better error messages here, anyway, in case the query has a convert function applied to an index field, but there's either no evaluator (this is the case here) or the type doesn't make sense, like to_ip applied to a multi-typed field where one of the types is double.
• I really think the follow up PR should also rename AbstractConvertFunction.supportedTypes to evaluableTypes to avoid footguns in the future.

#112668 is opened to follow up with this issue.

I agree that the solution should be in the Analyzer.ResolveUnionTypes, and also think it could be related to another idea we have, which is to support union-types for more functions than just convert functions. Currently we assume that all convert functions support union-types and no other functions do. But this discussion clarifies that there exist convert functions that do not. So what about creating an interface for functions that support union-types, make sure all but these foldable functions use that, and this opens up the option to add union-types to other functions too?