[CALCITE-7457] VALUES and SELECT produce different validation results for the same expression

[dssysolyatin]

Jira Link

CALCITE-7457

Changes Proposed

• VALUES now routes through validateQuery, triggering TableConstructorNamespace.validateImpl validation (which calls validateValues and inferUnknownTypes). The fix is a validateCall override in SqlValuesOperator.
• inferUnknownTypes no longer descends into subqueries (node.isA(SqlKind.QUERY)). Each query handles its own inferUnknownTypes during its own validation.
• Added additional tests inside existing tests which can be dropped. Just to show that now SELECT and VALUES behaves the same way

[xiedeyantu]

The javadoc should be removed "Top-level"?

[xiedeyantu]

I have no other comments except for the above minor one, you can feel free to squash and merge.

[mihaibudiu]

This is a regression, there is not ROW expression in the original program. The user will have difficulties finding the location of this error.

[dssysolyatin]

You are correct that the original query does not contain a ROW. However, during parsing, the OVERLAPS arguments are wrapped inside a ROW prior to any validation

Currently, VALUES follows the proper execution order:
a. Operand inference
b. Operand validation

Operand inference throws an exception, but it probably should not. It should probably simply leave the types unfilled. Operand validation would then fail with something like

Cannot apply 'OVERLAPS' to arguments of type '<RECORDTYPE(DATE EXPR$0, DATE EXPR$1)> OVERLAPS <RECORDTYPE(DATE EXPR$0, DATE EXPR$1, DATE EXPR$2)>'. Supported form(s): '(<DT>, <DT>) OVERLAPS (<DT>, <DT>)'
'(<DT>, <DT>) OVERLAPS (<DT>, <INTERVAL>)'
'(<DT>, <INTERVAL>) OVERLAPS (<DT>, <DT>)'
'(<DT>, <INTERVAL>) OVERLAPS (<DT>, <INTERVAL>)'

Regarding the regression, I'm not sure it is regression, since SELECT already had the same behavior. Also SELECT * FROM VALUES()/INSERT/UPDATE/DELETE behave the same as they call validateValues which first:

1. Does operand inference
2. And then does operand validation

Ironically, VALUES expression (when VALUES is top level node) were the exception.

[mihaibudiu]

Do you know why no checks were run in CI?
I would expect this error using ROW to appear in many more cases.

[dssysolyatin]

Ah, this is because I add LGMT-will-merge-soon after first approve, it run CI one more time but with specific CI tests

[mihaibudiu]

Can you please run all the tests to check that there aren't other tests affected by the ROW message?

[dssysolyatin]

Those all ran before I added the LGTM-will-merge-soon label. Anyway, I made a small tweak based on #4858 (comment). Re-running now

[mihaibudiu]

Why should this inferred type be different?
Something is off.

[dssysolyatin]

I explained above, in the SELECT section, why this happens. I'm not saying it's the correct behavior and I would expect that 1.0 + NULL = NULL with type DECIMAL(2,1) (common not null values type) in this case, but currently it works like that

[mihaibudiu]

So you say SELECT 1.0 + NULL already was DECIMAL(3,1), but when used in VALUES the result was different?

[dssysolyatin]

Right, if you open task description - https://issues.apache.org/jira/browse/CALCITE-7457. You can see test case for main branch

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud